Skip to content

Add log socket reconnections#69696

Open
guan404ming wants to merge 1 commit into
apache:mainfrom
guan404ming:logs-server-reaccept
Open

Add log socket reconnections#69696
guan404ming wants to merge 1 commit into
apache:mainfrom
guan404ming:logs-server-reaccept

Conversation

@guan404ming

Copy link
Copy Markdown
Member

Why

A task runtime that dropped its logs connection could never reconnect, so logs buffered during the outage were silently lost.

How

  • Register a non-blocking reaccepter on the logs server so runtimes can reconnect and flush buffered logs
  • Reject reconnections not owned by the child process tree; transient socket errors no longer stall the supervisor loop
  • Test reconnection forwarding, rejection paths, and spurious wakeups

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Fable 5)

Generated-by: Claude Code (Fable 5) following the guidelines

@boring-cyborg boring-cyborg Bot added AIP-108: Coordinator Change this to an 'area:' label after AIP acceptance. area:task-sdk labels Jul 10, 2026
@guan404ming guan404ming changed the title Keep accepting log socket reconnections so runtime logs are not lost Keep accepting log socket reconnections Jul 10, 2026
@guan404ming guan404ming force-pushed the logs-server-reaccept branch from 4372cc4 to e55d9e7 Compare July 10, 2026 09:35
@guan404ming guan404ming self-assigned this Jul 10, 2026
A runtime that drops its logs connection could never reconnect, so any
logs buffered during the outage were silently black-holed. The listener
now keeps accepting, verifies the peer belongs to the child process
tree, and tolerates transient socket errors without stalling or killing
the supervisor monitor loop.
@guan404ming guan404ming force-pushed the logs-server-reaccept branch from e55d9e7 to 52e27b7 Compare July 10, 2026 09:41
@guan404ming guan404ming marked this pull request as ready for review July 11, 2026 02:45
@guan404ming guan404ming changed the title Keep accepting log socket reconnections Add log socket reconnections Jul 11, 2026

@jason810496 jason810496 left a comment

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.

Another direction on this one. Do we really need to introduce the reconnecting for log socket at all?

I think it makes sense to introduce reconnecting/ retrying for those client-sever are on the different host machines.

Since the coordinator (Python task-SDK) and Lang-SDK (TS-SDK in this case) will always on the same worker and on the same host.

Would it be better to not reconnecting at all for both side, as the unexpected TCP connection close on Lang-SDK side means the serious errors from OS level (or SIGKILL etc) and remove the reconnecting part from TS-SDK (IIRC, Java and Go didn't implement the socket level reconnecting).

WDYT? Thanks!

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

Labels

AIP-108: Coordinator Change this to an 'area:' label after AIP acceptance. area:task-sdk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants