Skip to content

Drop idle persistent agent sockets after fatal failures - #4748

Open
bender6696 wants to merge 1 commit into
manticoresoftware:mainfrom
sweet-tv:fix/drop-idle-persistent-agent-sockets-upstream
Open

Drop idle persistent agent sockets after fatal failures#4748
bender6696 wants to merge 1 commit into
manticoresoftware:mainfrom
sweet-tv:fix/drop-idle-persistent-agent-sockets-upstream

Conversation

@bender6696

Copy link
Copy Markdown

Summary

  • close idle persistent agent sockets after a fatal remote agent failure
  • keep the existing failed in-flight socket close path unchanged
  • force subsequent persistent-agent requests for the same host to establish fresh connections

Why

With distributed tables that use persistent remote agents (conn=pconn), a fatal network/protocol failure can leave other idle sockets for the same remote host in the persistent pool. Those sockets may be stale or associated with the same broken remote state, and can be reused by later requests.

On a fatal failure we already close the active socket. This change also drops idle sockets in the same persistent pool so the next request does not reuse potentially poisoned connections.

Changes

  • add PersistentConnectionsPool_c::CloseIdleConnections()
  • call it from AgentConn_t::Fatal() for persistent agents before closing the failed socket

Checks

  • git diff --check upstream/main..HEAD

@sanikolaev

Copy link
Copy Markdown
Collaborator

Thanks for the PR @bender6696 . We'll review it.

@sanikolaev
sanikolaev requested a review from klirichek July 29, 2026 15:21
@klirichek

Copy link
Copy Markdown
Contributor

The logic is clear, however the reason why this change is significant is not sounds clear.
Is there a real cases where the problem exists? From explanation it sounds very like a 'may be'.
If real problem exists - can you provide a test case? May be a couple of dockers, and small scenario which illustrates the problem.
If there are not real problem but just a case of 'maybe' - I need more reasons or explanations to merge this PR.

Copy link
Copy Markdown
Author

Thanks, fair point. I agree the PR needs a concrete reproducer rather than a speculative explanation.

I'll prepare a small docker-compose scenario with a balancer + worker and a script that triggers the fatal agent path, then compare the behavior with and without the patch.

Once I have that, I'll update the PR with the reproducer and results.

@sanikolaev sanikolaev added the waiting Waiting for the original poster (in most cases) or something else label Aug 3, 2026
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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

Labels

waiting Waiting for the original poster (in most cases) or something else

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants