Skip to content

fix: prevent context timeout when messages delivered successfully - #1448

Open
anakys1997-stack wants to merge 1 commit into
segmentio:mainfrom
anakys1997-stack:fix/context-timeout-race
Open

fix: prevent context timeout when messages delivered successfully#1448
anakys1997-stack wants to merge 1 commit into
segmentio:mainfrom
anakys1997-stack:fix/context-timeout-race

Conversation

@anakys1997-stack

Copy link
Copy Markdown

Description

Implement double-check pattern in Writer.WriteMessages to prevent returning context.DeadlineExceeded when messages have been successfully delivered to the broker.

Problem

The issue occurred when both ctx.Done() and batch.done channels were ready simultaneously. Go's pseudo-random select could choose ctx.Done() even when the delivery promise held a successful response, causing:

  1. WriteMessages returns context.DeadlineExceeded for messages that were actually delivered
  2. Client applications execute retry logic, leading to duplicate records

Solution

The fix adds a non-blocking check on the batch.done channel before returning ctx.Err():

go select { case <-done: // Double-check if delivery completed before or concurrently select { case <-batch.done: if batch.err != nil { hasErrors = true } default: return ctx.Err() } case <-batch.done: if batch.err != nil { hasErrors = true } }

Testing

Added two unit tests:

  • TestWriterWriteMessagesContextTimeoutWithSuccessfulDelivery: Verifies successful delivery is not lost
  • TestWriterWriteMessagesContextCanceledBeforeDelivery: Verifies proper error when batch not delivered

Fixes #2

Implement double-check pattern in Writer.WriteMessages to prevent
returning context.DeadlineExceeded when messages have been successfully
delivered to the broker.

The issue occurred when both ctx.Done() and batch.done channels were
ready simultaneously. Go's pseudo-random select could choose ctx.Done()
even when the delivery promise held a successful response.

The fix adds a non-blocking check on the batch.done channel before
returning ctx.Err(), ensuring successful deliveries are not lost.

Fixes segmentio#2
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.

1 participant