Skip to content

fix: clean up async Table SessionPool acquisition tasks - #919

Closed
dimastark wants to merge 1 commit into
ydb-platform:mainfrom
dimastark:fix-aio-session-pool-task-cleanup
Closed

dimastark wants to merge 1 commit into
ydb-platform:mainfrom
dimastark:fix-aio-session-pool-task-cleanup

Conversation

@dimastark

Copy link
Copy Markdown

Pull request type

  • Bugfix

What is the current behavior?

ydb.aio.SessionPool._get_session_from_queue() leaves its Event.wait() task pending after a successful queued acquisition or a timeout. Repeated queued acquisitions therefore accumulate stop-waiter tasks until the pool stops. Cancelling an acquisition can also lose a session that was dequeued just before cancellation.

Issue Number: N/A

What is the new behavior?

Both queue and stop helper tasks are cancelled when unfinished and awaited before acquisition completes, following the existing cleanup pattern in ydb/aio/_utilities.py. A dequeued session abandoned by cancellation is returned with its original queue priority; after shutdown it is destroyed without starting a replacement. The timeout and existing shutdown behavior are preserved.

Nine deterministic unit tests cover repeated queued acquisitions, positive and nonpositive timeouts, cancellation, and shutdown races using a real pool, queues and events with a mocked driver. All nine fail against the original implementation and pass with the fix. The tests are in the existing ydb/table_test.py and require no YDB server.

Other information

Local validation on Python 3.11.16 / macOS:

  • tox -e black
  • tox -e style
  • tox -e mypy — no issues in 108 source files
  • tox -e py -- ydb -v — 247 tests passed

Docker integration tests were not run locally.

@codecov

codecov Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.72%. Comparing base (b28a9b5) to head (cfd24bb).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #919      +/-   ##
==========================================
+ Coverage   82.68%   82.72%   +0.03%     
==========================================
  Files          99       99              
  Lines       12961    12969       +8     
  Branches     1269     1273       +4     
==========================================
+ Hits        10717    10728      +11     
+ Misses       1793     1791       -2     
+ Partials      451      450       -1     
Flag Coverage Δ
integration 80.40% <54.54%> (-0.05%) ⬇️
unit 49.84% <95.45%> (+0.63%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
ydb/aio/table.py 81.98% <100.00%> (+1.45%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dimastark
dimastark marked this pull request as ready for review October 7, 2026 08:28
@vgvoleg

vgvoleg commented Oct 7, 2026

Copy link
Copy Markdown
Member

Hi @dimastark, thanks for the contribution! We don’t actively maintain the Table Service API and plan to remove it in the future. Unless this change addresses a specific problem for a user, I’d prefer to leave this part of the code as it is.

As a wise man once said, “Работает - не трогай” 🙂

@vgvoleg vgvoleg closed this Oct 7, 2026
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.

2 participants