cluster_async: Wrap internal state with Arc. #864
Conversation
|
@jaymell looks like we have some more flakyness - 2 test suites run for hours. |
Kicked off the jobs again. Have we seen this issue outside of this PR? I don't recall. |
|
@jaymell I'm not sure, I think I might've noticed this in the past.
|
|
But both runs seem to have been stuck on |
|
I believe I found &solved the deadlock. |
nihohit
force-pushed
the
split-cluster
branch
2 times, most recently
from
a8d1d90
to
1b3b366
Compare
|
Those deadlocks in the prior test failures have me a bit nervous. I think these latest changes look good, but I'm no longer confident I understand the risks of having an Also, what was your reasoning for switching to |
The risks of using a Mutex / RwLock are always that there might be a deadlock possible. On the other hand, the risks of not doing this are that the async cluster client won't behave correctly, or will implement fan-out commands serially instead of concurrently. IMO, since the test suite managed to find several issues, I'm feeling pretty confident about this.
The ability to hold multiple read locks during try_request. Although the locks are held very briefly each time, I would still prefer not to block concurrent requests at all. |
|
rebased @jaymell |
|
Sorry, rebase is again required, but let's get this in! |
This change allows us to send the internal state of the client to async functions without the `self` reference imposing a lifetime.
This solves a timing issue, where after calling `refresh_slots` there are old `InnerCore` objects with connection maps that are emptied or no longer relevant. Now the map is updated under a mutex, and no new `InnerCore` instanaces are created.
|
@jaymell rebased |
This change allows us to send the internal state of the client to async functions without the `self` reference imposing a lifetime.
This change allows us to send the internal state of the client to async functions without the
selfreference imposing a lifetime.