response-future: fence stale endpoint pool use#862
response-future: fence stale endpoint pool use#862dkropachev wants to merge 3 commits intodk/base-response-future-stale-pool-857from
Conversation
| if isinstance(host, Host): | ||
| with host.lock: | ||
| expected_endpoint = host.endpoint |
There was a problem hiding this comment.
Why do you need to be under lock for such simple assignment?
| expected_endpoint = None | ||
| if isinstance(host, Host): | ||
| with host.lock: | ||
| expected_endpoint = host.endpoint | ||
| pool = self.session._get_pool_by_host_identity( | ||
| host, expected_endpoint=expected_endpoint) | ||
| else: | ||
| pool = self.session._get_pool_by_host_identity(host) | ||
|
|
||
| if pool and expected_endpoint is not None: | ||
| with host.lock: | ||
| endpoint_changed = not self.session._endpoints_match( | ||
| host.endpoint, expected_endpoint) | ||
| if endpoint_changed: | ||
| self._errors[host] = ConnectionException( | ||
| "Host endpoint changed while borrowing connection") | ||
| return None | ||
|
|
There was a problem hiding this comment.
Waaait, this is PR from one of your branches to another one of your branches. Why?
I wanted to locally check what is _get_pool_by_host_identity because I don't understand the purpose of this change, but there is not such function.
I don't think you need PRs to make changes to your own work in progress. I think I should revisit this PR when it targets master.
There was a problem hiding this comment.
Ah, you mention this in cover letter, sorry.
There was a problem hiding this comment.
I still don't think there is much point in reviewing those changes before the base changes are understood and merged (or at least approved).
2 participants
Summary
Notes
This is a focused draft split from the host-state/pool race work. The base branch carries prerequisite fencing changes so this PR diff stays scoped to this issue.
Fixes #857
Testing
Not run for this split branch.