Skip to content

feat: surface the flush-failure cause in the retry log#26

Merged
acoshift merged 1 commit into
mainfrom
feat/log-flush-failure-cause
Jul 8, 2026
Merged

feat: surface the flush-failure cause in the retry log#26
acoshift merged 1 commit into
mainfrom
feat/log-flush-failure-cause

Conversation

@acoshift

@acoshift acoshift commented Jul 8, 2026

Copy link
Copy Markdown
Member

Problem

quickwit: flush failed, retrying indefinitely logged only an attempt counter. When an ingest stream stalls (observed live: hundreds of these/hour, attempt climbing to 20, with a multi-million-message pubsub backlog upstream), there was no way to tell from the log what actually failed — a transport error / timeout, a 5xx, or a backpressure 429/503. Worse, the two most common failure sites in flush()http.NewRequestWithContext and httpClient().Do — did return false and dropped the error entirely, so a network-level Quickwit outage produced pure attempt=N noise.

Change

Carry the most recent failure reason out of flush() into retryFlush() via a per-worker lastErr, mirroring the existing retryAfter pattern:

  • declared alongside retryAfter, reset at the top of every flush (so it only reflects the latest attempt),
  • set at each transient return false site (gzip encode, NewRequest, Do, non-2xx status, response read),
  • added as the "error" attribute on both retry log lines (retrying indefinitely and retrying while closing).

No behavior change beyond the log content. lastErr is a per-worker stack local like retryAfter, so no added cross-goroutine sharing.

Before:

INFO quickwit: flush failed, retrying indefinitely attempt=16

After (examples):

INFO quickwit: flush failed, retrying indefinitely attempt=16 error="quickwit: ingest request failed: Post \"http://…/ingest\": dial tcp …: connect: connection refused"
INFO quickwit: flush failed, retrying indefinitely attempt=3  error="quickwit: ingest status 503 Service Unavailable"

Tests

  • New TestFlushFailure_RetryLogIncludesCause: captures slog output, drives a server that returns two 500s then 200, and asserts the retry line carries an error attr naming the 500 status.
  • go vet, go test ./... (66), and go test -race ./... all green.

"quickwit: flush failed, retrying indefinitely" logged only an attempt
counter, so an operator watching an ingest stall could not tell a
transport error / timeout from a 5xx or a backpressure 429/503 — the two
silent return-false sites (http.NewRequest and httpClient.Do) dropped the
error entirely.

Carry the most recent failure out of flush() into retryFlush() via a
per-worker lastErr, mirroring the existing retryAfter pattern (reset at
the top of every flush, set at each transient return-false site), and add
it as the "error" attribute on both retry log lines. No behavior change
beyond the log content.
@acoshift
acoshift merged commit 6dc51db into main Jul 8, 2026
1 check passed
@acoshift
acoshift deleted the feat/log-flush-failure-cause branch July 8, 2026 04:13
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