Issue 4855: fail closed on entry log flush failure by yangxianjungree · Pull Request #4863 · apache/bookkeeper

yangxianjungree · 2026-08-05T03:24:38Z

Descriptions of the changes in this PR:

This PR makes entry-log write/flush failures fail closed instead of allowing a bookie to continue after the entry-log write position becomes uncertain. This is the master-first fix for #4855. A branch-4.16 backport is tracked separately in #4860.

Motivation

A partial entry-log write/flush/force failure can leave the physical log file behind the logical BufferedChannel position. If the bookie continues accepting writes after that point, ledger storage may persist locations that point past the bytes actually written to the entry log, making entries unreadable after restart or recovery.

Changes

Introduce EntryLogWriteException as the fatal boundary for entry-log-level write failures.
Poison BufferedChannel after entry-log write, flush, or force-write failure so later writes cannot continue on an uncertain channel.
Harden entry-log header and ledgers-map writes with full-write checks and fatal failure propagation.
Propagate entry-log write failures through DefaultEntryLogger, EntryLogManager, SyncThread, BookieImpl, DbLedgerStorage, and SingleDirectoryDbLedgerStorage.
Make startup requestFlush() fail on fatal entry-log flush failure instead of racing with asynchronous shutdown.
Fail closed for InterleavedLedgerStorage and SortedLedgerStorage entry-log write failures while keeping NoWritableLedgerDirException behavior unchanged.
Harden the entryLogPerLedgerEnabled eviction path so appendLedgersMap() failure triggers fatal shutdown and the failed channel is not treated as a normal rotated log.
Clean up newly allocated log channels if allocation fails before publication.
Make entry-log shutdown cleanup idempotent to avoid noisy repeated-shutdown failures after fatal entry-log errors.
Add unit coverage plus a real BookKeeper client e2e that verifies client-visible write failure and bookie shutdown after an entry-log flush failure.

Scope notes:

The DirectEntryLogger / direct I/O path is not changed in this PR.
This PR does not implement a two-phase header fsync / log-id publication protocol for new entry-log allocation. Allocation failure cleanup is included; stronger allocation lifecycle handling can be addressed separately.

Fixes #4855

Tests:

Red reproduction on apache/master with the BufferedChannelTest regression patch only: 5 failures.
mvn -pl bookkeeper-server -am -Dnative.io.pure.rust=true -Dtest=BufferedChannelTest -Dsurefire.failIfNoSpecifiedTests=false test
mvn -pl bookkeeper-server -am -Dnative.io.pure.rust=true -DtestRetryCount=0 -Dtest=BufferedChannelTest,SyncThreadTest,BookieImplTest,DefaultEntryLogTest,DbLedgerStorageTest,DbLedgerStorageWriteCacheTest,DbLedgerStorageEntryLogFlushFailureE2ETest -Dsurefire.failIfNoSpecifiedTests=false test
The targeted server suite above passed 3 times in a row; logs were checked for dump files, leak reports, unexpected executor errors, and refcount errors.

The ImportOrder rule requires lexicographic ordering, so EntryLocation must precede EntryLogWriteException and BookieImpl must precede BufferedChannel. Their misplacement failed the PR Validation gate before any test could run.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

This PR changes BookKeeper’s entry-log write/flush/force failure handling to “fail closed” by introducing a fatal exception boundary (EntryLogWriteException), poisoning writer channels after partial failures, propagating fatal errors up to trigger shutdown, and adding regression + E2E coverage to ensure clients observe write failure and bookies stop.

Changes:

