Skip to content

Simplier sessions expiration in Keeper and add comments#28519

Merged
alesapin merged 7 commits intomasterfrom
better_session_expiration
Sep 4, 2021
Merged

Simplier sessions expiration in Keeper and add comments#28519
alesapin merged 7 commits intomasterfrom
better_session_expiration

Conversation

@alesapin
Copy link
Copy Markdown
Member

@alesapin alesapin commented Sep 2, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Fix strange sessions expiration logic in Keeper. Probably it should help in CI: https://clickhouse-test-reports.s3.yandex.net/0/6bd9b82141c98dcd7796fd9d08326831095ba519/stress_test_(debug).html#fail1

@alesapin alesapin added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label Sep 2, 2021
@robot-clickhouse robot-clickhouse added the pr-improvement Pull request with some product improvements label Sep 2, 2021
@tavplubix tavplubix self-assigned this Sep 2, 2021
Comment on lines 22 to 23
/// Session -> timeout ms
std::unordered_map<int64_t, int64_t> session_to_timeout;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's rather deadline than timeout, because it represents a point in time when session will be expired, not a time interval. Also it can be confused with KeeperStorage::session_and_timeout, which actually contains timeout (a time interval). Maybe rename it to session_to_deadline or session_to_expiration_time?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree

@alesapin alesapin merged commit aa58f22 into master Sep 4, 2021
@alesapin alesapin deleted the better_session_expiration branch September 4, 2021 12:44
robot-clickhouse pushed a commit that referenced this pull request Sep 6, 2021
robot-clickhouse pushed a commit that referenced this pull request Sep 6, 2021
alesapin added a commit that referenced this pull request Sep 8, 2021
Backport #28519 to 21.8: Simplier sessions expiration in Keeper and add comments
alesapin added a commit that referenced this pull request Sep 8, 2021
Backport #28519 to 21.9: Simplier sessions expiration in Keeper and add comments
@Felixoid Felixoid added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-improvement Pull request with some product improvements pr-must-backport Pull request should be backported intentionally. Use this label with great care!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants