Skip to content

[Blockstore] Fix shared VolumeProxy connections - #7419

Open
Maximus5 wants to merge 2 commits into
mainfrom
users/maximus5/erase-connection-fix
Open

[Blockstore] Fix shared VolumeProxy connections#7419
Maximus5 wants to merge 2 commits into
mainfrom
users/maximus5/erase-connection-fix

Conversation

@Maximus5

@Maximus5 Maximus5 commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Problem

VolumeProxy kept only one connection for each Tablet, although several disk
connections can share it. Removing one connection could close a pipe still
in use. Removing the last connection could leave the pipe open. A disconnect
could leave requests on another connection waiting.

After a disk was resolved to a different real DiskId, the old mapping could
still point to its connection. This could leave a stale pointer when that
connection was removed.

Issue: #7418

Solution

Track all connections for each Tablet and close the shared pipe only when
the last connection is removed. Apply pipe connect and disconnect events to
the connections that use the pipe, while leaving connections that are still
resolving alone. Track active requests per connection so they can be
cancelled without scanning requests for other Tablets.

Remove old real DiskId mappings when a disk resolves to a new name. Handle
broken VolumeProxy state with an error instead of aborting the server.

Testing

All 25 VolumeProxy tests passed. They cover both connection expiry orders,
pipe shutdown after the last connection expires, request cancellation for
connections sharing a Tablet, re-describe with a changed real DiskId, and
pipe events while another connection is resolving.

./ya make -tA \
    cloud/blockstore/libs/storage/volume_proxy/ut \
    -F '*TVolumeProxyTest*'

@github-actions

github-actions Bot commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

  • ⚫ tasks + storage + fastshard (job skipped)
  • ⚫ blockstore (job skipped)
  • ⚫ disk + manager (job skipped)
  • ⚫ filestore (job skipped)

@asorotsky asorotsky added the blockstore Add this label to run only cloud/blockstore build and tests on PR label Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1396s): all tests PASSED for commit 81f3385.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
7585 7584 0 0 0 1 0

qkrorlqr
qkrorlqr previously approved these changes Oct 5, 2026
View reviewed changes
@Maximus5 Maximus5 added large-tests Launch large tests for PR and removed large-tests Launch large tests for PR labels Oct 5, 2026
@github-actions

github-actions Bot commented Oct 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 11891s): all tests PASSED for commit 46fb28b.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
8088 8061 0 0 0 27 0

sharpeye
sharpeye previously approved these changes Oct 6, 2026
View reviewed changes
@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 46fb28b to 3a112ba Compare October 9, 2026 15:27
@github-actions

github-actions Bot commented Oct 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🔴 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1796s): some tests FAILED for commit 3a112ba.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
7627 7615 0 11 0 1 0

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1015s): all tests PASSED for commit 3a112ba.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
7644 7643 0 0 0 1 0

@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 3a112ba to c06f4a0 Compare October 21, 2026 15:40
@github-actions

github-actions Bot commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

  • 🚫 blockstore (cancelled or timed out before reporting completion)

@Maximus5
Maximus5 added this pull request to stack #7592 October 21, 2026 15:43
@Maximus5 Maximus5 added the recheck Add this label to relaunch checks, it will be automatically removed label Oct 21, 2026
@Maximus5 Maximus5 changed the title [Blockstore] fix erase connection in volume proxy [Blockstore] Fix shared VolumeProxy connections Oct 21, 2026
@github-actions github-actions Bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

  • ⚫ blockstore (job skipped)

@Maximus5 Maximus5 added the recheck Add this label to relaunch checks, it will be automatically removed label Oct 21, 2026
@github-actions github-actions Bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Oct 21, 2026
@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from c06f4a0 to a1c764e Compare October 21, 2026 15:51
@Maximus5 Maximus5 added the recheck Add this label to relaunch checks, it will be automatically removed label Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

  • ⚠️ blockstore (job finished but report update failed)

@github-actions github-actions Bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Oct 21, 2026
@github-actions

github-actions Bot commented Oct 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1430s): all tests PASSED for commit a1c764e.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
7682 7681 0 0 0 1 0

@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from a1c764e to f04738e Compare October 21, 2026 18:19
@Maximus5 Maximus5 added the recheck Add this label to relaunch checks, it will be automatically removed label Oct 21, 2026
@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 264362b to 65256ca Compare October 23, 2026 17:30
@Maximus5 Maximus5 added the recheck Add this label to relaunch checks, it will be automatically removed label Oct 23, 2026
@github-actions github-actions Bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Oct 23, 2026
@github-actions