Introduces EntryLogWriteException and wraps/propagates entry-log write/flush/force failures as fatal.
Poisons BufferedChannel and hardens entry-log header / ledgers-map writes with full-write checks.
Propagates fatal entry-log failures through EntryLogManager*, SyncThread, BookieImpl, and Db-ledger-storage, with new unit + E2E tests.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java	Triggers bookie shutdown on fatal entry-log failures; wires Db storage fatal listener.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BufferedChannel.java	Poisons channel after write/flush/force failures; prevents further writes on uncertain state.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java	Hardens ledgers-map header writes; adds fatal listener plumbing; makes close idempotent.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManager.java	Adds fatal error listener hook to entry-log manager API.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java	Wraps IO failures as `EntryLogWriteException`; adds fatal notification + flush helpers.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java	Treats append-ledgers-map failures during eviction as fatal; routes flush via new helpers.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForSingleEntryLog.java	Routes flush/force through hardened helper methods.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogWriteException.java	New fatal exception type for uncertain entry-log writer state.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java	Cleans up newly allocated channels on allocation failures; avoids rejected-exec noise during stop.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java	Ensures fatal entry-log write exceptions propagate rather than being swallowed.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java	Forces shutdown (or read-only fallback) on fatal entry-log flush failures during skip-list flush.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java	Fails `requestFlush()` future on fatal entry-log flush failure; makes shutdown more idempotent.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java	Adds method to propagate fatal error listener to all single-dir storages.
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java	Ensures shutdown cleanup continues after flush failure; adds fatal notification on background flush failures.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java	Adds tests verifying both startup and background entry-log flush failures shut down bookie.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BufferedChannelTest.java	Adds regression tests for partial flush/force/header write failures poisoning buffered channel.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java	Adds/adjusts tests for idempotent close and ByteBuf release; adds eviction failure fatal-path coverage.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java	Adds tests for idempotent shutdown and fatal behavior on `EntryLogWriteException` during flush/checkpoint.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java	New e2e ensuring client sees write failure and bookie stops after injected flush failure.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java	Adds per-ledger eviction fatal-path test and improves ByteBuf release.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageWriteCacheTest.java	Adds regression ensuring entry-log flush failure triggers fatal listener and operation rejection.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java	New test storage wrapper to inject `EntryLogWriteException` on next flush.
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorageShutdownTest.java	New test ensuring shutdown cleanup continues after flush failure.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    final void checkWritable() throws IOException {
+        IOException failure = writeFailure;
+        if (failure != null) {
+            throw new IOException("BufferedChannel is in failed state", failure);
+        }
+    }
+
+    final void markWriteFailure(IOException e) {
+        if (writeFailure == null) {
+            writeFailure = e;
+        }
+    }


+    void notifyFatalEntryLogWriteFailure(String message, Throwable cause) {
+        log.error().exception(cause).log(message);
+        fatalErrorListener.fatalError();
+        for (LedgerDirsListener listener : ledgerDirsManager.getListeners()) {
+            if (listener != fatalErrorListener) {
+                listener.fatalError();


yangxianjungree · 2026-09-12T12:27:19Z

+            assertFalse(entryLogManager.getRotatedLogChannels().contains(logChannel),
+                    "Failed log channel should not be added to rotated logs");
+        } finally {
+            logChannel.close();


fixed by closing entryLogger

+public class FailOnFlushDbLedgerStorage extends DbLedgerStorage {
+    private static final AtomicBoolean failNextFlushWithEntryLogWriteException = new AtomicBoolean(false);
+
+    public static void injectFailureOnNextFlush() {
+        failNextFlushWithEntryLogWriteException.set(true);
+    }
+
+    public static void resetFailure() {
+        failNextFlushWithEntryLogWriteException.set(false);
+    }


+        tmpDir = File.createTempFile("bkTest", ".dir");
+        tmpDir.delete();
+        tmpDir.mkdir();


+        private static void writeFully(FileChannel fileChannel, ByteBuffer buffer, long position) throws IOException {
+            long writePosition = position;
+            while (buffer.hasRemaining()) {
+                int written = fileChannel.write(buffer, writePosition);
+                if (written <= 0) {
+                    throw new IOException("Unable to make progress while updating entry log header");
+                }
+                writePosition += written;
+            }
        }


root and others added 8 commits August 4, 2026 21:30

fix: fail closed after buffered entrylog write failure

12b1a94

fix: fail closed on entry log flush failure

0b74a31

test: add e2e coverage for entry log flush failure

0a18155

fix: close entrylog failure propagation gaps

3f42820

fix: keep fatal entrylog notifications compatible

5e3674b

fix: adapt fatal entrylog logging to master

d761317

test: release entrylog test buffers

8988984

fix: make entrylog failure shutdown cleanup idempotent

9088500

yangxianjungree changed the title ~~fix: fail closed on entry log flush failure~~ Issue 4855: fail closed on entry log flush failure Aug 5, 2026

This was referenced Aug 5, 2026

Issue 4855: fail closed on entry log flush failure #4860

Closed

Issue 4855: fail closed on entry log flush failure yangxianjungree/bookkeeper#1

Closed

yangxianjungree and others added 2 commits August 5, 2026 15:37

fix: continue db storage shutdown cleanup after flush failure

acff463

style: order new imports to satisfy checkstyle

198fa0f

The ImportOrder rule requires lexicographic ordering, so EntryLocation must precede EntryLogWriteException and BookieImpl must precede BufferedChannel. Their misplacement failed the PR Validation gate before any test could run.

StevenLuMT requested a lite review from Copilot September 10, 2026 01:51

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Copilot started reviewing on behalf of StevenLuMT September 10, 2026 02:35 View session

fix: clean up after entry log flush failure

c7f0106

Navigation Menu

Uh oh!

Issue 4855: fail closed on entry log flush failure - #4863

Issue 4855: fail closed on entry log flush failure#4863
yangxianjungree wants to merge 11 commits into
apache:masterfrom
yangxianjungree:issue-4855-master

yangxianjungree commented Aug 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

yangxianjungree Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

yangxianjungree commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

yangxianjungree Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yangxianjungree commented Aug 5, 2026 •

edited

Loading