-
Notifications
You must be signed in to change notification settings - Fork 747
Fix flaky sessions_pool test by aligning MinPoolSize with MaxActiveSessions #31673
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
base: main
Are you sure you want to change the base?
Conversation
|
Hi! Thank you for contributing! |
|
🟢 |
The test was flaky because MinPoolSize defaulted to 10 while MaxActiveSessions could be set to 1. When the periodic session cleanup task ran, it would check if sessionsCount > MinPoolSize before deleting idle sessions. With the default MinPoolSize of 10 and only 1 session, the condition (1 > 10) was false, so sessions should not be deleted. However, when sessions were removed from the pool for KeepAlive operations and the KeepAlive failed (due to network issues or timeouts during testing), the session could be lost, leading to an empty pool. By explicitly setting MinPoolSize to match MaxActiveSessions, we ensure that sessions are never deleted from the pool due to idle timeout, regardless of KeepAlive success or failure. This makes the test deterministic and prevents the flaky behavior where GetCurrentPoolSize() would sometimes return 0 instead of the expected activeSessionsLimit. Co-authored-by: asmyasnikov <[email protected]>
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a flaky test (YdbSdkSessionsPool.StressTestAsync/0) by aligning the MinPoolSize session pool setting with MaxActiveSessions to prevent sessions from being deleted during aggressive KeepAlive operations.
Key Changes
- Set
MinPoolSizeequal toMaxActiveSessionsin the test'sSetUp()method to ensure session pool stability during stress testing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
Removed executor configurations and added auto config settings.
Changelog entry
The
StressTestAsync/0test was flaky becauseMinPoolSizedefaulted to 10 whileMaxActiveSessionswas set to 1. Sessions removed for KeepAlive operations (triggered every 10ms by aggressive test thresholds) could be lost on failure, leaving the pool empty despite thesessionsCount > MinPoolSizedeletion guard.Changelog category
Description for reviewers
...
Changes
Set
MinPoolSizeto matchMaxActiveSessionsin test setup:This prevents session deletion regardless of KeepAlive outcomes, making the test deterministic.
Category
Changelog
Fix flaky
YdbSdkSessionsPool.StressTestAsync/0test by settingMinPoolSizeequal toMaxActiveSessions, preventing race condition where sessions could be lost during KeepAlive operations.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.