github-actions Bot commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

  • 🚫 blockstore (cancelled or timed out before reporting completion)

@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 65256ca to 1c2b0b5 Compare October 23, 2026 17:53
@Maximus5
Maximus5 removed this pull request from stack #7613 October 23, 2026 17:53
@Maximus5
Maximus5 added this pull request to stack #7615 October 23, 2026 17:54
@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 1c2b0b5 to 6c5cb37 Compare October 26, 2026 09:59
@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 6c5cb37 to eb6f25a Compare October 26, 2026 10:08
@Maximus5 Maximus5 added the recheck Add this label to relaunch checks, it will be automatically removed label Oct 26, 2026
@github-actions github-actions Bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Oct 26, 2026
@github-actions

github-actions Bot commented Oct 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1362s): all tests PASSED for commit eb6f25a.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
7689 7688 0 0 0 1 0

Base automatically changed from users/maximus5/fio-compress-canon to main October 26, 2026 11:57
@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from eb6f25a to 0222e07 Compare October 26, 2026 11:57
@github-actions

github-actions Bot commented Oct 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1418s): all tests PASSED for commit 0222e07.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
7692 7691 0 0 0 1 0

@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 0222e07 to 4d00f80 Compare October 26, 2026 19:37
@Maximus5 Maximus5 added the recheck Add this label to relaunch checks, it will be automatically removed label Oct 26, 2026
@github-actions github-actions Bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Oct 26, 2026
@github-actions

github-actions Bot commented Oct 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1457s): all tests PASSED for commit 4d00f80.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
7698 7697 0 0 0 1 0

@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 4d00f80 to 9bccefc Compare October 27, 2026 15:36
@github-actions

github-actions Bot commented Oct 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

🟢 linux-x86_64-relwithdebinfo target: cloud/blockstore/ (test time: 1501s): all tests PASSED for commit 9bccefc.

TESTS PASSED ERRORS FAILED FAILED BUILD SKIPPED MUTED?
7698 7697 0 0 0 1 0

@Maximus5

Maximus5 commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

AI Review: https://claude.ai/share/797b9730-72fd-4ca9-9dae-cdb5463166ba

comments fixed

@Maximus5
[Blockstore] Fix shared VolumeProxy connections
b9390ac 
VolumeProxy kept only one connection per Tablet. When two disk connections shared a Tablet, removing one could close
their pipe too early or leave it open after both connections were gone. A disconnect could also leave requests on the
other connection waiting.

Store all connection IDs for each Tablet and close the shared pipe only when the last connection is removed. Handle pipe
connect and disconnect events for every connection on that Tablet. Track active requests per connection so they can be
cancelled without scanning requests for other Tablets.

Tested with:
* tests for both expiry orders
* closing the pipe after the last connection expires
* cancelling requests on both connections

```
./ya make -tA cloud/blockstore/libs/storage/volume_proxy/ut -F '*TVolumeProxyTest*'
```
@Maximus5
[Blockstore] Fix idle connection cleanup after pipe disconnect
ef6643c 
**Problem**

After disconnect, an idle VolumeProxy connection could remain without a
cleanup timer and keep a shared replacement pipe open.

This happened when its timer expired during a pending request, or when
its first request was cancelled before any successful response.

**Solution**

- Schedule cleanup after disconnect when the connection has no requests
  and no cleanup timer.
- Remove idle connections after the inactivity timeout.
- Close the shared pipe when its last connection is removed.

**Testing**

Added regression tests for both scenarios. Both reproduced the leak
without the fix.

All 27 VolumeProxy tests passed with the fix.

```bash
./ya make -tA cloud/blockstore/libs/storage/volume_proxy/ut
```
@Maximus5
Maximus5 force-pushed the users/maximus5/erase-connection-fix branch from 9bccefc to ef6643c Compare October 28, 2026 13:55
@Maximus5 Maximus5 added the recheck Add this label to relaunch checks, it will be automatically removed label Oct 28, 2026
@github-actions github-actions Bot removed the recheck Add this label to relaunch checks, it will be automatically removed label Oct 28, 2026
@github-actions

github-actions Bot commented Oct 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Note

This is an automated comment that will be appended during run.

Note

All workloads for linux-x86_64-relwithdebinfo have completed.

Tip

Planned checks for linux-x86_64-relwithdebinfo.

  • ⚫ blockstore (job skipped)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qkrorlqr qkrorlqr qkrorlqr left review comments
@sharpeye sharpeye sharpeye left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

blockstore Add this label to run only cloud/blockstore build and tests on PR

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Maximus5 @qkrorlqr @sharpeye @asorotsky