fix: broker cannot gracefully shutdown when fenced by a duplicate node

[vishalmaurya850]

Fixes #718

When a duplicate broker with the same node ID starts, it can fence the original broker by overwriting the S3 WAL reservation object or bumping the node epoch. The original broker then hangs indefinitely during shutdown because:

1. Pending WAL appends never complete (WAL is fenced), so S3Stream.close() blocks waiting for pending requests
2. closeStream RPC fails with NODE_EPOCH_EXPIRED, but the error is treated as fatal rather than terminal
3. S3StreamClient.closeStreams() loops forever waiting for streams that can never close

Changes:

• S3Stream.close0(): Make forceUpload best-effort — catch and log failures rather than blocking the close path. Handle NODE_EPOCH_EXPIRED/NODE_FENCED errors from closeStream as successful close, since the new epoch owner is responsible for stream lifecycle.
• S3StreamClient.closeStreams(): Add a configurable timeout (default 30s, via AUTOMQ_STREAM_GRACEFUL_CLOSE_TIMEOUT_MS env var). If graceful close does not complete in time, escalate to force close — force-completing pending requests and unblocking shutdown.

Testing strategy:

• Existing S3StreamClient and S3Stream unit tests continue to pass, confirming no regression in the happy-path close flow.
• The fix targets a race condition between broker shutdown and duplicate-node fencing that is inherently timing-dependent. Verification relies on integration testing: start broker A, start duplicate broker B (fails), confirm broker A shuts down within a bounded time.
• The force-close timeout is configurable via environment variable to allow tuning in different deployment environments.

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[CLAassistant]

All committers have signed the CLA.

[superhx]

@vishalmaurya850 May I ask what scenario caused the duplicate nodeId? Theoretically, nodeId should not be duplicated during deployment.

[vishalmaurya850]

@superhx Thanks for having a look on my PR

Most common cause: Kubernetes pod restart races — the new pod starts before the old one fully terminates, briefly creating two brokers with the same node ID.

Also: operator config mistakes, VM clones, or rolling upgrade timing issues.

It's an edge case, but the broker shouldn't hang forever when it happens.

[superhx]

@vishalmaurya850 Judging from the problem description, can it be shut down successfully as long as this issue 'Pending WAL appends never complete (WAL is fenced)' is resolved?

[vishalmaurya850]

@superhx Yes, largely. The main blocker is the hanging pending WAL appends. Once those complete or fail fast, StreamWrapper.close() removes the stream from tracking even if closeStream returns NODE_EPOCH_EXPIRED, so shutdown proceeds. The NODE_EPOCH_EXPIRED handling is just for cleaner logs, and the force-close timeout is the safety net to guarantee appends don't hang.

[superhx]

@superhx Yes, largely. The main blocker is the hanging pending WAL appends. Once those complete or fail fast, StreamWrapper.close() removes the stream from tracking even if closeStream returns NODE_EPOCH_EXPIRED, so shutdown proceeds. The NODE_EPOCH_EXPIRED handling is just for cleaner logs, and the force-close timeout is the safety net to guarantee appends don't hang.

Could we adjust this to fix the issue where Pending WAL appends never complete? Right now it mostly acts as a fallback measure.

[vishalmaurya850]

@superhx Let me deep dive in the code and will let you know If we can solve it.

[vishalmaurya850]

Complete request.cf exceptionally when the WAL append fails, so pending appends drain on fencing instead of blocking shutdown indefinitely.

[vishalmaurya850]

@superhx I have created the fix

[superhx]

@superhx I have created the fix

Great, after this fix, do we still need to modify the existing S3Stream and S3StreamClient?

[vishalmaurya850]

@superhx The S3Storage fix is necessary but not sufficient — the S3Stream and S3StreamClient changes remain essential for shutdown resilience.

[superhx]

@superhx The S3Storage fix is necessary but not sufficient — the S3Stream and S3StreamClient changes remain essential for shutdown resilience.

Could you please describe why these fixes are necessary?

[vishalmaurya850]

The S3Storage fix only covers the append path — it doesn't touch the close path, which has its own independent failure points:

• S3Stream: close0() calls forceUpload(streamId) and closeStream() — both can fail with fencing errors during shutdown. Without handling these, the close future fails and the stream gets stuck.

• S3StreamClient: The S3Storage fix intentionally leaves the caller's future incomplete, which means pending appends never resolve → close() waits on them forever → the infinite for(;;) loop in closeStreams() never breaks. The timeout + force-close escalation is what unblocks this.

[superhx]

The S3Storage fix only covers the append path — it doesn't touch the close path, which has its own independent failure points:

• S3Stream: close0() calls forceUpload(streamId) and closeStream() — both can fail with fencing errors during shutdown. Without handling these, the close future fails and the stream gets stuck.
• S3StreamClient: The S3Storage fix intentionally leaves the caller's future incomplete, which means pending appends never resolve → close() waits on them forever → the infinite for(;;) loop in closeStreams() never breaks. The timeout + force-close escalation is what unblocks this.

If we fix the append stuck issue, close should theoretically not get stuck.

[vishalmaurya850]

If we fix the append stuck issue, close should theoretically not get stuck.

You are right. We can remove it if you prefer; however, I believe it should remain as a fallback mechanism to ensure that shutdown is handled gracefully under all circumstances.