Skip to content

fix: preserve WAL on link recovery failure - #3722

Open
shantanukawale-stripe wants to merge 1 commit into
AutoMQ:mainfrom
shantanukawale-stripe:fix/preserve-wal-on-recovery-failure
Open

shantanukawale-stripe wants to merge 1 commit into
AutoMQ:mainfrom
shantanukawale-stripe:fix/preserve-wal-on-recovery-failure

Conversation

@shantanukawale-stripe

Copy link
Copy Markdown
Contributor

Why

Link-record recovery currently treats a read or decode failure as the end of the recoverable WAL, discards the remaining global WAL suffix, and returns success. S3Storage.recover0() then resets the WAL. Records in that suffix can already have been acknowledged: storage append completion does not require the cached data to have been uploaded and committed to stream objects. A failure in one stream can therefore discard recoverable data from other streams.

Fixes #3721.

What

Abort recovery on link sizing or decode failure and propagate the failure to the caller. Keep the persisted WAL and opening streams available for another recovery attempt. A broker with an unresolved recovery failure stays unavailable rather than completing startup after discarding records.

How

Consume the concurrent decode futures in WAL order and propagate the first failure with its original cause. Log the failed WAL position and stream with [RECOVERY_FAILED]. Release the failed entry, remaining prefetched entries, and any decoded records still owned by the iterator as recovery unwinds.

Successful earlier batches may already have been committed; they remain valid progress. The failed batch does not advance the recovered stream offsets, and the propagated exception prevents S3Storage from reaching WAL reset or stream closure. Recovery can be retried after the underlying read problem is resolved. Persistent malformed metadata requires repair rather than automatic truncation.

The regression coverage protects cross-stream records, retry after a transient read failure, error ordering, malformed and oversized link metadata, failure after earlier batches, and buffer ownership on each failure path.

Reviewer notes

Read WALRecovery.DecodedIterator.collectDecodedRecords() with its iterator cleanup, then the recovery and retry contract in S3StorageTest. Existing truncation tests now assert failure propagation while retaining their ordering, offset, and cleanup checks.

Development assistance: OpenAI Codex (GPT-6).

Committer Checklist (excluded from commit message)

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

Propagate link sizing and decode failures before recovery can reset the WAL or close opening streams. Keep durable records available for retry while releasing pending decoded records and preserving failure ordering.

Cover recovery failure and retry across streams, and retain offset, prefetch, and buffer cleanup assertions in the existing recovery tests.
@shantanukawale-stripe

Copy link
Copy Markdown
Contributor Author

Validation completed with Java 17.

The new storage recovery regression failed against the previous production behavior because recovery returned successfully instead of propagating the link failure. With this fix, it passes and verifies that the WAL remains intact and a subsequent retry recovers records across all three streams.

Focused recovery and storage tests passed:

./gradlew --build-cache --max-workers=2 -PmaxParallelForks=1 \
  s3stream:test \
  --tests com.automq.stream.s3.WALRecoveryTest \
  --tests com.automq.stream.s3.S3StorageTest

The broader test suites and required style checks passed:

./gradlew --build-cache --max-workers=2 \
  -PmaxParallelForks=1 -PmaxScalacThreads=2 \
  rat checkstyleMain checkstyleTest spotlessJavaCheck \
  metadata:S3UnitTest core:S3UnitTest s3stream:test

RAT is disabled in the repository and was skipped by Gradle. git diff --check passed.

The three test suites reported 2,051 test cases: 1,998 passed, 53 skipped, zero failures or errors.

This branch has not been deployed

No deployments
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.

WAL recovery can discard acknowledged records after a link decode failure

1 participant