Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleaning: removes redundant api call to Pool.get_all #6138

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

GabrielBuica
Copy link
Contributor

With the changes made here, 3ba6e6d, calling Pool.get_all no longer updates the last_active field unless 10 minutes have elapsed since session creation. Therefore, this call should be redundant now.

Passes BVT + BST: 208579 (Dev Run) (except one sequence that hasn't been scheduled)

With the changes made here, 3ba6e6d,
calling `Pool.get_all` no longer updates the `last_active` field unless
10 minutes have elapsed since session creation.

Signed-off-by: Gabriel Buica <[email protected]>
@robhoes
Copy link
Member

robhoes commented Nov 26, 2024

See #5986 (comment). This call was there to "correct" the last_active field in case the pool member had a large amount of clock skew with respect to the coordinator. In particular, if the skew is larger than 24 hours, the session may be deleted by the DB GC very quickly. A pretty unusual scenario I suppose...

@edwintorok
Copy link
Contributor

if the skew is larger than 24 hours,

It is a bit odd if the host assigning the session time and the host running db_gc isn't the same one, I would've expected the coordinator to be that one in both cases (well I guess could happen during master promotion, or if this code runs on a member and the members insert the session directly into the DB).

@robhoes
Copy link
Member

robhoes commented Nov 27, 2024

@edwintorok This is the case particularly for pool members getting their own sessions, i.e. the thing being improved by the session-reuse PR.

@edwintorok
Copy link
Contributor

@edwintorok This is the case particularly for pool members getting their own sessions, i.e. the thing being improved by the session-reuse PR.

If needed we could make the API call only the first time when the session is created, and avoid making the call when we are reusing the session (that one needs some other mechanism of keeping it alive, and I think we will have some mechanism there to avoid destroying those sessions; although we might've temporarily reverted that code)

@GabrielBuica
Copy link
Contributor Author

I'll move this to draft while we discuss more solutions.

@GabrielBuica GabrielBuica marked this pull request as draft December 2, 2024 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants