From b37aa0afc3153c67539b11cb99b23af42f5a46c1 Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Tue, 4 Aug 2026 15:01:12 +0800 Subject: [PATCH 1/9] fix: fail closed after buffered entrylog write failure (cherry picked from commit 200818fe18cf496d17b4083a5b4606fdc4c02c2f) --- .../bookkeeper/bookie/BufferedChannel.java | 77 +++- .../bookkeeper/bookie/DefaultEntryLogger.java | 19 +- .../bookie/BufferedChannelTest.java | 359 ++++++++++++++++++ 3 files changed, 434 insertions(+), 21 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BufferedChannel.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BufferedChannel.java index 3197165827a..2114293c3a2 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BufferedChannel.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BufferedChannel.java @@ -71,6 +71,7 @@ public class BufferedChannel extends BufferedReadChannel implements Closeable { protected final AtomicLong unpersistedBytes; private boolean closed = false; + private volatile IOException writeFailure; // make constructor to be public for unit test public BufferedChannel(ByteBufAllocator allocator, FileChannel fc, int capacity) throws IOException { @@ -118,25 +119,31 @@ public void write(ByteBuf src) throws IOException { int copied = 0; boolean shouldForceWrite = false; synchronized (this) { + checkWritable(); int len = src.readableBytes(); - while (copied < len) { - int bytesToCopy = Math.min(src.readableBytes() - copied, writeBuffer.writableBytes()); - writeBuffer.writeBytes(src, src.readerIndex() + copied, bytesToCopy); - copied += bytesToCopy; + try { + while (copied < len) { + int bytesToCopy = Math.min(src.readableBytes() - copied, writeBuffer.writableBytes()); + writeBuffer.writeBytes(src, src.readerIndex() + copied, bytesToCopy); + copied += bytesToCopy; - // if we have run out of buffer space, we should flush to the - // file - if (!writeBuffer.isWritable()) { - flush(); + // if we have run out of buffer space, we should flush to the + // file + if (!writeBuffer.isWritable()) { + flush(); + } } - } - position += copied; - if (doRegularFlushes) { - unpersistedBytes.addAndGet(copied); - if (unpersistedBytes.get() >= unpersistedBytesBound) { - flush(); - shouldForceWrite = true; + position += copied; + if (doRegularFlushes) { + unpersistedBytes.addAndGet(copied); + if (unpersistedBytes.get() >= unpersistedBytesBound) { + flush(); + shouldForceWrite = true; + } } + } catch (IOException e) { + markWriteFailure(e); + throw e; } } if (shouldForceWrite) { @@ -170,6 +177,7 @@ public long getFileChannelPosition() { * @throws IOException */ public void flushAndForceWrite(boolean forceMetadata) throws IOException { + checkWritable(); flush(); forceWrite(forceMetadata); } @@ -184,6 +192,7 @@ public void flushAndForceWrite(boolean forceMetadata) throws IOException { * @throws IOException */ public void flushAndForceWriteIfRegularFlush(boolean forceMetadata) throws IOException { + checkWritable(); if (doRegularFlushes) { flushAndForceWrite(forceMetadata); } @@ -196,10 +205,19 @@ public void flushAndForceWriteIfRegularFlush(boolean forceMetadata) throws IOExc * @throws IOException if the write fails. */ public synchronized void flush() throws IOException { + checkWritable(); ByteBuffer toWrite = writeBuffer.internalNioBuffer(0, writeBuffer.writerIndex()); - do { - fileChannel.write(toWrite); - } while (toWrite.hasRemaining()); + try { + while (toWrite.hasRemaining()) { + int written = fileChannel.write(toWrite); + if (written <= 0) { + throw new IOException("Unable to make progress while flushing buffered channel"); + } + } + } catch (IOException e) { + markWriteFailure(e); + throw e; + } writeBuffer.clear(); writeBufferStartPosition.set(fileChannel.position()); } @@ -211,6 +229,7 @@ public synchronized void flush() throws IOException { * @throws IOException */ public long forceWrite(boolean forceMetadata) throws IOException { + checkWritable(); // This is the point up to which we had flushed to the file system page cache // before issuing this force write hence is guaranteed to be made durable by // the force write, any flush that happens after this may or may @@ -237,7 +256,12 @@ public long forceWrite(boolean forceMetadata) throws IOException { } } - fileChannel.force(forceMetadata); + try { + fileChannel.force(forceMetadata); + } catch (IOException e) { + markWriteFailure(e); + throw e; + } return positionForceWrite; } @@ -295,4 +319,17 @@ public synchronized int getNumOfBytesInWriteBuffer() { long getUnpersistedBytes() { return unpersistedBytes.get(); } -} \ No newline at end of file + + 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; + } + } +} diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java index a8065411eed..831f24a344f 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java @@ -137,6 +137,7 @@ public String toString() { * Updates the entry log file header with the offset and size of the map. */ void appendLedgersMap() throws IOException { + checkWritable(); long ledgerMapOffset = this.position(); @@ -205,7 +206,23 @@ public void accept(long ledgerId, long size) { mapInfo.putLong(ledgerMapOffset); mapInfo.putInt(numberOfLedgers); mapInfo.flip(); - this.fileChannel.write(mapInfo, LEDGERS_MAP_OFFSET_POSITION); + try { + writeFully(this.fileChannel, mapInfo, LEDGERS_MAP_OFFSET_POSITION); + } catch (IOException e) { + markWriteFailure(e); + throw e; + } + } + + 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; + } } } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BufferedChannelTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BufferedChannelTest.java index cd3e34d35e3..daf2e904876 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BufferedChannelTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BufferedChannelTest.java @@ -25,8 +25,14 @@ import io.netty.buffer.Unpooled; import io.netty.buffer.UnpooledByteBufAllocator; import java.io.File; +import java.io.IOException; import java.io.RandomAccessFile; +import java.nio.ByteBuffer; +import java.nio.MappedByteBuffer; import java.nio.channels.FileChannel; +import java.nio.channels.FileLock; +import java.nio.channels.ReadableByteChannel; +import java.nio.channels.WritableByteChannel; import java.util.Random; import org.junit.Assert; import org.junit.Test; @@ -70,6 +76,191 @@ public void testBufferedChannelFlushForceWrite() throws Exception { testBufferedChannel(5000, 30, 0, true, true); } + @Test + public void testPartialFlushFailurePoisonsBufferedChannel() throws Exception { + File newLogFile = File.createTempFile("test", "log"); + newLogFile.deleteOnExit(); + FileChannel delegate = new RandomAccessFile(newLogFile, "rw").getChannel(); + PartialFailingFileChannel fileChannel = new PartialFailingFileChannel(delegate, 8); + + BufferedChannel logChannel = new BufferedChannel(UnpooledByteBufAllocator.DEFAULT, fileChannel, + 16, INTERNAL_BUFFER_READ_CAPACITY, 0); + + try { + logChannel.write(Unpooled.wrappedBuffer(new byte[16])); + Assert.fail("Expected the internal flush to fail"); + } catch (IOException expected) { + // Expected. + } + + Assert.assertEquals(0, logChannel.position()); + Assert.assertEquals(8, fileChannel.position()); + + try { + logChannel.write(Unpooled.wrappedBuffer(new byte[1])); + Assert.fail("Expected writes after a partial flush failure to fail"); + } catch (IOException expected) { + // Expected. + } + try { + logChannel.flush(); + Assert.fail("Expected flush after a partial flush failure to fail"); + } catch (IOException expected) { + // Expected. + } + try { + logChannel.forceWrite(false); + Assert.fail("Expected forceWrite after a partial flush failure to fail"); + } catch (IOException expected) { + // Expected. + } + + Assert.assertEquals(0, logChannel.position()); + Assert.assertEquals(8, fileChannel.position()); + + logChannel.close(); + } + + @Test + public void testPartialFlushFailurePreventsLedgersMapHeaderUpdate() throws Exception { + File newLogFile = File.createTempFile("test", "log"); + newLogFile.deleteOnExit(); + FileChannel delegate = new RandomAccessFile(newLogFile, "rw").getChannel(); + delegate.position(DefaultEntryLogger.LOGFILE_HEADER_SIZE); + PartialFailingFileChannel fileChannel = new PartialFailingFileChannel(delegate, 8); + + DefaultEntryLogger.BufferedLogChannel logChannel = new DefaultEntryLogger.BufferedLogChannel( + UnpooledByteBufAllocator.DEFAULT, fileChannel, 16, INTERNAL_BUFFER_READ_CAPACITY, 1L, newLogFile, 0); + + try { + logChannel.write(Unpooled.wrappedBuffer(new byte[16])); + Assert.fail("Expected the internal flush to fail"); + } catch (IOException expected) { + // Expected. + } + + logChannel.registerWrittenEntry(1234L, 99L); + try { + logChannel.appendLedgersMap(); + Assert.fail("Expected appendLedgersMap after a partial flush failure to fail"); + } catch (IOException expected) { + // Expected. + } + + ByteBuffer mapInfo = ByteBuffer.allocate(Long.BYTES + Integer.BYTES); + Assert.assertEquals(mapInfo.capacity(), + fileChannel.read(mapInfo, DefaultEntryLogger.LEDGERS_MAP_OFFSET_POSITION)); + mapInfo.flip(); + Assert.assertEquals(0L, mapInfo.getLong()); + Assert.assertEquals(0, mapInfo.getInt()); + + logChannel.close(); + } + + @Test + public void testForceWriteFailurePoisonsBufferedChannel() throws Exception { + File newLogFile = File.createTempFile("test", "log"); + newLogFile.deleteOnExit(); + FileChannel delegate = new RandomAccessFile(newLogFile, "rw").getChannel(); + PartialFailingFileChannel fileChannel = PartialFailingFileChannel.failingForce(delegate); + + BufferedChannel logChannel = new BufferedChannel(UnpooledByteBufAllocator.DEFAULT, fileChannel, + 16, INTERNAL_BUFFER_READ_CAPACITY, 0); + + logChannel.write(Unpooled.wrappedBuffer(new byte[] { 1 })); + logChannel.flush(); + + try { + logChannel.forceWrite(false); + Assert.fail("Expected forceWrite failure"); + } catch (IOException expected) { + // Expected. + } + try { + logChannel.write(Unpooled.wrappedBuffer(new byte[] { 2 })); + Assert.fail("Expected writes after forceWrite failure to fail"); + } catch (IOException expected) { + // Expected. + } + + Assert.assertEquals(1, logChannel.position()); + + logChannel.close(); + } + + @Test + public void testLedgersMapHeaderPositionedWriteIsFullyWritten() throws Exception { + File newLogFile = File.createTempFile("test", "log"); + newLogFile.deleteOnExit(); + FileChannel delegate = new RandomAccessFile(newLogFile, "rw").getChannel(); + writeEntryLogHeader(delegate); + PartialFailingFileChannel fileChannel = PartialFailingFileChannel.shortPositionedWrite( + delegate, DefaultEntryLogger.LEDGERS_MAP_OFFSET_POSITION, Long.BYTES); + + DefaultEntryLogger.BufferedLogChannel logChannel = new DefaultEntryLogger.BufferedLogChannel( + UnpooledByteBufAllocator.DEFAULT, fileChannel, 64, INTERNAL_BUFFER_READ_CAPACITY, 1L, newLogFile, 0); + + logChannel.write(Unpooled.wrappedBuffer(new byte[] { 1 })); + logChannel.flush(); + + long actualMapOffset = logChannel.position(); + logChannel.registerWrittenEntry(1234L, 1L); + logChannel.appendLedgersMap(); + + ByteBuffer mapInfo = ByteBuffer.allocate(Long.BYTES + Integer.BYTES); + Assert.assertEquals(mapInfo.capacity(), + fileChannel.read(mapInfo, DefaultEntryLogger.LEDGERS_MAP_OFFSET_POSITION)); + mapInfo.flip(); + Assert.assertEquals(actualMapOffset, mapInfo.getLong()); + Assert.assertEquals(1, mapInfo.getInt()); + Assert.assertTrue(fileChannel.shortPositionedWriteDone); + + logChannel.close(); + } + + @Test + public void testLedgersMapHeaderWriteFailurePoisonsBufferedChannel() throws Exception { + File newLogFile = File.createTempFile("test", "log"); + newLogFile.deleteOnExit(); + FileChannel delegate = new RandomAccessFile(newLogFile, "rw").getChannel(); + writeEntryLogHeader(delegate); + PartialFailingFileChannel fileChannel = PartialFailingFileChannel.failingPositionedWrite( + delegate, DefaultEntryLogger.LEDGERS_MAP_OFFSET_POSITION); + + DefaultEntryLogger.BufferedLogChannel logChannel = new DefaultEntryLogger.BufferedLogChannel( + UnpooledByteBufAllocator.DEFAULT, fileChannel, 64, INTERNAL_BUFFER_READ_CAPACITY, 1L, newLogFile, 0); + + logChannel.write(Unpooled.wrappedBuffer(new byte[] { 1 })); + logChannel.flush(); + long mapOffset = logChannel.position(); + logChannel.registerWrittenEntry(1234L, 1L); + + try { + logChannel.appendLedgersMap(); + Assert.fail("Expected header write failure"); + } catch (IOException expected) { + // Expected. + } + try { + logChannel.write(Unpooled.wrappedBuffer(new byte[] { 2 })); + Assert.fail("Expected writes after header write failure to fail"); + } catch (IOException expected) { + // Expected. + } + + ByteBuffer mapInfo = ByteBuffer.allocate(Long.BYTES + Integer.BYTES); + Assert.assertEquals(mapInfo.capacity(), + fileChannel.read(mapInfo, DefaultEntryLogger.LEDGERS_MAP_OFFSET_POSITION)); + mapInfo.flip(); + Assert.assertEquals(0L, mapInfo.getLong()); + Assert.assertEquals(0, mapInfo.getInt()); + + Assert.assertEquals(mapOffset + DefaultEntryLogger.LEDGERS_MAP_HEADER_SIZE + + DefaultEntryLogger.LEDGERS_MAP_ENTRY_SIZE, logChannel.position()); + + logChannel.close(); + } + public void testBufferedChannel(int byteBufLength, int numOfWrites, int unpersistedBytesBound, boolean flush, boolean shouldForceWrite) throws Exception { File newLogFile = File.createTempFile("test", "log"); @@ -133,4 +324,172 @@ private static ByteBuf generateEntry(int length) { bb.writeBytes(data); return bb; } + + private static void writeEntryLogHeader(FileChannel fileChannel) throws IOException { + ByteBuffer header = ByteBuffer.allocate(DefaultEntryLogger.LOGFILE_HEADER_SIZE); + header.put("BKLO".getBytes("UTF-8")); + header.putInt(DefaultEntryLogger.HEADER_CURRENT_VERSION); + header.position(DefaultEntryLogger.LOGFILE_HEADER_SIZE); + header.flip(); + while (header.hasRemaining()) { + fileChannel.write(header); + } + fileChannel.position(DefaultEntryLogger.LOGFILE_HEADER_SIZE); + } + + private static final class PartialFailingFileChannel extends FileChannel { + private final FileChannel delegate; + private final int bytesBeforeFailure; + private final boolean failRegularWrite; + private final long shortPositionedWritePosition; + private final int shortPositionedWriteBytes; + private final long failingPositionedWritePosition; + private final boolean failForce; + private boolean partialWriteDone; + private boolean failureInjected; + private boolean shortPositionedWriteDone; + private boolean positionedWriteFailureInjected; + + private PartialFailingFileChannel(FileChannel delegate, int bytesBeforeFailure) { + this(delegate, bytesBeforeFailure, true, -1L, -1, -1L, false); + } + + private PartialFailingFileChannel(FileChannel delegate, int bytesBeforeFailure, + boolean failRegularWrite, long shortPositionedWritePosition, + int shortPositionedWriteBytes, long failingPositionedWritePosition, + boolean failForce) { + this.delegate = delegate; + this.bytesBeforeFailure = bytesBeforeFailure; + this.failRegularWrite = failRegularWrite; + this.shortPositionedWritePosition = shortPositionedWritePosition; + this.shortPositionedWriteBytes = shortPositionedWriteBytes; + this.failingPositionedWritePosition = failingPositionedWritePosition; + this.failForce = failForce; + } + + private static PartialFailingFileChannel shortPositionedWrite(FileChannel delegate, long position, int bytes) { + return new PartialFailingFileChannel(delegate, 0, false, position, bytes, -1L, false); + } + + private static PartialFailingFileChannel failingPositionedWrite(FileChannel delegate, long position) { + return new PartialFailingFileChannel(delegate, 0, false, -1L, -1, position, false); + } + + private static PartialFailingFileChannel failingForce(FileChannel delegate) { + return new PartialFailingFileChannel(delegate, 0, false, -1L, -1, -1L, true); + } + + @Override + public int write(ByteBuffer src) throws IOException { + if (failRegularWrite && !partialWriteDone) { + int oldLimit = src.limit(); + src.limit(src.position() + bytesBeforeFailure); + int written = delegate.write(src); + src.limit(oldLimit); + partialWriteDone = true; + return written; + } else if (failRegularWrite && !failureInjected) { + failureInjected = true; + throw new IOException("simulated write failure after partial write"); + } + return delegate.write(src); + } + + @Override + public long position() throws IOException { + return delegate.position(); + } + + @Override + public FileChannel position(long newPosition) throws IOException { + delegate.position(newPosition); + return this; + } + + @Override + public int read(ByteBuffer dst) throws IOException { + return delegate.read(dst); + } + + @Override + public long read(ByteBuffer[] dsts, int offset, int length) throws IOException { + return delegate.read(dsts, offset, length); + } + + @Override + public long write(ByteBuffer[] srcs, int offset, int length) throws IOException { + return delegate.write(srcs, offset, length); + } + + @Override + public long size() throws IOException { + return delegate.size(); + } + + @Override + public FileChannel truncate(long size) throws IOException { + delegate.truncate(size); + return this; + } + + @Override + public void force(boolean metaData) throws IOException { + if (failForce) { + throw new IOException("simulated force failure"); + } + delegate.force(metaData); + } + + @Override + public long transferTo(long position, long count, WritableByteChannel target) throws IOException { + return delegate.transferTo(position, count, target); + } + + @Override + public long transferFrom(ReadableByteChannel src, long position, long count) throws IOException { + return delegate.transferFrom(src, position, count); + } + + @Override + public int read(ByteBuffer dst, long position) throws IOException { + return delegate.read(dst, position); + } + + @Override + public int write(ByteBuffer src, long position) throws IOException { + if (!positionedWriteFailureInjected && position == failingPositionedWritePosition) { + positionedWriteFailureInjected = true; + throw new IOException("simulated positioned write failure"); + } + if (!shortPositionedWriteDone && position == shortPositionedWritePosition) { + int oldLimit = src.limit(); + src.limit(src.position() + Math.min(shortPositionedWriteBytes, src.remaining())); + int written = delegate.write(src, position); + src.limit(oldLimit); + shortPositionedWriteDone = true; + return written; + } + return delegate.write(src, position); + } + + @Override + public MappedByteBuffer map(MapMode mode, long position, long size) throws IOException { + return delegate.map(mode, position, size); + } + + @Override + public FileLock lock(long position, long size, boolean shared) throws IOException { + return delegate.lock(position, size, shared); + } + + @Override + public FileLock tryLock(long position, long size, boolean shared) throws IOException { + return delegate.tryLock(position, size, shared); + } + + @Override + protected void implCloseChannel() throws IOException { + delegate.close(); + } + } } From 9bd0858f3c8c71f9bcd419ffbb35254d1fc8daa8 Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Tue, 4 Aug 2026 15:01:12 +0800 Subject: [PATCH 2/9] fix: fail closed on entry log flush failure (cherry picked from commit 17042aa9f50ae62e87036a57c14e39dd1d279cbb) --- .../apache/bookkeeper/bookie/BookieImpl.java | 9 ++ .../bookie/EntryLogManagerBase.java | 80 +++++++++++--- .../EntryLogManagerForEntryLogPerLedger.java | 6 +- .../EntryLogManagerForSingleEntryLog.java | 4 +- .../bookie/EntryLogWriteException.java | 35 ++++++ .../bookie/EntryLoggerAllocator.java | 32 ++++-- .../bookie/InterleavedLedgerStorage.java | 2 + .../bookie/SortedLedgerStorage.java | 14 +++ .../apache/bookkeeper/bookie/SyncThread.java | 8 ++ .../bookie/storage/ldb/DbLedgerStorage.java | 7 ++ .../ldb/SingleDirectoryDbLedgerStorage.java | 15 +++ .../bookkeeper/bookie/BookieImplTest.java | 101 ++++++++++++++++++ .../bookkeeper/bookie/SyncThreadTest.java | 55 ++++++++++ .../ldb/DbLedgerStorageWriteCacheTest.java | 91 ++++++++++++---- 14 files changed, 406 insertions(+), 53 deletions(-) create mode 100644 bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogWriteException.java diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java index 968977ecdc3..13a8d58f957 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieImpl.java @@ -489,6 +489,9 @@ public void ledgerDeleted(long ledgerId) { ledgerStorage.setStateManager(stateManager); ledgerStorage.setCheckpointSource(checkpointSource); ledgerStorage.setCheckpointer(syncThread); + if (isDbLedgerStorage) { + ((DbLedgerStorage) ledgerStorage).setFatalErrorListener(getLedgerDirsListener()); + } ledgerStorage.registerLedgerDeletionListener(ledgerDeletionListener); handles = new HandleFactoryImpl(ledgerStorage); @@ -979,6 +982,9 @@ public void recoveryAddEntry(ByteBuf entry, WriteCallback cb, Object ctx, byte[] addEntryInternal(handle, entry, false /* ackBeforeSync */, cb, ctx, masterKey); } success = true; + } catch (EntryLogWriteException e) { + triggerBookieShutdown(ExitCode.BOOKIE_EXCEPTION); + throw e; } catch (NoWritableLedgerDirException e) { stateManager.transitionToReadOnlyMode(); throw new IOException(e); @@ -1073,6 +1079,9 @@ public void addEntry(ByteBuf entry, boolean ackBeforeSync, WriteCallback cb, Obj addEntryInternal(handle, entry, ackBeforeSync, cb, ctx, masterKey); } success = true; + } catch (EntryLogWriteException e) { + triggerBookieShutdown(ExitCode.BOOKIE_EXCEPTION); + throw e; } catch (NoWritableLedgerDirException e) { stateManager.transitionToReadOnlyMode(); throw new IOException(e); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java index 36ce928a089..b8312bf4503 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java @@ -73,13 +73,20 @@ public long addEntry(long ledger, ByteBuf entry, boolean rollLog) throws IOExcep ByteBuf sizeBuffer = sizeBufferForAdd.get(); sizeBuffer.clear(); sizeBuffer.writeInt(entry.readableBytes()); - logChannel.write(sizeBuffer); - - long pos = logChannel.position(); - logChannel.write(entry); - logChannel.registerWrittenEntry(ledger, entrySize); - - return (logChannel.getLogId() << 32L) | pos; + try { + logChannel.write(sizeBuffer); + + long pos = logChannel.position(); + logChannel.write(entry); + logChannel.registerWrittenEntry(ledger, entrySize); + + return (logChannel.getLogId() << 32L) | pos; + } catch (EntryLogWriteException e) { + throw e; + } catch (IOException e) { + throw new EntryLogWriteException( + "Failed to write entry to entry log " + logChannel.getLogId() + " for ledger " + ledger, e); + } } boolean reachEntryLogLimit(BufferedLogChannel logChannel, long size) { @@ -125,13 +132,33 @@ public void flush() throws IOException { void flushLogChannel(BufferedLogChannel logChannel, boolean forceMetadata) throws IOException { if (logChannel != null) { - logChannel.flushAndForceWrite(forceMetadata); + flushAndForceWrite(logChannel, forceMetadata); if (log.isDebugEnabled()) { log.debug("Flush and sync current entry logger {}", logChannel.getLogId()); } } } + void flushAndForceWrite(BufferedLogChannel logChannel, boolean forceMetadata) throws IOException { + try { + logChannel.flushAndForceWrite(forceMetadata); + } catch (EntryLogWriteException e) { + throw e; + } catch (IOException e) { + throw new EntryLogWriteException("Failed to flush entry log " + logChannel.getLogId(), e); + } + } + + void flushAndForceWriteIfRegularFlush(BufferedLogChannel logChannel, boolean forceMetadata) throws IOException { + try { + logChannel.flushAndForceWriteIfRegularFlush(forceMetadata); + } catch (EntryLogWriteException e) { + throw e; + } catch (IOException e) { + throw new EntryLogWriteException("Failed to flush entry log " + logChannel.getLogId(), e); + } + } + /* * Creates a new log file. This method should be guarded by a lock, * so callers of this method should be in right scope of the lock. @@ -155,12 +182,27 @@ void createNewLog(long ledgerId, String reason) throws IOException { if (null != logChannel) { // flush the internal buffer back to filesystem but not sync disk - logChannel.flush(); - - // Append ledgers map at the end of entry log - logChannel.appendLedgersMap(); + try { + logChannel.flush(); + + // Append ledgers map at the end of entry log + logChannel.appendLedgersMap(); + } catch (EntryLogWriteException e) { + throw e; + } catch (IOException e) { + throw new EntryLogWriteException( + "Failed to rotate entry log " + logChannel.getLogId() + " for ledger " + ledgerId, e); + } - BufferedLogChannel newLogChannel = entryLoggerAllocator.createNewLog(selectDirForNextEntryLog()); + File dirForNextEntryLog = selectDirForNextEntryLog(); + BufferedLogChannel newLogChannel; + try { + newLogChannel = entryLoggerAllocator.createNewLog(dirForNextEntryLog); + } catch (EntryLogWriteException e) { + throw e; + } catch (IOException e) { + throw new EntryLogWriteException("Failed to create a new entry log for ledger " + ledgerId, e); + } setCurrentLogForLedgerAndAddToRotate(ledgerId, newLogChannel); log.info("Flushing entry logger {} back to filesystem, pending for syncing entry loggers : {}.", logChannel.getLogId(), rotatedLogChannels); @@ -168,8 +210,16 @@ void createNewLog(long ledgerId, String reason) throws IOException { listener.onRotateEntryLog(); } } else { - setCurrentLogForLedgerAndAddToRotate(ledgerId, - entryLoggerAllocator.createNewLog(selectDirForNextEntryLog())); + File dirForNextEntryLog = selectDirForNextEntryLog(); + BufferedLogChannel newLogChannel; + try { + newLogChannel = entryLoggerAllocator.createNewLog(dirForNextEntryLog); + } catch (EntryLogWriteException e) { + throw e; + } catch (IOException e) { + throw new EntryLogWriteException("Failed to create a new entry log for ledger " + ledgerId, e); + } + setCurrentLogForLedgerAndAddToRotate(ledgerId, newLogChannel); } } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java index 60da9e60335..c71a54ace25 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java @@ -664,10 +664,10 @@ BufferedLogChannel getCurrentLogForLedgerForAddEntry(long ledgerId, int entrySiz * logChannel, since Bookie must have turned to readonly mode and * the addEntry traffic would be from GC and it is ok to proceed in * this case. - */ + */ if ((diskFull && (!allDisksFull)) || reachEntryLogLimit || (logChannel == null)) { if (logChannel != null) { - logChannel.flushAndForceWriteIfRegularFlush(false); + flushAndForceWriteIfRegularFlush(logChannel, false); } createNewLog(ledgerId, ": diskFull = " + diskFull + ", allDisksFull = " + allDisksFull @@ -683,7 +683,7 @@ BufferedLogChannel getCurrentLogForLedgerForAddEntry(long ledgerId, int entrySiz @Override public void flushRotatedLogs() throws IOException { for (BufferedLogChannel channel : rotatedLogChannels) { - channel.flushAndForceWrite(true); + flushAndForceWrite(channel, true); // since this channel is only used for writing, after flushing the channel, // we had to close the underlying file channel. Otherwise, we might end up // leaking fds which cause the disk spaces could not be reclaimed. diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForSingleEntryLog.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForSingleEntryLog.java index 59bcc02a577..364b6c73fac 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForSingleEntryLog.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForSingleEntryLog.java @@ -102,7 +102,7 @@ synchronized BufferedLogChannel getCurrentLogForLedgerForAddEntry(long ledgerId, boolean createNewLog = shouldCreateNewEntryLog.get(); if (createNewLog || reachEntryLogLimit) { if (activeLogChannel != null) { - activeLogChannel.flushAndForceWriteIfRegularFlush(false); + flushAndForceWriteIfRegularFlush(activeLogChannel, false); } createNewLog(UNASSIGNED_LEDGERID, ": createNewLog = " + createNewLog + ", reachEntryLogLimit = " + reachEntryLogLimit); @@ -188,7 +188,7 @@ void flushRotatedLogs() throws IOException { while (chIter.hasNext()) { BufferedLogChannel channel = chIter.next(); try { - channel.flushAndForceWrite(true); + flushAndForceWrite(channel, true); } catch (IOException ioe) { // rescue from flush exception, add unflushed channels back synchronized (this) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogWriteException.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogWriteException.java new file mode 100644 index 00000000000..11d5ad32d14 --- /dev/null +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogWriteException.java @@ -0,0 +1,35 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ + +package org.apache.bookkeeper.bookie; + +import java.io.IOException; + +/** + * Indicates an entry log write, flush, or fsync failure that leaves the writer state uncertain. + */ +public class EntryLogWriteException extends IOException { + private static final long serialVersionUID = -6456783806451195011L; + + public EntryLogWriteException(String message, IOException cause) { + super(message, cause); + } +} diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java index e9ff5030d18..832539dcc29 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java @@ -168,19 +168,31 @@ private synchronized BufferedLogChannel allocateNewLog(File dirForNextEntryLog, BufferedLogChannel logChannel = new BufferedLogChannel(byteBufAllocator, channel, conf.getWriteBufferBytes(), conf.getReadBufferBytes(), preallocatedLogId, newLogFile, conf.getFlushIntervalInBytes()); - logfileHeader.readerIndex(0); - logChannel.write(logfileHeader); + boolean success = false; + try { + logfileHeader.readerIndex(0); + logChannel.write(logfileHeader); - for (File f : ledgersDirs) { - setLastLogId(f, preallocatedLogId); - } + for (File f : ledgersDirs) { + setLastLogId(f, preallocatedLogId); + } - if (suffix.equals(DefaultEntryLogger.LOG_FILE_SUFFIX)) { - recentlyCreatedEntryLogsStatus.createdEntryLog(preallocatedLogId); - } + if (suffix.equals(DefaultEntryLogger.LOG_FILE_SUFFIX)) { + recentlyCreatedEntryLogsStatus.createdEntryLog(preallocatedLogId); + } - log.info("Created new entry log file {} for logId {}.", newLogFile, preallocatedLogId); - return logChannel; + log.info("Created new entry log file {} for logId {}.", newLogFile, preallocatedLogId); + success = true; + return logChannel; + } finally { + if (!success) { + try { + logChannel.close(); + } catch (IOException closeError) { + log.warn("Failed to close entry log file {} after allocation failure", newLogFile, closeError); + } + } + } } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java index 2b90e3b0081..cf5e793aaa2 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/InterleavedLedgerStorage.java @@ -467,6 +467,8 @@ private void flushOrCheckpoint(boolean isCheckpointFlush) } else { entryLogger.flush(); } + } catch (EntryLogWriteException e) { + throw e; } catch (LedgerDirsManager.NoWritableLedgerDirException e) { throw e; } catch (IOException ioe) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java index c3c9e972138..bca51044ba3 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SortedLedgerStorage.java @@ -317,6 +317,8 @@ public void run() { if (interleavedLedgerStorage.getEntryLogger().commitEntryMemTableFlush()) { interleavedLedgerStorage.checkpointer.startCheckpoint(cp); } + } catch (EntryLogWriteException e) { + fatalEntryLogWriteFailure(e); } catch (Exception e) { stateManager.transitionToReadOnlyMode(); LOG.error("Exception thrown while flushing skip list cache.", e); @@ -333,6 +335,18 @@ public void onRotateEntryLog() { // flushed to the entry log file. } + private void fatalEntryLogWriteFailure(EntryLogWriteException e) { + LOG.error("Fatal entry log write failure while flushing skip list cache.", e); + if (stateManager instanceof BookieStateManager) { + StateManager.ShutdownHandler shutdownHandler = ((BookieStateManager) stateManager).getShutdownHandler(); + if (shutdownHandler != null) { + shutdownHandler.shutdown(ExitCode.BOOKIE_EXCEPTION); + return; + } + } + stateManager.transitionToReadOnlyMode(); + } + BookieStateManager getStateManager(){ return (BookieStateManager) stateManager; } diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java index 7c5ad7c4991..4a8a205128c 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java @@ -146,6 +146,10 @@ private void flush() { log.error("No writeable ledger directories", e); dirsListener.allDisksFull(true); return; + } catch (EntryLogWriteException e) { + log.error("Fatal entry log write failure while flushing ledgers", e); + dirsListener.fatalError(); + return; } catch (IOException e) { log.error("Exception flushing ledgers", e); return; @@ -177,6 +181,10 @@ public void checkpoint(Checkpoint checkpoint) { log.error("No writeable ledger directories", e); dirsListener.allDisksFull(true); return; + } catch (EntryLogWriteException e) { + log.error("Fatal entry log write failure while checkpointing ledgers", e); + dirsListener.fatalError(); + return; } catch (IOException e) { log.error("Exception flushing ledgers", e); return; diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java index 60f752e2264..d631e59471d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java @@ -52,6 +52,7 @@ import org.apache.bookkeeper.bookie.LastAddConfirmedUpdateNotification; import org.apache.bookkeeper.bookie.LedgerCache; import org.apache.bookkeeper.bookie.LedgerDirsManager; +import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener; import org.apache.bookkeeper.bookie.LedgerStorage; import org.apache.bookkeeper.bookie.StateManager; import org.apache.bookkeeper.bookie.storage.EntryLogger; @@ -301,6 +302,12 @@ public void setCheckpointer(Checkpointer checkpointer) { ledgerStorageList.forEach(s -> s.setCheckpointer(checkpointer)); } + public void setFatalErrorListener(LedgerDirsListener fatalErrorListener) { + if (fatalErrorListener != null) { + ledgerStorageList.forEach(s -> s.setFatalErrorListener(fatalErrorListener)); + } + } + @Override public void start() { ledgerStorageList.forEach(LedgerStorage::start); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java index 867ba905ffe..7ec0e309368 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java @@ -56,6 +56,7 @@ import org.apache.bookkeeper.bookie.CheckpointSource.Checkpoint; import org.apache.bookkeeper.bookie.Checkpointer; import org.apache.bookkeeper.bookie.CompactableLedgerStorage; +import org.apache.bookkeeper.bookie.EntryLogWriteException; import org.apache.bookkeeper.bookie.EntryLocation; import org.apache.bookkeeper.bookie.GarbageCollectionStatus; import org.apache.bookkeeper.bookie.GarbageCollectorThread; @@ -133,6 +134,7 @@ protected Thread newThread(Runnable r, String name) { private CheckpointSource checkpointSource = CheckpointSource.DEFAULT; private Checkpoint lastCheckpoint = Checkpoint.MIN; + private volatile LedgerDirsListener fatalErrorListener = new LedgerDirsListener() { }; private final long writeCacheMaxSize; private final long readCacheMaxSize; @@ -251,6 +253,12 @@ public void setCheckpointSource(CheckpointSource checkpointSource) { @Override public void setCheckpointer(Checkpointer checkpointer) { } + void setFatalErrorListener(LedgerDirsListener fatalErrorListener) { + if (fatalErrorListener != null) { + this.fatalErrorListener = fatalErrorListener; + } + } + /** * Evict all the ledger info object that were not used recently. */ @@ -527,6 +535,8 @@ private void triggerFlushAndAddEntry(long ledgerId, long entryId, ByteBuf entry) long startTime = System.nanoTime(); try { flush(); + } catch (EntryLogWriteException e) { + notifyFatalEntryLogWriteFailure(e); } catch (IOException e) { log.error("Error during flush", e); } finally { @@ -561,6 +571,11 @@ private void triggerFlushAndAddEntry(long ledgerId, long entryId, ByteBuf entry) throw new OperationRejectedException(); } + private void notifyFatalEntryLogWriteFailure(EntryLogWriteException e) { + log.error("Fatal entry log write failure during background flush", e); + fatalErrorListener.fatalError(); + } + @Override public ByteBuf getEntry(long ledgerId, long entryId) throws IOException, BookieException { long startTime = MathUtils.nowInNano(); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java index 4787ae8d36f..562ceb94d8f 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java @@ -29,19 +29,28 @@ import com.google.protobuf.ByteString; import com.google.protobuf.UnsafeByteOperations; import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufAllocator; import io.netty.buffer.Unpooled; import io.netty.buffer.UnpooledByteBufAllocator; +import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.bookkeeper.bookie.storage.EntryLogger; +import org.apache.bookkeeper.bookie.storage.ldb.DbLedgerStorage; +import org.apache.bookkeeper.bookie.storage.ldb.SingleDirectoryDbLedgerStorage; import org.apache.bookkeeper.client.BookKeeper; import org.apache.bookkeeper.conf.ServerConfiguration; import org.apache.bookkeeper.conf.TestBKConfiguration; import org.apache.bookkeeper.discover.RegistrationManager; +import org.apache.bookkeeper.meta.LedgerManager; import org.apache.bookkeeper.meta.MetadataBookieDriver; import org.apache.bookkeeper.net.BookieId; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks; import org.apache.bookkeeper.proto.checksum.DigestManager; import org.apache.bookkeeper.stats.NullStatsLogger; +import org.apache.bookkeeper.stats.StatsLogger; import org.apache.bookkeeper.test.BookKeeperClusterTestCase; import org.apache.bookkeeper.util.ByteBufList; import org.apache.bookkeeper.util.PortManager; @@ -128,6 +137,43 @@ public void testRecoveryAddEntry() throws Exception { mockAddEntryReleased(RECOVERY_ADD); } + @Test + public void testBackgroundEntryLogFlushFailureShutsDownBookie() throws Exception { + ServerConfiguration conf = newServerConfiguration(); + conf.setLedgerStorageClass(FailOnFlushDbLedgerStorage.class.getName()); + conf.setJournalWriteData(false); + conf.setProperty(DbLedgerStorage.WRITE_CACHE_MAX_SIZE_MB, 1); + conf.setProperty("dbStorage_maxThrottleTimeMs", 1000); + FailOnFlushDbLedgerStorage.failNextFlushWithEntryLogWriteException.set(false); + + CountDownLatch shutdownLatch = new CountDownLatch(1); + TestBookieImpl.Resources resources = new TestBookieImpl.ResourceBuilder(conf).build(); + BookieImpl bookie = new TestBookieImpl(resources) { + @Override + int shutdown(int exitCode) { + int result = super.shutdown(exitCode); + if (exitCode == ExitCode.BOOKIE_EXCEPTION) { + shutdownLatch.countDown(); + } + return result; + } + }; + + try { + bookie.start(); + FailOnFlushDbLedgerStorage.failNextFlushWithEntryLogWriteException.set(true); + writeUntilCacheFlushIsTriggered(bookie, 10L, "masterKey".getBytes(StandardCharsets.UTF_8)); + + assertTrue("Entry log flush failure should shut down the bookie", + shutdownLatch.await(10, TimeUnit.SECONDS)); + } finally { + FailOnFlushDbLedgerStorage.failNextFlushWithEntryLogWriteException.set(false); + if (bookie.isRunning()) { + bookie.shutdown(); + } + } + } + public void mockAddEntryReleased(int flag) throws Exception { final String metadataServiceUri = zkUtil.getMetadataServiceUri(); ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); @@ -192,4 +238,59 @@ private ByteBuf generateEntry(long ledger, long entry) { bb.writeBytes(data); return bb; } + + private void writeUntilCacheFlushIsTriggered(BookieImpl bookie, long ledgerId, byte[] masterKey) + throws Exception { + for (int i = 0; i < 20; i++) { + try { + bookie.addEntry(generateLargeEntry(ledgerId, i), false, + (rc, lid, eid, addr, ctx) -> { }, null, masterKey); + } catch (BookieException.OperationRejectedException e) { + return; + } + } + } + + private ByteBuf generateLargeEntry(long ledger, long entry) { + ByteBuf bb = Unpooled.buffer(100 * 1024 + 3 * Long.BYTES); + bb.writeLong(ledger); + bb.writeLong(entry); + bb.writeLong(entry - 1); + bb.writeZero(100 * 1024); + return bb; + } + + public static class FailOnFlushDbLedgerStorage extends DbLedgerStorage { + private static final AtomicBoolean failNextFlushWithEntryLogWriteException = new AtomicBoolean(false); + + @Override + protected SingleDirectoryDbLedgerStorage newSingleDirectoryDbLedgerStorage(ServerConfiguration conf, + LedgerManager ledgerManager, LedgerDirsManager ledgerDirsManager, LedgerDirsManager indexDirsManager, + EntryLogger entryLogger, StatsLogger statsLogger, long writeCacheSize, long readCacheSize, + int readAheadCacheBatchSize, long readAheadCacheBatchBytesSize) + throws IOException { + return new FailOnFlushSingleDirectoryDbLedgerStorage(conf, ledgerManager, ledgerDirsManager, + indexDirsManager, entryLogger, statsLogger, allocator, writeCacheSize, readCacheSize, + readAheadCacheBatchSize, readAheadCacheBatchBytesSize); + } + } + + private static class FailOnFlushSingleDirectoryDbLedgerStorage extends SingleDirectoryDbLedgerStorage { + FailOnFlushSingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager ledgerManager, + LedgerDirsManager ledgerDirsManager, LedgerDirsManager indexDirsManager, EntryLogger entryLogger, + StatsLogger statsLogger, ByteBufAllocator allocator, long writeCacheSize, long readCacheSize, + int readAheadCacheBatchSize, long readAheadCacheBatchBytesSize) + throws IOException { + super(conf, ledgerManager, ledgerDirsManager, indexDirsManager, entryLogger, statsLogger, allocator, + writeCacheSize, readCacheSize, readAheadCacheBatchSize, readAheadCacheBatchBytesSize); + } + + @Override + public void flush() throws IOException { + if (FailOnFlushDbLedgerStorage.failNextFlushWithEntryLogWriteException.compareAndSet(true, false)) { + throw new EntryLogWriteException("entry log flush failed", new IOException("injected")); + } + super.flush(); + } + } } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java index 6df1bacb80d..c5548d112b2 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java @@ -223,6 +223,61 @@ public void checkpoint(Checkpoint checkpoint) t.shutdown(); } + @Test + public void testSyncThreadShutdownOnEntryLogWriteFailureDuringCheckpoint() throws Exception { + int flushInterval = 100; + ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); + conf.setFlushInterval(flushInterval); + CheckpointSource checkpointSource = new DummyCheckpointSource(); + final CountDownLatch fatalLatch = new CountDownLatch(1); + LedgerDirsListener listener = new LedgerDirsListener() { + @Override + public void fatalError() { + fatalLatch.countDown(); + } + }; + + LedgerStorage storage = new DummyLedgerStorage() { + @Override + public void checkpoint(Checkpoint checkpoint) + throws IOException { + throw new EntryLogWriteException( + "entry log flush failed", new IOException("injected")); + } + }; + final SyncThread t = new SyncThread(conf, listener, storage, checkpointSource, NullStatsLogger.INSTANCE); + t.startCheckpoint(Checkpoint.MAX); + assertTrue("Should have called fatal error", fatalLatch.await(10, TimeUnit.SECONDS)); + t.shutdown(); + } + + @Test + public void testSyncThreadShutdownOnEntryLogWriteFailureDuringFlush() throws Exception { + int flushInterval = 100; + ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); + conf.setFlushInterval(flushInterval); + CheckpointSource checkpointSource = new DummyCheckpointSource(); + final CountDownLatch fatalLatch = new CountDownLatch(1); + LedgerDirsListener listener = new LedgerDirsListener() { + @Override + public void fatalError() { + fatalLatch.countDown(); + } + }; + + LedgerStorage storage = new DummyLedgerStorage() { + @Override + public void flush() throws IOException { + throw new EntryLogWriteException( + "entry log flush failed", new IOException("injected")); + } + }; + final SyncThread t = new SyncThread(conf, listener, storage, checkpointSource, NullStatsLogger.INSTANCE); + t.requestFlush().get(10, TimeUnit.SECONDS); + assertTrue("Should have called fatal error", fatalLatch.await(10, TimeUnit.SECONDS)); + t.shutdown(); + } + /** * Test that if the ledger storage throws * a disk full exception, the owner of the sync diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageWriteCacheTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageWriteCacheTest.java index 102f7f5addc..0ddb3fe59e4 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageWriteCacheTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageWriteCacheTest.java @@ -21,6 +21,7 @@ package org.apache.bookkeeper.bookie.storage.ldb; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import io.netty.buffer.ByteBuf; @@ -28,10 +29,15 @@ import io.netty.buffer.Unpooled; import java.io.File; import java.io.IOException; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import org.apache.bookkeeper.bookie.Bookie; import org.apache.bookkeeper.bookie.BookieException.OperationRejectedException; import org.apache.bookkeeper.bookie.BookieImpl; +import org.apache.bookkeeper.bookie.EntryLogWriteException; import org.apache.bookkeeper.bookie.LedgerDirsManager; +import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener; import org.apache.bookkeeper.bookie.TestBookieImpl; import org.apache.bookkeeper.bookie.storage.EntryLogger; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -64,6 +70,8 @@ protected SingleDirectoryDbLedgerStorage newSingleDirectoryDbLedgerStorage(Serve } private static class MockedSingleDirectoryDbLedgerStorage extends SingleDirectoryDbLedgerStorage { + private static final AtomicBoolean failNextFlushWithEntryLogWriteException = new AtomicBoolean(false); + public MockedSingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager ledgerManager, LedgerDirsManager ledgerDirsManager, LedgerDirsManager indexDirsManager, EntryLogger entryLogger, StatsLogger statsLogger, @@ -75,29 +83,33 @@ public MockedSingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerMana readAheadCacheBatchBytesSize); } - @Override - public void flush() throws IOException { - flushMutex.lock(); - try { - // Swap the write caches and block indefinitely to simulate a slow disk - WriteCache tmp = writeCacheBeingFlushed; - writeCacheBeingFlushed = writeCache; - writeCache = tmp; - - // since the cache is switched, we can allow flush to be triggered - hasFlushBeenTriggered.set(false); - - // Block the flushing thread - try { - Thread.sleep(1000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - return; - } - } finally { - flushMutex.unlock(); - } - } + @Override + public void flush() throws IOException { + if (failNextFlushWithEntryLogWriteException.compareAndSet(true, false)) { + throw new EntryLogWriteException("entry log flush failed", new IOException("injected")); + } + + flushMutex.lock(); + try { + // Swap the write caches and block indefinitely to simulate a slow disk + WriteCache tmp = writeCacheBeingFlushed; + writeCacheBeingFlushed = writeCache; + writeCache = tmp; + + // since the cache is switched, we can allow flush to be triggered + hasFlushBeenTriggered.set(false); + + // Block the flushing thread + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return; + } + } finally { + flushMutex.unlock(); + } + } } } @@ -116,6 +128,7 @@ public void setup() throws Exception { conf.setProperty(DbLedgerStorage.WRITE_CACHE_MAX_SIZE_MB, 1); conf.setProperty(DbLedgerStorage.MAX_THROTTLE_TIME_MILLIS, 1000); conf.setLedgerDirNames(new String[] { tmpDir.toString() }); + MockedDbLedgerStorage.MockedSingleDirectoryDbLedgerStorage.failNextFlushWithEntryLogWriteException.set(false); Bookie bookie = new TestBookieImpl(conf); storage = (DbLedgerStorage) bookie.getLedgerStorage(); @@ -165,4 +178,36 @@ public void writeCacheFull() throws Exception { // Expected } } + + @Test + public void writeCacheFullEntryLogWriteFailureTriggersFatalError() throws Exception { + storage.setMasterKey(4, "key".getBytes()); + CountDownLatch fatalLatch = new CountDownLatch(1); + storage.setFatalErrorListener(new LedgerDirsListener() { + @Override + public void fatalError() { + fatalLatch.countDown(); + } + }); + MockedDbLedgerStorage.MockedSingleDirectoryDbLedgerStorage.failNextFlushWithEntryLogWriteException.set(true); + + try { + for (int i = 0; i < 10; i++) { + storage.addEntry(newEntry(4, i)); + } + fail("Should have thrown exception"); + } catch (OperationRejectedException e) { + // Expected because the background flush failed before rotating the write cache. + } + + assertTrue("Should have called fatal error", fatalLatch.await(10, TimeUnit.SECONDS)); + } + + private static ByteBuf newEntry(long ledgerId, long entryId) { + ByteBuf entry = Unpooled.buffer(100 * 1024 + 2 * 8); + entry.writeLong(ledgerId); + entry.writeLong(entryId); + entry.writeZero(100 * 1024); + return entry; + } } From a00d96be4a4867d5af10d8309fefe9f8f73c8f72 Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Tue, 4 Aug 2026 15:01:12 +0800 Subject: [PATCH 3/9] test: add e2e coverage for entry log flush failure (cherry picked from commit 4058ab58b53a7ab7c6e483669419c5b09b2085e5) --- .../bookkeeper/bookie/BookieImplTest.java | 46 +----------- ...gerStorageEntryLogFlushFailureE2ETest.java | 74 +++++++++++++++++++ .../ldb/FailOnFlushDbLedgerStorage.java | 73 ++++++++++++++++++ 3 files changed, 151 insertions(+), 42 deletions(-) create mode 100644 bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java create mode 100644 bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java index 562ceb94d8f..77a206ae145 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java @@ -29,28 +29,23 @@ import com.google.protobuf.ByteString; import com.google.protobuf.UnsafeByteOperations; import io.netty.buffer.ByteBuf; -import io.netty.buffer.ByteBufAllocator; import io.netty.buffer.Unpooled; import io.netty.buffer.UnpooledByteBufAllocator; -import java.io.IOException; import java.nio.charset.StandardCharsets; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; -import org.apache.bookkeeper.bookie.storage.EntryLogger; import org.apache.bookkeeper.bookie.storage.ldb.DbLedgerStorage; -import org.apache.bookkeeper.bookie.storage.ldb.SingleDirectoryDbLedgerStorage; +import org.apache.bookkeeper.bookie.storage.ldb.FailOnFlushDbLedgerStorage; import org.apache.bookkeeper.client.BookKeeper; import org.apache.bookkeeper.conf.ServerConfiguration; import org.apache.bookkeeper.conf.TestBKConfiguration; import org.apache.bookkeeper.discover.RegistrationManager; -import org.apache.bookkeeper.meta.LedgerManager; import org.apache.bookkeeper.meta.MetadataBookieDriver; import org.apache.bookkeeper.net.BookieId; import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks; import org.apache.bookkeeper.proto.checksum.DigestManager; import org.apache.bookkeeper.stats.NullStatsLogger; -import org.apache.bookkeeper.stats.StatsLogger; import org.apache.bookkeeper.test.BookKeeperClusterTestCase; import org.apache.bookkeeper.util.ByteBufList; import org.apache.bookkeeper.util.PortManager; @@ -144,7 +139,7 @@ public void testBackgroundEntryLogFlushFailureShutsDownBookie() throws Exception conf.setJournalWriteData(false); conf.setProperty(DbLedgerStorage.WRITE_CACHE_MAX_SIZE_MB, 1); conf.setProperty("dbStorage_maxThrottleTimeMs", 1000); - FailOnFlushDbLedgerStorage.failNextFlushWithEntryLogWriteException.set(false); + FailOnFlushDbLedgerStorage.resetFailure(); CountDownLatch shutdownLatch = new CountDownLatch(1); TestBookieImpl.Resources resources = new TestBookieImpl.ResourceBuilder(conf).build(); @@ -161,13 +156,13 @@ int shutdown(int exitCode) { try { bookie.start(); - FailOnFlushDbLedgerStorage.failNextFlushWithEntryLogWriteException.set(true); + FailOnFlushDbLedgerStorage.injectFailureOnNextFlush(); writeUntilCacheFlushIsTriggered(bookie, 10L, "masterKey".getBytes(StandardCharsets.UTF_8)); assertTrue("Entry log flush failure should shut down the bookie", shutdownLatch.await(10, TimeUnit.SECONDS)); } finally { - FailOnFlushDbLedgerStorage.failNextFlushWithEntryLogWriteException.set(false); + FailOnFlushDbLedgerStorage.resetFailure(); if (bookie.isRunning()) { bookie.shutdown(); } @@ -260,37 +255,4 @@ private ByteBuf generateLargeEntry(long ledger, long entry) { return bb; } - public static class FailOnFlushDbLedgerStorage extends DbLedgerStorage { - private static final AtomicBoolean failNextFlushWithEntryLogWriteException = new AtomicBoolean(false); - - @Override - protected SingleDirectoryDbLedgerStorage newSingleDirectoryDbLedgerStorage(ServerConfiguration conf, - LedgerManager ledgerManager, LedgerDirsManager ledgerDirsManager, LedgerDirsManager indexDirsManager, - EntryLogger entryLogger, StatsLogger statsLogger, long writeCacheSize, long readCacheSize, - int readAheadCacheBatchSize, long readAheadCacheBatchBytesSize) - throws IOException { - return new FailOnFlushSingleDirectoryDbLedgerStorage(conf, ledgerManager, ledgerDirsManager, - indexDirsManager, entryLogger, statsLogger, allocator, writeCacheSize, readCacheSize, - readAheadCacheBatchSize, readAheadCacheBatchBytesSize); - } - } - - private static class FailOnFlushSingleDirectoryDbLedgerStorage extends SingleDirectoryDbLedgerStorage { - FailOnFlushSingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager ledgerManager, - LedgerDirsManager ledgerDirsManager, LedgerDirsManager indexDirsManager, EntryLogger entryLogger, - StatsLogger statsLogger, ByteBufAllocator allocator, long writeCacheSize, long readCacheSize, - int readAheadCacheBatchSize, long readAheadCacheBatchBytesSize) - throws IOException { - super(conf, ledgerManager, ledgerDirsManager, indexDirsManager, entryLogger, statsLogger, allocator, - writeCacheSize, readCacheSize, readAheadCacheBatchSize, readAheadCacheBatchBytesSize); - } - - @Override - public void flush() throws IOException { - if (FailOnFlushDbLedgerStorage.failNextFlushWithEntryLogWriteException.compareAndSet(true, false)) { - throw new EntryLogWriteException("entry log flush failed", new IOException("injected")); - } - super.flush(); - } - } } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java new file mode 100644 index 00000000000..ce093ecd367 --- /dev/null +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java @@ -0,0 +1,74 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.bookkeeper.bookie.storage.ldb; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; + +import java.util.concurrent.TimeUnit; +import org.apache.bookkeeper.bookie.BookieImpl; +import org.apache.bookkeeper.client.BKException; +import org.apache.bookkeeper.client.BookKeeper.DigestType; +import org.apache.bookkeeper.client.LedgerHandle; +import org.apache.bookkeeper.test.BookKeeperClusterTestCase; +import org.awaitility.Awaitility; +import org.junit.Test; + +public class DbLedgerStorageEntryLogFlushFailureE2ETest extends BookKeeperClusterTestCase { + private static final byte[] PASSWD = "passwd".getBytes(UTF_8); + + public DbLedgerStorageEntryLogFlushFailureE2ETest() { + super(1); + baseConf.setLedgerStorageClass(FailOnFlushDbLedgerStorage.class.getName()); + baseConf.setFlushInterval(60000); + baseConf.setGcWaitTime(60000); + baseConf.setProperty(DbLedgerStorage.WRITE_CACHE_MAX_SIZE_MB, 1); + baseConf.setProperty(DbLedgerStorage.MAX_THROTTLE_TIME_MILLIS, 1000); + baseClientConf.setAddEntryTimeout(5); + } + + @Test + public void testClientWriteFailsAndBookieShutsDownAfterEntryLogFlushFailure() throws Exception { + BookieImpl bookie = (BookieImpl) serverByIndex(0).getBookie(); + LedgerHandle lh = bkc.createLedger(1, 1, 1, DigestType.CRC32, PASSWD); + byte[] payload = new byte[100 * 1024]; + BKException clientFailure = null; + + FailOnFlushDbLedgerStorage.injectFailureOnNextFlush(); + try { + for (int i = 0; i < 20; i++) { + try { + lh.addEntry(payload); + } catch (BKException e) { + clientFailure = e; + break; + } + } + + assertNotNull("Client should observe a write failure after the entry log flush failure", clientFailure); + Awaitility.await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> + assertFalse("Bookie should be shut down after entry log flush failure", bookie.isRunning())); + } finally { + FailOnFlushDbLedgerStorage.resetFailure(); + } + } +} diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java new file mode 100644 index 00000000000..cc6286de042 --- /dev/null +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java @@ -0,0 +1,73 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.bookkeeper.bookie.storage.ldb; + +import io.netty.buffer.ByteBufAllocator; +import java.io.IOException; +import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.bookkeeper.bookie.EntryLogWriteException; +import org.apache.bookkeeper.bookie.LedgerDirsManager; +import org.apache.bookkeeper.bookie.storage.EntryLogger; +import org.apache.bookkeeper.conf.ServerConfiguration; +import org.apache.bookkeeper.meta.LedgerManager; +import org.apache.bookkeeper.stats.StatsLogger; + +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); + } + + @Override + protected SingleDirectoryDbLedgerStorage newSingleDirectoryDbLedgerStorage(ServerConfiguration conf, + LedgerManager ledgerManager, LedgerDirsManager ledgerDirsManager, LedgerDirsManager indexDirsManager, + EntryLogger entryLogger, StatsLogger statsLogger, long writeCacheSize, long readCacheSize, + int readAheadCacheBatchSize, long readAheadCacheBatchBytesSize) + throws IOException { + return new FailOnFlushSingleDirectoryDbLedgerStorage(conf, ledgerManager, ledgerDirsManager, + indexDirsManager, entryLogger, statsLogger, allocator, writeCacheSize, readCacheSize, + readAheadCacheBatchSize, readAheadCacheBatchBytesSize); + } + + private static class FailOnFlushSingleDirectoryDbLedgerStorage extends SingleDirectoryDbLedgerStorage { + FailOnFlushSingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager ledgerManager, + LedgerDirsManager ledgerDirsManager, LedgerDirsManager indexDirsManager, EntryLogger entryLogger, + StatsLogger statsLogger, ByteBufAllocator allocator, long writeCacheSize, long readCacheSize, + int readAheadCacheBatchSize, long readAheadCacheBatchBytesSize) + throws IOException { + super(conf, ledgerManager, ledgerDirsManager, indexDirsManager, entryLogger, statsLogger, allocator, + writeCacheSize, readCacheSize, readAheadCacheBatchSize, readAheadCacheBatchBytesSize); + } + + @Override + public void flush() throws IOException { + if (failNextFlushWithEntryLogWriteException.compareAndSet(true, false)) { + throw new EntryLogWriteException("entry log flush failed", new IOException("injected")); + } + super.flush(); + } + } +} From 0c6c801a93890040829341cc24c5e7411bb6b98f Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Tue, 4 Aug 2026 16:58:55 +0800 Subject: [PATCH 4/9] fix: close entrylog failure propagation gaps --- .../EntryLogManagerForEntryLogPerLedger.java | 9 +++- .../apache/bookkeeper/bookie/SyncThread.java | 7 ++- .../bookkeeper/bookie/BookieImplTest.java | 36 ++++++++++++++ .../bookie/DefaultEntryLogTest.java | 49 +++++++++++++++++++ .../bookkeeper/bookie/SyncThreadTest.java | 10 +++- .../ldb/FailOnFlushDbLedgerStorage.java | 5 +- 6 files changed, 110 insertions(+), 6 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java index c71a54ace25..6a78c8c3dba 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java @@ -356,8 +356,13 @@ private void onCacheEntryRemoval(RemovalNotification // Append ledgers map at the end of entry log try { logChannel.appendLedgersMap(); - } catch (Exception e) { - log.error("Got IOException while trying to appendLedgersMap in cacheEntryRemoval callback", e); + } catch (IOException e) { + log.error("Fatal entry log write failure while trying to appendLedgersMap " + + "in cacheEntryRemoval callback", e); + for (LedgerDirsListener listener : ledgerDirsManager.getListeners()) { + listener.fatalError(); + } + return; } replicaOfCurrentLogChannels.remove(logChannel.getLogId()); rotatedLogChannels.add(logChannel); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java index 4a8a205128c..1de3f5a068d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java @@ -130,15 +130,18 @@ public Future requestFlush() { long startTime = System.nanoTime(); try { flush(); + } catch (EntryLogWriteException e) { + throw e; } catch (Throwable t) { log.error("Exception flushing ledgers ", t); } finally { syncExecutorTime.addLatency(MathUtils.elapsedNanos(startTime), TimeUnit.NANOSECONDS); } + return null; }); } - private void flush() { + private void flush() throws EntryLogWriteException { Checkpoint checkpoint = checkpointSource.newCheckpoint(); try { ledgerStorage.flush(); @@ -149,7 +152,7 @@ private void flush() { } catch (EntryLogWriteException e) { log.error("Fatal entry log write failure while flushing ledgers", e); dirsListener.fatalError(); - return; + throw e; } catch (IOException e) { log.error("Exception flushing ledgers", e); return; diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java index 77a206ae145..7ad6a51e48b 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java @@ -21,6 +21,7 @@ package org.apache.bookkeeper.bookie; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; @@ -169,6 +170,41 @@ int shutdown(int exitCode) { } } + @Test + public void testStartupEntryLogFlushFailureStopsBookieBeforeRunning() throws Exception { + ServerConfiguration conf = newServerConfiguration(); + conf.setLedgerStorageClass(FailOnFlushDbLedgerStorage.class.getName()); + conf.setJournalWriteData(false); + FailOnFlushDbLedgerStorage.resetFailure(); + + CountDownLatch shutdownLatch = new CountDownLatch(1); + TestBookieImpl.Resources resources = new TestBookieImpl.ResourceBuilder(conf).build(); + BookieImpl bookie = new TestBookieImpl(resources) { + @Override + int shutdown(int exitCode) { + int result = super.shutdown(exitCode); + if (exitCode == ExitCode.BOOKIE_EXCEPTION) { + shutdownLatch.countDown(); + } + return result; + } + }; + + try { + FailOnFlushDbLedgerStorage.injectFailureOnNextFlush(); + bookie.start(); + + assertTrue("Startup entry log flush failure should shut down the bookie", + shutdownLatch.await(10, TimeUnit.SECONDS)); + assertFalse("Bookie should not keep running after startup entry log flush failure", bookie.isRunning()); + } finally { + FailOnFlushDbLedgerStorage.resetFailure(); + if (bookie.isRunning()) { + bookie.shutdown(); + } + } + } + public void mockAddEntryReleased(int flag) throws Exception { final String metadataServiceUri = zkUtil.getMetadataServiceUri(); ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java index cbf12511126..47139cf8b92 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java @@ -58,11 +58,13 @@ import java.util.concurrent.CyclicBarrier; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLongArray; import java.util.concurrent.locks.Lock; import org.apache.bookkeeper.bookie.DefaultEntryLogger.BufferedLogChannel; +import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener; import org.apache.bookkeeper.bookie.LedgerDirsManager.NoWritableLedgerDirException; import org.apache.bookkeeper.common.testing.annotations.FlakyTest; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -1293,6 +1295,53 @@ public void testAppendLedgersMapOnCacheRemoval() throws Exception { Assert.assertEquals("Total size of entries", (entrySize + 4) * numOfEntries, ledgersMap.get(ledgerId)); } + @Test + public void testAppendLedgersMapFailureOnCacheRemovalTriggersFatalError() throws Exception { + int evictionPeriod = 1; + + ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); + conf.setEntryLogFilePreAllocationEnabled(false); + conf.setEntryLogPerLedgerEnabled(true); + conf.setLedgerDirNames(createAndGetLedgerDirs(1)); + conf.setEntrylogMapAccessExpiryTimeInSeconds(evictionPeriod); + LedgerDirsManager ledgerDirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(), + new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); + + CountDownLatch fatalLatch = new CountDownLatch(1); + ledgerDirsManager.addLedgerDirsListener(new LedgerDirsListener() { + @Override + public void fatalError() { + fatalLatch.countDown(); + } + }); + + DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); + EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) entryLogger + .getEntryLogManager(); + + long ledgerId = 0L; + File tmpFile = File.createTempFile("entrylog", "failed-eviction"); + tmpFile.deleteOnExit(); + FileChannel fileChannel = new RandomAccessFile(tmpFile, "rw").getChannel(); + BufferedLogChannel logChannel = new BufferedLogChannel(UnpooledByteBufAllocator.DEFAULT, fileChannel, 10, 10, + 0L, tmpFile, conf.getFlushIntervalInBytes()); + + try { + entryLogManager.setCurrentLogForLedgerAndAddToRotate(ledgerId, logChannel); + + fileChannel.close(); + Thread.sleep(evictionPeriod * 1000 + 100); + entryLogManager.doEntryLogMapCleanup(); + + assertTrue("Cache removal appendLedgersMap failure should trigger fatal error", + fatalLatch.await(10, TimeUnit.SECONDS)); + Assert.assertFalse("Failed log channel should not be added to rotated logs", + entryLogManager.getRotatedLogChannels().contains(logChannel)); + } finally { + logChannel.close(); + } + } + /** * test EntryLogManager.EntryLogManagerForEntryLogPerLedger doesn't removes * the ledger from its cache map if ledger's corresponding state is accessed diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java index c5548d112b2..98857c616c3 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java @@ -31,6 +31,7 @@ import java.util.EnumSet; import java.util.PrimitiveIterator.OfLong; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -273,7 +274,14 @@ public void flush() throws IOException { } }; final SyncThread t = new SyncThread(conf, listener, storage, checkpointSource, NullStatsLogger.INSTANCE); - t.requestFlush().get(10, TimeUnit.SECONDS); + Future flush = t.requestFlush(); + try { + flush.get(10, TimeUnit.SECONDS); + fail("Flush future should fail on entry log write failure"); + } catch (ExecutionException e) { + assertTrue("Flush future should fail with entry log write failure", + e.getCause() instanceof EntryLogWriteException); + } assertTrue("Should have called fatal error", fatalLatch.await(10, TimeUnit.SECONDS)); t.shutdown(); } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java index cc6286de042..19d97fe8da2 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java @@ -32,6 +32,7 @@ public class FailOnFlushDbLedgerStorage extends DbLedgerStorage { private static final AtomicBoolean failNextFlushWithEntryLogWriteException = new AtomicBoolean(false); + private static final AtomicBoolean entryLogFlushFailed = new AtomicBoolean(false); public static void injectFailureOnNextFlush() { failNextFlushWithEntryLogWriteException.set(true); @@ -39,6 +40,7 @@ public static void injectFailureOnNextFlush() { public static void resetFailure() { failNextFlushWithEntryLogWriteException.set(false); + entryLogFlushFailed.set(false); } @Override @@ -64,7 +66,8 @@ private static class FailOnFlushSingleDirectoryDbLedgerStorage extends SingleDir @Override public void flush() throws IOException { - if (failNextFlushWithEntryLogWriteException.compareAndSet(true, false)) { + if (entryLogFlushFailed.get() || failNextFlushWithEntryLogWriteException.compareAndSet(true, false)) { + entryLogFlushFailed.set(true); throw new EntryLogWriteException("entry log flush failed", new IOException("injected")); } super.flush(); From e5b080c7b125b1237e8100d376b979400b161a55 Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Tue, 4 Aug 2026 20:11:29 +0800 Subject: [PATCH 5/9] fix: keep fatal entrylog notifications compatible --- .../bookkeeper/bookie/DefaultEntryLogger.java | 5 ++ .../bookkeeper/bookie/EntryLogManager.java | 6 ++ .../bookie/EntryLogManagerBase.java | 18 +++++ .../EntryLogManagerForEntryLogPerLedger.java | 10 +-- .../ldb/SingleDirectoryDbLedgerStorage.java | 4 + .../storage/ldb/DbLedgerStorageTest.java | 79 +++++++++++++++++++ 6 files changed, 116 insertions(+), 6 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java index 831f24a344f..32ece69f606 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java @@ -57,6 +57,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.regex.Pattern; +import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener; import org.apache.bookkeeper.bookie.storage.CompactionEntryLog; import org.apache.bookkeeper.bookie.storage.EntryLogScanner; import org.apache.bookkeeper.bookie.storage.EntryLogger; @@ -392,6 +393,10 @@ EntryLogManager getEntryLogManager() { return entryLogManager; } + public void setFatalErrorListener(LedgerDirsListener fatalErrorListener) { + entryLogManager.setFatalErrorListener(fatalErrorListener); + } + void addListener(EntryLogListener listener) { if (null != listener) { listeners.add(listener); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManager.java index 7364fb08e56..763243fbbf0 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManager.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManager.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.List; import org.apache.bookkeeper.bookie.DefaultEntryLogger.BufferedLogChannel; +import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener; interface EntryLogManager { @@ -66,6 +67,11 @@ interface EntryLogManager { */ void forceClose(); + /* + * notify the owning bookie when entry-log-level writes become fatal. + */ + void setFatalErrorListener(LedgerDirsListener fatalErrorListener); + /* * prepare entrylogger/entrylogmanager before doing SortedLedgerStorage * Checkpoint. diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java index b8312bf4503..9594b99c01e 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerBase.java @@ -33,6 +33,7 @@ import lombok.extern.slf4j.Slf4j; import org.apache.bookkeeper.bookie.DefaultEntryLogger.BufferedLogChannel; import org.apache.bookkeeper.bookie.DefaultEntryLogger.EntryLogListener; +import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener; import org.apache.bookkeeper.bookie.LedgerDirsManager.NoWritableLedgerDirException; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -42,6 +43,8 @@ abstract class EntryLogManagerBase implements EntryLogManager { final EntryLoggerAllocator entryLoggerAllocator; final LedgerDirsManager ledgerDirsManager; private final List listeners; + private static final LedgerDirsListener NOOP_FATAL_ERROR_LISTENER = new LedgerDirsListener() { }; + private volatile LedgerDirsListener fatalErrorListener = NOOP_FATAL_ERROR_LISTENER; /** * The maximum size of a entry logger file. */ @@ -124,6 +127,21 @@ List getRotatedLogChannels() { return rotatedLogChannels; } + @Override + public void setFatalErrorListener(LedgerDirsListener fatalErrorListener) { + this.fatalErrorListener = fatalErrorListener != null ? fatalErrorListener : NOOP_FATAL_ERROR_LISTENER; + } + + void notifyFatalEntryLogWriteFailure(String message, Throwable cause) { + log.error(message, cause); + fatalErrorListener.fatalError(); + for (LedgerDirsListener listener : ledgerDirsManager.getListeners()) { + if (listener != fatalErrorListener) { + listener.fatalError(); + } + } + } + @Override public void flush() throws IOException { flushCurrentLogs(); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java index 6a78c8c3dba..5fe5d5e8b67 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogManagerForEntryLogPerLedger.java @@ -356,12 +356,10 @@ private void onCacheEntryRemoval(RemovalNotification // Append ledgers map at the end of entry log try { logChannel.appendLedgersMap(); - } catch (IOException e) { - log.error("Fatal entry log write failure while trying to appendLedgersMap " - + "in cacheEntryRemoval callback", e); - for (LedgerDirsListener listener : ledgerDirsManager.getListeners()) { - listener.fatalError(); - } + } catch (IOException | RuntimeException e) { + notifyFatalEntryLogWriteFailure( + "Fatal entry log write failure while trying to appendLedgersMap in cacheEntryRemoval callback", + e); return; } replicaOfCurrentLogChannels.remove(logChannel.getLogId()); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java index 7ec0e309368..be219fcebb3 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java @@ -56,6 +56,7 @@ import org.apache.bookkeeper.bookie.CheckpointSource.Checkpoint; import org.apache.bookkeeper.bookie.Checkpointer; import org.apache.bookkeeper.bookie.CompactableLedgerStorage; +import org.apache.bookkeeper.bookie.DefaultEntryLogger; import org.apache.bookkeeper.bookie.EntryLogWriteException; import org.apache.bookkeeper.bookie.EntryLocation; import org.apache.bookkeeper.bookie.GarbageCollectionStatus; @@ -256,6 +257,9 @@ public void setCheckpointer(Checkpointer checkpointer) { } void setFatalErrorListener(LedgerDirsListener fatalErrorListener) { if (fatalErrorListener != null) { this.fatalErrorListener = fatalErrorListener; + if (entryLogger instanceof DefaultEntryLogger) { + ((DefaultEntryLogger) entryLogger).setFatalErrorListener(fatalErrorListener); + } } } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java index dfc2459678b..f0b39ca9b12 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java @@ -34,18 +34,25 @@ import java.io.FileInputStream; import java.io.IOException; import java.lang.reflect.Field; +import java.lang.reflect.Method; import java.nio.ByteBuffer; +import java.nio.channels.FileChannel; import java.util.List; import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; import org.apache.bookkeeper.bookie.Bookie; import org.apache.bookkeeper.bookie.Bookie.NoEntryException; import org.apache.bookkeeper.bookie.BookieException; +import org.apache.bookkeeper.bookie.BufferedChannel; +import org.apache.bookkeeper.bookie.BufferedChannelBase; import org.apache.bookkeeper.bookie.BookieImpl; import org.apache.bookkeeper.bookie.CheckpointSource; import org.apache.bookkeeper.bookie.CheckpointSourceList; import org.apache.bookkeeper.bookie.DefaultEntryLogger; import org.apache.bookkeeper.bookie.EntryLocation; import org.apache.bookkeeper.bookie.LedgerDirsManager; +import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener; import org.apache.bookkeeper.bookie.LedgerStorage; import org.apache.bookkeeper.bookie.LogMark; import org.apache.bookkeeper.bookie.TestBookieImpl; @@ -53,6 +60,7 @@ import org.apache.bookkeeper.conf.ServerConfiguration; import org.apache.bookkeeper.conf.TestBKConfiguration; import org.apache.bookkeeper.proto.BookieProtocol; +import org.apache.commons.io.FileUtils; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -255,6 +263,58 @@ public void testBookieCompaction() throws Exception { assertEquals(newEntry3, res); } + @Test + public void testPerLedgerEvictionFailurePropagatesToDbFatalErrorListener() throws Exception { + File perLedgerDir = File.createTempFile("bkTestPerLedger", ".dir"); + perLedgerDir.delete(); + perLedgerDir.mkdir(); + File curDir = BookieImpl.getCurrentDirectory(perLedgerDir); + BookieImpl.checkDirectoryStructure(curDir); + + ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); + conf.setGcWaitTime(1000); + conf.setLedgerStorageClass(DbLedgerStorage.class.getName()); + conf.setLedgerDirNames(new String[] { perLedgerDir.toString() }); + conf.setEntryLogFilePreAllocationEnabled(false); + conf.setEntryLogPerLedgerEnabled(true); + conf.setEntrylogMapAccessExpiryTimeInSeconds(1); + + BookieImpl bookie = new TestBookieImpl(conf); + DbLedgerStorage storage = (DbLedgerStorage) bookie.getLedgerStorage(); + CountDownLatch fatalLatch = new CountDownLatch(1); + storage.setFatalErrorListener(new LedgerDirsListener() { + @Override + public void fatalError() { + fatalLatch.countDown(); + } + }); + + try { + SingleDirectoryDbLedgerStorage singleDirStorage = storage.getLedgerStorageList().get(0); + DefaultEntryLogger entryLogger = (DefaultEntryLogger) singleDirStorage.getEntryLogger(); + long ledgerId = 4L; + ByteBuf entry = Unpooled.buffer(1024); + entry.writeLong(ledgerId); + entry.writeLong(1L); + entry.writeBytes("entry-1".getBytes()); + entryLogger.addEntry(ledgerId, entry); + + Object entryLogManager = getEntryLogManager(entryLogger); + BufferedChannel currentLogChannel = (BufferedChannel) invoke(entryLogManager, + "getCurrentLogForLedger", new Class[] { long.class }, ledgerId); + closeUnderlyingFileChannel(currentLogChannel); + + Thread.sleep(TimeUnit.SECONDS.toMillis(2)); + invoke(entryLogManager, "doEntryLogMapCleanup", new Class[] { }); + + assertTrue("Per-ledger eviction failure should propagate through DbLedgerStorage fatal listener", + fatalLatch.await(10, TimeUnit.SECONDS)); + } finally { + bookie.shutdown(); + FileUtils.deleteDirectory(perLedgerDir); + } + } + @Test public void doubleDirectory() throws Exception { int gcWaitTime = 1000; @@ -274,6 +334,25 @@ public void doubleDirectory() throws Exception { bookie.shutdown(); } + private Object getEntryLogManager(DefaultEntryLogger entryLogger) throws Exception { + Method method = DefaultEntryLogger.class.getDeclaredMethod("getEntryLogManager"); + method.setAccessible(true); + return method.invoke(entryLogger); + } + + private void closeUnderlyingFileChannel(BufferedChannel channel) throws Exception { + Field fileChannelField = BufferedChannelBase.class.getDeclaredField("fileChannel"); + fileChannelField.setAccessible(true); + ((FileChannel) fileChannelField.get(channel)).close(); + } + + private Object invoke(Object target, String methodName, Class[] parameterTypes, Object... args) + throws Exception { + Method method = target.getClass().getDeclaredMethod(methodName, parameterTypes); + method.setAccessible(true); + return method.invoke(target, args); + } + @Test public void testRewritingEntries() throws Exception { storage.setMasterKey(1, "key".getBytes()); From cf7be85ab664fd5dcc0b202c3c586a6ca0b41edd Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Tue, 4 Aug 2026 22:40:33 +0800 Subject: [PATCH 6/9] test: release per-ledger eviction test entry --- .../bookie/storage/ldb/DbLedgerStorageTest.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java index f0b39ca9b12..448f3737875 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java @@ -294,10 +294,14 @@ public void fatalError() { DefaultEntryLogger entryLogger = (DefaultEntryLogger) singleDirStorage.getEntryLogger(); long ledgerId = 4L; ByteBuf entry = Unpooled.buffer(1024); - entry.writeLong(ledgerId); - entry.writeLong(1L); - entry.writeBytes("entry-1".getBytes()); - entryLogger.addEntry(ledgerId, entry); + try { + entry.writeLong(ledgerId); + entry.writeLong(1L); + entry.writeBytes("entry-1".getBytes()); + entryLogger.addEntry(ledgerId, entry); + } finally { + ReferenceCountUtil.release(entry); + } Object entryLogManager = getEntryLogManager(entryLogger); BufferedChannel currentLogChannel = (BufferedChannel) invoke(entryLogManager, From db5310899510748f0a7667497b188068bf134605 Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Wed, 5 Aug 2026 15:37:14 +0800 Subject: [PATCH 7/9] fix: continue db storage shutdown cleanup after flush failure (cherry picked from commit acff4638b3b1d53ebc25dfbb2eb2feee21aa1ae6) --- .../ldb/SingleDirectoryDbLedgerStorage.java | 37 ++++- ...eDirectoryDbLedgerStorageShutdownTest.java | 130 ++++++++++++++++++ 2 files changed, 160 insertions(+), 7 deletions(-) create mode 100644 bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorageShutdownTest.java diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java index be219fcebb3..dccfb461d4e 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorage.java @@ -78,6 +78,7 @@ import org.apache.bookkeeper.stats.OpStatsLogger; import org.apache.bookkeeper.stats.StatsLogger; import org.apache.bookkeeper.stats.ThreadRegistry; +import org.apache.bookkeeper.util.IOUtils; import org.apache.bookkeeper.util.MathUtils; import org.apache.bookkeeper.util.collections.ConcurrentLongHashMap; import org.apache.commons.collections4.CollectionUtils; @@ -353,25 +354,47 @@ public List getEntryLocationDBPath() { @Override public void shutdown() throws InterruptedException { + InterruptedException interrupted = null; + try { flush(); + } catch (IOException e) { + log.error("Error flushing db storage during shutdown", e); + } finally { + try { + gcThread.shutdown(); + } catch (InterruptedException e) { + interrupted = e; + Thread.currentThread().interrupt(); + } - gcThread.shutdown(); - entryLogger.close(); + try { + entryLogger.close(); + } catch (IOException e) { + log.error("Error closing entry logger during shutdown", e); + } cleanupExecutor.shutdown(); - cleanupExecutor.awaitTermination(1, TimeUnit.SECONDS); + try { + cleanupExecutor.awaitTermination(1, TimeUnit.SECONDS); + } catch (InterruptedException e) { + if (interrupted == null) { + interrupted = e; + } + Thread.currentThread().interrupt(); + } - ledgerIndex.close(); - entryLocationIndex.close(); + IOUtils.close(log, ledgerIndex); + IOUtils.close(log, entryLocationIndex); writeCache.close(); writeCacheBeingFlushed.close(); readCache.close(); executor.shutdown(); + } - } catch (IOException e) { - log.error("Error closing db storage", e); + if (interrupted != null) { + throw interrupted; } } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorageShutdownTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorageShutdownTest.java new file mode 100644 index 00000000000..079cb1d251c --- /dev/null +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/SingleDirectoryDbLedgerStorageShutdownTest.java @@ -0,0 +1,130 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.bookkeeper.bookie.storage.ldb; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +import io.netty.buffer.ByteBufAllocator; +import java.io.File; +import java.io.IOException; +import java.lang.reflect.Field; +import java.util.concurrent.ExecutorService; +import org.apache.bookkeeper.bookie.BookieImpl; +import org.apache.bookkeeper.bookie.EntryLogWriteException; +import org.apache.bookkeeper.bookie.GarbageCollectorThread; +import org.apache.bookkeeper.bookie.LedgerDirsManager; +import org.apache.bookkeeper.bookie.storage.EntryLogger; +import org.apache.bookkeeper.conf.ServerConfiguration; +import org.apache.bookkeeper.conf.TestBKConfiguration; +import org.apache.bookkeeper.meta.LedgerManager; +import org.apache.bookkeeper.stats.NullStatsLogger; +import org.apache.bookkeeper.stats.StatsLogger; +import org.apache.bookkeeper.util.DiskChecker; +import org.apache.commons.io.FileUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * Tests shutdown cleanup for {@link SingleDirectoryDbLedgerStorage}. + */ +public class SingleDirectoryDbLedgerStorageShutdownTest { + + private static final long MB = 1024 * 1024; + + private File tmpDir; + private EntryLogger entryLogger; + private FailingFlushSingleDirectoryDbLedgerStorage storage; + + @Before + public void setup() throws Exception { + tmpDir = File.createTempFile("bkTest", ".dir"); + tmpDir.delete(); + tmpDir.mkdir(); + File curDir = BookieImpl.getCurrentDirectory(tmpDir); + BookieImpl.checkDirectoryStructure(curDir); + + ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); + conf.setGcWaitTime(1000); + conf.setLedgerDirNames(new String[] { tmpDir.toString() }); + DiskChecker diskChecker = new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold()); + LedgerDirsManager ledgerDirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(), diskChecker); + LedgerDirsManager indexDirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(), diskChecker); + entryLogger = mock(EntryLogger.class); + + storage = new FailingFlushSingleDirectoryDbLedgerStorage(conf, mock(LedgerManager.class), + ledgerDirsManager, indexDirsManager, entryLogger, NullStatsLogger.INSTANCE, + ByteBufAllocator.DEFAULT, MB, MB, 1, 1024); + } + + @After + public void teardown() throws Exception { + if (storage != null) { + storage.shutdown(); + } + FileUtils.deleteDirectory(tmpDir); + } + + @Test + public void shutdownContinuesCleanupAfterFlushFailure() throws Exception { + storage.shutdown(); + verify(entryLogger).close(); + assertFalse(isGcThreadRunning()); + assertTrue(getCleanupExecutor().isShutdown()); + storage = null; + } + + private boolean isGcThreadRunning() throws Exception { + Field gcThreadField = SingleDirectoryDbLedgerStorage.class.getDeclaredField("gcThread"); + gcThreadField.setAccessible(true); + GarbageCollectorThread gcThread = (GarbageCollectorThread) gcThreadField.get(storage); + + Field runningField = GarbageCollectorThread.class.getDeclaredField("running"); + runningField.setAccessible(true); + return runningField.getBoolean(gcThread); + } + + private ExecutorService getCleanupExecutor() throws Exception { + Field cleanupExecutorField = SingleDirectoryDbLedgerStorage.class.getDeclaredField("cleanupExecutor"); + cleanupExecutorField.setAccessible(true); + return (ExecutorService) cleanupExecutorField.get(storage); + } + + private static class FailingFlushSingleDirectoryDbLedgerStorage extends SingleDirectoryDbLedgerStorage { + + FailingFlushSingleDirectoryDbLedgerStorage(ServerConfiguration conf, LedgerManager ledgerManager, + LedgerDirsManager ledgerDirsManager, LedgerDirsManager indexDirsManager, EntryLogger entryLogger, + StatsLogger statsLogger, ByteBufAllocator allocator, long writeCacheSize, long readCacheSize, + int readAheadCacheBatchSize, long readAheadCacheBatchBytesSize) + throws IOException { + super(conf, ledgerManager, ledgerDirsManager, indexDirsManager, entryLogger, statsLogger, allocator, + writeCacheSize, readCacheSize, readAheadCacheBatchSize, readAheadCacheBatchBytesSize); + } + + @Override + public void flush() throws IOException { + throw new EntryLogWriteException("entry log flush failed", new IOException("injected")); + } + } +} From 2dff579cbd471b3d1c123876e81b412130e49861 Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Wed, 5 Aug 2026 09:46:53 +0800 Subject: [PATCH 8/9] test: release entrylog test buffers (cherry picked from commit 8988984d483f4204936ec1514d785fb52c179538) --- .../bookie/DefaultEntryLogTest.java | 1173 +++++++++-------- .../storage/ldb/DbLedgerStorageTest.java | 61 +- 2 files changed, 693 insertions(+), 541 deletions(-) diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java index 47139cf8b92..64fb678b7c7 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java @@ -229,6 +229,24 @@ private ByteBuf generateEntry(long ledger, long entry, int length) { return bb; } + private static long addEntryAndRelease(LedgerStorage ledgerStorage, ByteBuf entry) + throws IOException, BookieException { + try { + return ledgerStorage.addEntry(entry); + } finally { + ReferenceCountUtil.release(entry); + } + } + + private static void assertEntryCompareEqualsAndRelease(ByteBuf expected, ByteBuf actual) { + try { + Assert.assertEquals(0, expected.compareTo(actual)); + } finally { + ReferenceCountUtil.release(actual); + ReferenceCountUtil.release(expected); + } + } + private static String generateDataString(long ledger, long entry) { return ("ledger-" + ledger + "-" + entry); } @@ -323,24 +341,29 @@ public void testAddEntryFailureOnDiskFull() throws Exception { BookieImpl bookie = new TestBookieImpl(conf); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, bookie.getLedgerDirsManager()); - InterleavedLedgerStorage ledgerStorage = - ((InterleavedLedgerStorage) bookie.ledgerStorage.getUnderlyingLedgerStorage()); - ledgerStorage.entryLogger = entryLogger; - // Create ledgers - ledgerStorage.setMasterKey(1, "key".getBytes()); - ledgerStorage.setMasterKey(2, "key".getBytes()); - ledgerStorage.setMasterKey(3, "key".getBytes()); - // Add entries - ledgerStorage.addEntry(generateEntry(1, 1)); - ledgerStorage.addEntry(generateEntry(2, 1)); - // Add entry with disk full failure simulation - bookie.getLedgerDirsManager().addToFilledDirs(((EntryLogManagerBase) entryLogger.getEntryLogManager()) - .getCurrentLogForLedger(DefaultEntryLogger.UNASSIGNED_LEDGERID).getLogFile().getParentFile()); - ledgerStorage.addEntry(generateEntry(3, 1)); - // Verify written entries - Assert.assertTrue(0 == generateEntry(1, 1).compareTo(ledgerStorage.getEntry(1, 1))); - Assert.assertTrue(0 == generateEntry(2, 1).compareTo(ledgerStorage.getEntry(2, 1))); - Assert.assertTrue(0 == generateEntry(3, 1).compareTo(ledgerStorage.getEntry(3, 1))); + try { + InterleavedLedgerStorage ledgerStorage = + ((InterleavedLedgerStorage) bookie.ledgerStorage.getUnderlyingLedgerStorage()); + ledgerStorage.entryLogger = entryLogger; + // Create ledgers + ledgerStorage.setMasterKey(1, "key".getBytes()); + ledgerStorage.setMasterKey(2, "key".getBytes()); + ledgerStorage.setMasterKey(3, "key".getBytes()); + // Add entries + addEntryAndRelease(ledgerStorage, generateEntry(1, 1)); + addEntryAndRelease(ledgerStorage, generateEntry(2, 1)); + // Add entry with disk full failure simulation + bookie.getLedgerDirsManager().addToFilledDirs(((EntryLogManagerBase) entryLogger.getEntryLogManager()) + .getCurrentLogForLedger(DefaultEntryLogger.UNASSIGNED_LEDGERID).getLogFile().getParentFile()); + addEntryAndRelease(ledgerStorage, generateEntry(3, 1)); + // Verify written entries + assertEntryCompareEqualsAndRelease(generateEntry(1, 1), ledgerStorage.getEntry(1, 1)); + assertEntryCompareEqualsAndRelease(generateEntry(2, 1), ledgerStorage.getEntry(2, 1)); + assertEntryCompareEqualsAndRelease(generateEntry(3, 1), ledgerStorage.getEntry(3, 1)); + } finally { + entryLogger.close(); + bookie.shutdown(); + } } /** @@ -678,9 +701,11 @@ static class LedgerStorageReadTask implements Callable { @Override public Boolean call() throws IOException, BookieException { + ByteBuf expectedByteBuf = null; + ByteBuf actualByteBuf = null; try { - ByteBuf expectedByteBuf = generateEntry(ledgerId, entryId); - ByteBuf actualByteBuf = ledgerStorage.getEntry(ledgerId, entryId); + expectedByteBuf = generateEntry(ledgerId, entryId); + actualByteBuf = ledgerStorage.getEntry(ledgerId, entryId); if (!expectedByteBuf.equals(actualByteBuf)) { LOG.error("Expected Entry: {} Actual Entry: {}", expectedByteBuf.toString(Charset.defaultCharset()), actualByteBuf.toString(Charset.defaultCharset())); @@ -691,6 +716,9 @@ public Boolean call() throws IOException, BookieException { LOG.error("Got Exception for GetEntry call. LedgerId: " + ledgerId + " entryId: " + entryId, e); throw new IOException("Got Exception for GetEntry call. LedgerId: " + ledgerId + " entryId: " + entryId, e); + } finally { + ReferenceCountUtil.release(actualByteBuf); + ReferenceCountUtil.release(expectedByteBuf); } return true; } @@ -922,21 +950,33 @@ public void testFlushIntervalInBytes() throws Exception { */ conf.setEntryLogPerLedgerEnabled(false); DefaultEntryLogger newEntryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManager newEntryLogManager = newEntryLogger.getEntryLogManager(); - Assert.assertEquals("EntryLogManager class type", EntryLogManagerForSingleEntryLog.class, - newEntryLogManager.getClass()); - - ByteBuf buf = newEntryLogger.readEntry(ledgerId, 0L, entry0Position); - long readLedgerId = buf.readLong(); - long readEntryId = buf.readLong(); - Assert.assertEquals("LedgerId", ledgerId, readLedgerId); - Assert.assertEquals("EntryId", 0L, readEntryId); - - buf = newEntryLogger.readEntry(ledgerId, 1L, entry1Position); - readLedgerId = buf.readLong(); - readEntryId = buf.readLong(); - Assert.assertEquals("LedgerId", ledgerId, readLedgerId); - Assert.assertEquals("EntryId", 1L, readEntryId); + try { + EntryLogManager newEntryLogManager = newEntryLogger.getEntryLogManager(); + Assert.assertEquals("EntryLogManager class type", EntryLogManagerForSingleEntryLog.class, + newEntryLogManager.getClass()); + + ByteBuf buf = newEntryLogger.readEntry(ledgerId, 0L, entry0Position); + try { + long readLedgerId = buf.readLong(); + long readEntryId = buf.readLong(); + Assert.assertEquals("LedgerId", ledgerId, readLedgerId); + Assert.assertEquals("EntryId", 0L, readEntryId); + } finally { + ReferenceCountUtil.release(buf); + } + + buf = newEntryLogger.readEntry(ledgerId, 1L, entry1Position); + try { + long readLedgerId = buf.readLong(); + long readEntryId = buf.readLong(); + Assert.assertEquals("LedgerId", ledgerId, readLedgerId); + Assert.assertEquals("EntryId", 1L, readEntryId); + } finally { + ReferenceCountUtil.release(buf); + } + } finally { + newEntryLogger.close(); + } } @Test @@ -945,19 +985,28 @@ public void testReadEntryWithoutLedgerID() throws Exception { // `+ 1` is not a typo: create one more log file than the max number of o cached readers for (int i = 0; i < 10; i++) { ByteBuf e = makeEntry(1L, i, 100); - long loc = entryLogger.addEntry(1L, e.slice()); - locations.add(loc); + try { + long loc = entryLogger.addEntry(1L, e.slice()); + locations.add(loc); + } finally { + ReferenceCountUtil.release(e); + } } entryLogger.flush(); for (Long loc : locations) { int i = locations.indexOf(loc); ByteBuf data = entryLogger.readEntry(loc); - assertEntryEquals(data, makeEntry(1L, i, 100)); - long readLedgerId = data.readLong(); - long readEntryId = data.readLong(); - Assert.assertEquals("LedgerId", 1L, readLedgerId); - Assert.assertEquals("EntryId", i, readEntryId); - ReferenceCountUtil.release(data); + ByteBuf expected = makeEntry(1L, i, 100); + try { + assertEntryEquals(data, expected); + long readLedgerId = data.readLong(); + long readEntryId = data.readLong(); + Assert.assertEquals("LedgerId", 1L, readLedgerId); + Assert.assertEquals("EntryId", i, readEntryId); + } finally { + ReferenceCountUtil.release(data); + ReferenceCountUtil.release(expected); + } } } @@ -976,71 +1025,77 @@ public void testEntryLogManagerInterfaceForEntryLogPerLedger() throws Exception new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) entryLogger - .getEntryLogManager(); + try { + EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) entryLogger + .getEntryLogManager(); - Assert.assertEquals("Number of current active EntryLogs ", 0, entryLogManager.getCopyOfCurrentLogs().size()); - Assert.assertEquals("Number of Rotated Logs ", 0, entryLogManager.getRotatedLogChannels().size()); + Assert.assertEquals("Number of current active EntryLogs ", 0, + entryLogManager.getCopyOfCurrentLogs().size()); + Assert.assertEquals("Number of Rotated Logs ", 0, entryLogManager.getRotatedLogChannels().size()); - int numOfLedgers = 5; - int numOfThreadsPerLedger = 10; - validateLockAcquireAndRelease(numOfLedgers, numOfThreadsPerLedger, entryLogManager); + int numOfLedgers = 5; + int numOfThreadsPerLedger = 10; + validateLockAcquireAndRelease(numOfLedgers, numOfThreadsPerLedger, entryLogManager); - for (long i = 0; i < numOfLedgers; i++) { - entryLogManager.setCurrentLogForLedgerAndAddToRotate(i, - createDummyBufferedLogChannel(entryLogger, i, conf)); - } - - for (long i = 0; i < numOfLedgers; i++) { - Assert.assertEquals("LogChannel for ledger: " + i, entryLogManager.getCurrentLogIfPresent(i), - entryLogManager.getCurrentLogForLedger(i)); - } + for (long i = 0; i < numOfLedgers; i++) { + entryLogManager.setCurrentLogForLedgerAndAddToRotate(i, + createDummyBufferedLogChannel(entryLogger, i, conf)); + } - Assert.assertEquals("Number of current active EntryLogs ", numOfLedgers, - entryLogManager.getCopyOfCurrentLogs().size()); - Assert.assertEquals("Number of Rotated Logs ", 0, entryLogManager.getRotatedLogChannels().size()); + for (long i = 0; i < numOfLedgers; i++) { + Assert.assertEquals("LogChannel for ledger: " + i, entryLogManager.getCurrentLogIfPresent(i), + entryLogManager.getCurrentLogForLedger(i)); + } - for (long i = 0; i < numOfLedgers; i++) { - entryLogManager.setCurrentLogForLedgerAndAddToRotate(i, - createDummyBufferedLogChannel(entryLogger, numOfLedgers + i, conf)); - } + Assert.assertEquals("Number of current active EntryLogs ", numOfLedgers, + entryLogManager.getCopyOfCurrentLogs().size()); + Assert.assertEquals("Number of Rotated Logs ", 0, entryLogManager.getRotatedLogChannels().size()); - /* - * since new entryLogs are set for all the ledgers, previous entrylogs would be added to rotatedLogChannels - */ - Assert.assertEquals("Number of current active EntryLogs ", numOfLedgers, - entryLogManager.getCopyOfCurrentLogs().size()); - Assert.assertEquals("Number of Rotated Logs ", numOfLedgers, - entryLogManager.getRotatedLogChannels().size()); - - for (long i = 0; i < numOfLedgers; i++) { - entryLogManager.setCurrentLogForLedgerAndAddToRotate(i, - createDummyBufferedLogChannel(entryLogger, 2 * numOfLedgers + i, conf)); - } + for (long i = 0; i < numOfLedgers; i++) { + entryLogManager.setCurrentLogForLedgerAndAddToRotate(i, + createDummyBufferedLogChannel(entryLogger, numOfLedgers + i, conf)); + } - /* - * again since new entryLogs are set for all the ledgers, previous entrylogs would be added to - * rotatedLogChannels - */ - Assert.assertEquals("Number of current active EntryLogs ", numOfLedgers, - entryLogManager.getCopyOfCurrentLogs().size()); - Assert.assertEquals("Number of Rotated Logs ", 2 * numOfLedgers, - entryLogManager.getRotatedLogChannels().size()); + /* + * since new entryLogs are set for all the ledgers, previous entrylogs would be added to rotatedLogChannels + */ + Assert.assertEquals("Number of current active EntryLogs ", numOfLedgers, + entryLogManager.getCopyOfCurrentLogs().size()); + Assert.assertEquals("Number of Rotated Logs ", numOfLedgers, + entryLogManager.getRotatedLogChannels().size()); + + for (long i = 0; i < numOfLedgers; i++) { + entryLogManager.setCurrentLogForLedgerAndAddToRotate(i, + createDummyBufferedLogChannel(entryLogger, 2 * numOfLedgers + i, conf)); + } - for (BufferedLogChannel logChannel : entryLogManager.getRotatedLogChannels()) { - entryLogManager.getRotatedLogChannels().remove(logChannel); - } - Assert.assertEquals("Number of Rotated Logs ", 0, entryLogManager.getRotatedLogChannels().size()); + /* + * again since new entryLogs are set for all the ledgers, previous entrylogs would be added to + * rotatedLogChannels + */ + Assert.assertEquals("Number of current active EntryLogs ", numOfLedgers, + entryLogManager.getCopyOfCurrentLogs().size()); + Assert.assertEquals("Number of Rotated Logs ", 2 * numOfLedgers, + entryLogManager.getRotatedLogChannels().size()); + + for (BufferedLogChannel logChannel : new ArrayList<>(entryLogManager.getRotatedLogChannels())) { + entryLogManager.getRotatedLogChannels().remove(logChannel); + logChannel.close(); + } + Assert.assertEquals("Number of Rotated Logs ", 0, entryLogManager.getRotatedLogChannels().size()); - // entrylogid is sequential - for (long i = 0; i < numOfLedgers; i++) { - assertEquals("EntryLogid for Ledger " + i, 2 * numOfLedgers + i, - entryLogManager.getCurrentLogForLedger(i).getLogId()); - } + // entrylogid is sequential + for (long i = 0; i < numOfLedgers; i++) { + assertEquals("EntryLogid for Ledger " + i, 2 * numOfLedgers + i, + entryLogManager.getCurrentLogForLedger(i).getLogId()); + } - for (long i = 2 * numOfLedgers; i < (3 * numOfLedgers); i++) { - assertTrue("EntryLog with logId: " + i + " should be present", - entryLogManager.getCurrentLogIfPresent(i) != null); + for (long i = 2 * numOfLedgers; i < (3 * numOfLedgers); i++) { + assertTrue("EntryLog with logId: " + i + " should be present", + entryLogManager.getCurrentLogIfPresent(i) != null); + } + } finally { + entryLogger.close(); } } @@ -1129,34 +1184,39 @@ public void testEntryLogManagerExpiryRemoval() throws Exception { new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerForEntryLogPerLedger entryLogManager = - (EntryLogManagerForEntryLogPerLedger) entryLogger.getEntryLogManager(); + try { + EntryLogManagerForEntryLogPerLedger entryLogManager = + (EntryLogManagerForEntryLogPerLedger) entryLogger.getEntryLogManager(); - long ledgerId = 0L; + long ledgerId = 0L; + + BufferedLogChannel logChannel = createDummyBufferedLogChannel(entryLogger, 0, conf); + entryLogManager.setCurrentLogForLedgerAndAddToRotate(ledgerId, logChannel); - BufferedLogChannel logChannel = createDummyBufferedLogChannel(entryLogger, 0, conf); - entryLogManager.setCurrentLogForLedgerAndAddToRotate(ledgerId, logChannel); + BufferedLogChannel currentLogForLedger = entryLogManager.getCurrentLogForLedger(ledgerId); + assertEquals("LogChannel for ledger " + ledgerId + " should match", logChannel, currentLogForLedger); - BufferedLogChannel currentLogForLedger = entryLogManager.getCurrentLogForLedger(ledgerId); - assertEquals("LogChannel for ledger " + ledgerId + " should match", logChannel, currentLogForLedger); + Thread.sleep(evictionPeriod * 1000 + 100); + entryLogManager.doEntryLogMapCleanup(); - Thread.sleep(evictionPeriod * 1000 + 100); - entryLogManager.doEntryLogMapCleanup(); + /* + * since for more than evictionPeriod, that ledger is not accessed and cache is cleaned up, mapping for that + * ledger should not be available anymore + */ + currentLogForLedger = entryLogManager.getCurrentLogForLedger(ledgerId); + assertEquals("LogChannel for ledger " + ledgerId + " should be null", null, currentLogForLedger); + Assert.assertEquals("Number of current active EntryLogs ", 0, + entryLogManager.getCopyOfCurrentLogs().size()); + Assert.assertEquals("Number of rotated EntryLogs ", 1, entryLogManager.getRotatedLogChannels().size()); + Assert.assertTrue("CopyOfRotatedLogChannels should contain the created LogChannel", + entryLogManager.getRotatedLogChannels().contains(logChannel)); - /* - * since for more than evictionPeriod, that ledger is not accessed and cache is cleaned up, mapping for that - * ledger should not be available anymore - */ - currentLogForLedger = entryLogManager.getCurrentLogForLedger(ledgerId); - assertEquals("LogChannel for ledger " + ledgerId + " should be null", null, currentLogForLedger); - Assert.assertEquals("Number of current active EntryLogs ", 0, entryLogManager.getCopyOfCurrentLogs().size()); - Assert.assertEquals("Number of rotated EntryLogs ", 1, entryLogManager.getRotatedLogChannels().size()); - Assert.assertTrue("CopyOfRotatedLogChannels should contain the created LogChannel", - entryLogManager.getRotatedLogChannels().contains(logChannel)); - - Assert.assertTrue("since mapentry must have been evicted, it should be null", - (entryLogManager.getCacheAsMap().get(ledgerId) == null) - || (entryLogManager.getCacheAsMap().get(ledgerId).getEntryLogWithDirInfo() == null)); + Assert.assertTrue("since mapentry must have been evicted, it should be null", + (entryLogManager.getCacheAsMap().get(ledgerId) == null) + || (entryLogManager.getCacheAsMap().get(ledgerId).getEntryLogWithDirInfo() == null)); + } finally { + entryLogger.close(); + } } /* @@ -1200,37 +1260,45 @@ public void testLongLedgerIdsWithEntryLogPerLedger() throws Exception { new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) entryLogger - .getEntryLogManager(); + try { + EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) entryLogger + .getEntryLogManager(); - int numOfLedgers = 5; - int numOfEntries = 4; - long[][] pos = new long[numOfLedgers][numOfEntries]; - for (int i = 0; i < numOfLedgers; i++) { - long ledgerId = Long.MAX_VALUE - i; - entryLogManager.createNewLog(ledgerId); - for (int entryId = 0; entryId < numOfEntries; entryId++) { - pos[i][entryId] = entryLogger.addEntry(ledgerId, generateEntry(ledgerId, entryId).nioBuffer()); + int numOfLedgers = 5; + int numOfEntries = 4; + long[][] pos = new long[numOfLedgers][numOfEntries]; + for (int i = 0; i < numOfLedgers; i++) { + long ledgerId = Long.MAX_VALUE - i; + entryLogManager.createNewLog(ledgerId); + for (int entryId = 0; entryId < numOfEntries; entryId++) { + pos[i][entryId] = entryLogger.addEntry(ledgerId, generateEntry(ledgerId, entryId).nioBuffer()); + } } - } - /* - * do checkpoint to make sure entrylog files are persisted - */ - entryLogger.checkpoint(); + /* + * do checkpoint to make sure entrylog files are persisted + */ + entryLogger.checkpoint(); - for (int i = 0; i < numOfLedgers; i++) { - long ledgerId = Long.MAX_VALUE - i; - for (int entryId = 0; entryId < numOfEntries; entryId++) { - String expectedValue = generateDataString(ledgerId, entryId); - ByteBuf buf = entryLogger.readEntry(ledgerId, entryId, pos[i][entryId]); - long readLedgerId = buf.readLong(); - long readEntryId = buf.readLong(); - byte[] readData = new byte[buf.readableBytes()]; - buf.readBytes(readData); - assertEquals("LedgerId ", ledgerId, readLedgerId); - assertEquals("EntryId ", entryId, readEntryId); - assertEquals("Entry Data ", expectedValue, new String(readData)); + for (int i = 0; i < numOfLedgers; i++) { + long ledgerId = Long.MAX_VALUE - i; + for (int entryId = 0; entryId < numOfEntries; entryId++) { + String expectedValue = generateDataString(ledgerId, entryId); + ByteBuf buf = entryLogger.readEntry(ledgerId, entryId, pos[i][entryId]); + try { + long readLedgerId = buf.readLong(); + long readEntryId = buf.readLong(); + byte[] readData = new byte[buf.readableBytes()]; + buf.readBytes(readData); + assertEquals("LedgerId ", ledgerId, readLedgerId); + assertEquals("EntryId ", entryId, readEntryId); + assertEquals("Entry Data ", expectedValue, new String(readData)); + } finally { + ReferenceCountUtil.release(buf); + } + } } + } finally { + entryLogger.close(); } } @@ -1252,47 +1320,51 @@ public void testAppendLedgersMapOnCacheRemoval() throws Exception { new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) entryLogger - .getEntryLogManager(); + try { + EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) entryLogger + .getEntryLogManager(); - long ledgerId = 0L; - entryLogManager.createNewLog(ledgerId); - int entrySize = 200; - int numOfEntries = 4; - for (int i = 0; i < numOfEntries; i++) { - entryLogger.addEntry(ledgerId, generateEntry(ledgerId, i, entrySize)); - } + long ledgerId = 0L; + entryLogManager.createNewLog(ledgerId); + int entrySize = 200; + int numOfEntries = 4; + for (int i = 0; i < numOfEntries; i++) { + entryLogger.addEntry(ledgerId, generateEntry(ledgerId, i, entrySize)); + } - BufferedLogChannel logChannelForledger = entryLogManager.getCurrentLogForLedger(ledgerId); - long logIdOfLedger = logChannelForledger.getLogId(); - /* - * do checkpoint to make sure entrylog files are persisted - */ - entryLogger.checkpoint(); + BufferedLogChannel logChannelForledger = entryLogManager.getCurrentLogForLedger(ledgerId); + long logIdOfLedger = logChannelForledger.getLogId(); + /* + * do checkpoint to make sure entrylog files are persisted + */ + entryLogger.checkpoint(); - try { - entryLogger.extractEntryLogMetadataFromIndex(logIdOfLedger); - } catch (IOException ie) { - // expected because appendLedgersMap wouldn't have been called - } + try { + entryLogger.extractEntryLogMetadataFromIndex(logIdOfLedger); + } catch (IOException ie) { + // expected because appendLedgersMap wouldn't have been called + } - /* - * create entrylogs for more ledgers, so that ledgerIdEntryLogMap would - * reach its limit and remove the oldest entrylog. - */ - for (int i = 1; i <= cacheMaximumSize; i++) { - entryLogManager.createNewLog(i); - } - /* - * do checkpoint to make sure entrylog files are persisted - */ - entryLogger.checkpoint(); + /* + * create entrylogs for more ledgers, so that ledgerIdEntryLogMap would + * reach its limit and remove the oldest entrylog. + */ + for (int i = 1; i <= cacheMaximumSize; i++) { + entryLogManager.createNewLog(i); + } + /* + * do checkpoint to make sure entrylog files are persisted + */ + entryLogger.checkpoint(); - EntryLogMetadata entryLogMetadata = entryLogger.extractEntryLogMetadataFromIndex(logIdOfLedger); - ConcurrentLongLongHashMap ledgersMap = entryLogMetadata.getLedgersMap(); - Assert.assertEquals("There should be only one entry in entryLogMetadata", 1, ledgersMap.size()); - Assert.assertTrue("Usage should be 1", Double.compare(1.0, entryLogMetadata.getUsage()) == 0); - Assert.assertEquals("Total size of entries", (entrySize + 4) * numOfEntries, ledgersMap.get(ledgerId)); + EntryLogMetadata entryLogMetadata = entryLogger.extractEntryLogMetadataFromIndex(logIdOfLedger); + ConcurrentLongLongHashMap ledgersMap = entryLogMetadata.getLedgersMap(); + Assert.assertEquals("There should be only one entry in entryLogMetadata", 1, ledgersMap.size()); + Assert.assertTrue("Usage should be 1", Double.compare(1.0, entryLogMetadata.getUsage()) == 0); + Assert.assertEquals("Total size of entries", (entrySize + 4) * numOfEntries, ledgersMap.get(ledgerId)); + } finally { + entryLogger.close(); + } } @Test @@ -1361,36 +1433,42 @@ public void testExpiryRemovalByAccessingOnAnotherThread() throws Exception { new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerForEntryLogPerLedger entryLogManager = - (EntryLogManagerForEntryLogPerLedger) entryLogger.getEntryLogManager(); + try { + EntryLogManagerForEntryLogPerLedger entryLogManager = + (EntryLogManagerForEntryLogPerLedger) entryLogger.getEntryLogManager(); - long ledgerId = 0L; + long ledgerId = 0L; - BufferedLogChannel newLogChannel = createDummyBufferedLogChannel(entryLogger, 1, conf); - entryLogManager.setCurrentLogForLedgerAndAddToRotate(ledgerId, newLogChannel); + BufferedLogChannel newLogChannel = createDummyBufferedLogChannel(entryLogger, 1, conf); + entryLogManager.setCurrentLogForLedgerAndAddToRotate(ledgerId, newLogChannel); - Thread t = new Thread() { - public void run() { - try { - Thread.sleep((evictionPeriod * 1000) / 2); - entryLogManager.getCurrentLogForLedger(ledgerId); - } catch (InterruptedException | IOException e) { + Thread t = new Thread() { + public void run() { + try { + Thread.sleep((evictionPeriod * 1000) / 2); + entryLogManager.getCurrentLogForLedger(ledgerId); + } catch (InterruptedException | IOException e) { + } } - } - }; + }; - t.start(); - Thread.sleep(evictionPeriod * 1000 + 100); - entryLogManager.doEntryLogMapCleanup(); + t.start(); + Thread.sleep(evictionPeriod * 1000 + 100); + entryLogManager.doEntryLogMapCleanup(); + t.join(); - /* - * in this scenario, that ledger is accessed by other thread during - * eviction period time, so it should not be evicted. - */ - BufferedLogChannel currentLogForLedger = entryLogManager.getCurrentLogForLedger(ledgerId); - assertEquals("LogChannel for ledger " + ledgerId, newLogChannel, currentLogForLedger); - Assert.assertEquals("Number of current active EntryLogs ", 1, entryLogManager.getCopyOfCurrentLogs().size()); - Assert.assertEquals("Number of rotated EntryLogs ", 0, entryLogManager.getRotatedLogChannels().size()); + /* + * in this scenario, that ledger is accessed by other thread during + * eviction period time, so it should not be evicted. + */ + BufferedLogChannel currentLogForLedger = entryLogManager.getCurrentLogForLedger(ledgerId); + assertEquals("LogChannel for ledger " + ledgerId, newLogChannel, currentLogForLedger); + Assert.assertEquals("Number of current active EntryLogs ", 1, + entryLogManager.getCopyOfCurrentLogs().size()); + Assert.assertEquals("Number of rotated EntryLogs ", 0, entryLogManager.getRotatedLogChannels().size()); + } finally { + entryLogger.close(); + } } /** @@ -1415,59 +1493,65 @@ public void testExpiryRemovalByAccessingNonCacheRelatedMethods() throws Exceptio new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerForEntryLogPerLedger entryLogManager = - (EntryLogManagerForEntryLogPerLedger) entryLogger.getEntryLogManager(); - - long ledgerId = 0L; - - BufferedLogChannel newLogChannel = createDummyBufferedLogChannel(entryLogger, 1, conf); - entryLogManager.setCurrentLogForLedgerAndAddToRotate(ledgerId, newLogChannel); - - AtomicBoolean exceptionOccured = new AtomicBoolean(false); - Thread t = new Thread() { - public void run() { - try { - Thread.sleep(500); - /* - * any of the following operations should not access entry - * of 'ledgerId' in the cache - */ - entryLogManager.getCopyOfCurrentLogs(); - entryLogManager.getRotatedLogChannels(); - entryLogManager.getCurrentLogIfPresent(newLogChannel.getLogId()); - entryLogManager.getDirForNextEntryLog(ledgerDirsManager.getWritableLedgerDirs()); - long newLedgerId = 100; - BufferedLogChannel logChannelForNewLedger = - createDummyBufferedLogChannel(entryLogger, newLedgerId, conf); - entryLogManager.setCurrentLogForLedgerAndAddToRotate(newLedgerId, logChannelForNewLedger); - entryLogManager.getCurrentLogIfPresent(newLedgerId); - } catch (Exception e) { - LOG.error("Got Exception in thread", e); - exceptionOccured.set(true); + try { + EntryLogManagerForEntryLogPerLedger entryLogManager = + (EntryLogManagerForEntryLogPerLedger) entryLogger.getEntryLogManager(); + + long ledgerId = 0L; + + BufferedLogChannel newLogChannel = createDummyBufferedLogChannel(entryLogger, 1, conf); + entryLogManager.setCurrentLogForLedgerAndAddToRotate(ledgerId, newLogChannel); + + AtomicBoolean exceptionOccured = new AtomicBoolean(false); + Thread t = new Thread() { + public void run() { + try { + Thread.sleep(500); + /* + * any of the following operations should not access entry + * of 'ledgerId' in the cache + */ + entryLogManager.getCopyOfCurrentLogs(); + entryLogManager.getRotatedLogChannels(); + entryLogManager.getCurrentLogIfPresent(newLogChannel.getLogId()); + entryLogManager.getDirForNextEntryLog(ledgerDirsManager.getWritableLedgerDirs()); + long newLedgerId = 100; + BufferedLogChannel logChannelForNewLedger = + createDummyBufferedLogChannel(entryLogger, newLedgerId, conf); + entryLogManager.setCurrentLogForLedgerAndAddToRotate(newLedgerId, logChannelForNewLedger); + entryLogManager.getCurrentLogIfPresent(newLedgerId); + } catch (Exception e) { + LOG.error("Got Exception in thread", e); + exceptionOccured.set(true); + } } - } - }; + }; - t.start(); - Thread.sleep(evictionPeriod * 1000 + 100); - entryLogManager.doEntryLogMapCleanup(); - Assert.assertFalse("Exception occured in thread, which is not expected", exceptionOccured.get()); + t.start(); + Thread.sleep(evictionPeriod * 1000 + 100); + entryLogManager.doEntryLogMapCleanup(); + t.join(); + Assert.assertFalse("Exception occured in thread, which is not expected", exceptionOccured.get()); - /* - * since for more than evictionPeriod, that ledger is not accessed and cache is cleaned up, mapping for that - * ledger should not be available anymore - */ - BufferedLogChannel currentLogForLedger = entryLogManager.getCurrentLogForLedger(ledgerId); - assertEquals("LogChannel for ledger " + ledgerId + " should be null", null, currentLogForLedger); - // expected number of current active entryLogs is 1 since we created entrylog for 'newLedgerId' - Assert.assertEquals("Number of current active EntryLogs ", 1, entryLogManager.getCopyOfCurrentLogs().size()); - Assert.assertEquals("Number of rotated EntryLogs ", 1, entryLogManager.getRotatedLogChannels().size()); - Assert.assertTrue("CopyOfRotatedLogChannels should contain the created LogChannel", - entryLogManager.getRotatedLogChannels().contains(newLogChannel)); - - Assert.assertTrue("since mapentry must have been evicted, it should be null", - (entryLogManager.getCacheAsMap().get(ledgerId) == null) - || (entryLogManager.getCacheAsMap().get(ledgerId).getEntryLogWithDirInfo() == null)); + /* + * since for more than evictionPeriod, that ledger is not accessed and cache is cleaned up, mapping for that + * ledger should not be available anymore + */ + BufferedLogChannel currentLogForLedger = entryLogManager.getCurrentLogForLedger(ledgerId); + assertEquals("LogChannel for ledger " + ledgerId + " should be null", null, currentLogForLedger); + // expected number of current active entryLogs is 1 since we created entrylog for 'newLedgerId' + Assert.assertEquals("Number of current active EntryLogs ", 1, + entryLogManager.getCopyOfCurrentLogs().size()); + Assert.assertEquals("Number of rotated EntryLogs ", 1, entryLogManager.getRotatedLogChannels().size()); + Assert.assertTrue("CopyOfRotatedLogChannels should contain the created LogChannel", + entryLogManager.getRotatedLogChannels().contains(newLogChannel)); + + Assert.assertTrue("since mapentry must have been evicted, it should be null", + (entryLogManager.getCacheAsMap().get(ledgerId) == null) + || (entryLogManager.getCacheAsMap().get(ledgerId).getEntryLogWithDirInfo() == null)); + } finally { + entryLogger.close(); + } } /* @@ -1483,75 +1567,79 @@ public void testEntryLogManagerForEntryLogPerLedger() throws Exception { LedgerDirsManager ledgerDirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(), new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerBase entryLogManager = (EntryLogManagerBase) entryLogger.getEntryLogManager(); - Assert.assertEquals("EntryLogManager class type", EntryLogManagerForEntryLogPerLedger.class, - entryLogManager.getClass()); + try { + EntryLogManagerBase entryLogManager = (EntryLogManagerBase) entryLogger.getEntryLogManager(); + Assert.assertEquals("EntryLogManager class type", EntryLogManagerForEntryLogPerLedger.class, + entryLogManager.getClass()); - int numOfActiveLedgers = 20; - int numEntries = 5; + int numOfActiveLedgers = 20; + int numEntries = 5; + + for (int j = 0; j < numEntries; j++) { + for (long i = 0; i < numOfActiveLedgers; i++) { + entryLogger.addEntry(i, generateEntry(i, j)); + } + } - for (int j = 0; j < numEntries; j++) { for (long i = 0; i < numOfActiveLedgers; i++) { - entryLogger.addEntry(i, generateEntry(i, j)); + BufferedLogChannel logChannel = entryLogManager.getCurrentLogForLedger(i); + Assert.assertTrue("unpersistedBytes should be greater than LOGFILE_HEADER_SIZE", + logChannel.getUnpersistedBytes() > DefaultEntryLogger.LOGFILE_HEADER_SIZE); } - } - for (long i = 0; i < numOfActiveLedgers; i++) { - BufferedLogChannel logChannel = entryLogManager.getCurrentLogForLedger(i); - Assert.assertTrue("unpersistedBytes should be greater than LOGFILE_HEADER_SIZE", - logChannel.getUnpersistedBytes() > DefaultEntryLogger.LOGFILE_HEADER_SIZE); - } + for (long i = 0; i < numOfActiveLedgers; i++) { + entryLogManager.createNewLog(i); + } - for (long i = 0; i < numOfActiveLedgers; i++) { - entryLogManager.createNewLog(i); - } + /* + * since we created new entrylog for all the activeLedgers, entrylogs of all the ledgers + * should be rotated and hence the size of copyOfRotatedLogChannels should be numOfActiveLedgers + */ + List rotatedLogs = entryLogManager.getRotatedLogChannels(); + Assert.assertEquals("Number of rotated entrylogs", numOfActiveLedgers, rotatedLogs.size()); - /* - * since we created new entrylog for all the activeLedgers, entrylogs of all the ledgers - * should be rotated and hence the size of copyOfRotatedLogChannels should be numOfActiveLedgers - */ - List rotatedLogs = entryLogManager.getRotatedLogChannels(); - Assert.assertEquals("Number of rotated entrylogs", numOfActiveLedgers, rotatedLogs.size()); + /* + * Since newlog is created for all slots, so they are moved to rotated logs and hence unpersistedBytes of all + * the slots should be just EntryLogger.LOGFILE_HEADER_SIZE + * + */ + for (long i = 0; i < numOfActiveLedgers; i++) { + BufferedLogChannel logChannel = entryLogManager.getCurrentLogForLedger(i); + Assert.assertEquals("unpersistedBytes should be LOGFILE_HEADER_SIZE", + DefaultEntryLogger.LOGFILE_HEADER_SIZE, logChannel.getUnpersistedBytes()); + } - /* - * Since newlog is created for all slots, so they are moved to rotated logs and hence unpersistedBytes of all - * the slots should be just EntryLogger.LOGFILE_HEADER_SIZE - * - */ - for (long i = 0; i < numOfActiveLedgers; i++) { - BufferedLogChannel logChannel = entryLogManager.getCurrentLogForLedger(i); - Assert.assertEquals("unpersistedBytes should be LOGFILE_HEADER_SIZE", - DefaultEntryLogger.LOGFILE_HEADER_SIZE, logChannel.getUnpersistedBytes()); - } + for (int j = numEntries; j < 2 * numEntries; j++) { + for (long i = 0; i < numOfActiveLedgers; i++) { + entryLogger.addEntry(i, generateEntry(i, j)); + } + } - for (int j = numEntries; j < 2 * numEntries; j++) { for (long i = 0; i < numOfActiveLedgers; i++) { - entryLogger.addEntry(i, generateEntry(i, j)); + BufferedLogChannel logChannel = entryLogManager.getCurrentLogForLedger(i); + Assert.assertTrue("unpersistedBytes should be greater than LOGFILE_HEADER_SIZE", + logChannel.getUnpersistedBytes() > DefaultEntryLogger.LOGFILE_HEADER_SIZE); } - } - - for (long i = 0; i < numOfActiveLedgers; i++) { - BufferedLogChannel logChannel = entryLogManager.getCurrentLogForLedger(i); - Assert.assertTrue("unpersistedBytes should be greater than LOGFILE_HEADER_SIZE", - logChannel.getUnpersistedBytes() > DefaultEntryLogger.LOGFILE_HEADER_SIZE); - } - Assert.assertEquals("LeastUnflushedloggerID", 0, entryLogger.getLeastUnflushedLogId()); + Assert.assertEquals("LeastUnflushedloggerID", 0, entryLogger.getLeastUnflushedLogId()); - /* - * here flush is called so all the rotatedLogChannels should be file closed and there shouldn't be any - * rotatedlogchannel and also leastUnflushedLogId should be advanced to numOfActiveLedgers - */ - entryLogger.flush(); - Assert.assertEquals("Number of rotated entrylogs", 0, entryLogManager.getRotatedLogChannels().size()); - Assert.assertEquals("LeastUnflushedloggerID", numOfActiveLedgers, entryLogger.getLeastUnflushedLogId()); + /* + * here flush is called so all the rotatedLogChannels should be file closed and there shouldn't be any + * rotatedlogchannel and also leastUnflushedLogId should be advanced to numOfActiveLedgers + */ + entryLogger.flush(); + Assert.assertEquals("Number of rotated entrylogs", 0, entryLogManager.getRotatedLogChannels().size()); + Assert.assertEquals("LeastUnflushedloggerID", numOfActiveLedgers, entryLogger.getLeastUnflushedLogId()); - /* - * after flush (flushCurrentLogs) unpersistedBytes should be 0. - */ - for (long i = 0; i < numOfActiveLedgers; i++) { - BufferedLogChannel logChannel = entryLogManager.getCurrentLogForLedger(i); - Assert.assertEquals("unpersistedBytes should be 0", 0L, logChannel.getUnpersistedBytes()); + /* + * after flush (flushCurrentLogs) unpersistedBytes should be 0. + */ + for (long i = 0; i < numOfActiveLedgers; i++) { + BufferedLogChannel logChannel = entryLogManager.getCurrentLogForLedger(i); + Assert.assertEquals("unpersistedBytes should be 0", 0L, logChannel.getUnpersistedBytes()); + } + } finally { + entryLogger.close(); } } @@ -1585,67 +1673,79 @@ public void testReadAddCallsOfMultipleEntryLogs() throws Exception { new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerBase entryLogManagerBase = ((EntryLogManagerBase) entryLogger.getEntryLogManager()); - - int numOfActiveLedgers = 10; - int numEntries = 10; - long[][] positions = new long[numOfActiveLedgers][]; - for (int i = 0; i < numOfActiveLedgers; i++) { - positions[i] = new long[numEntries]; - } + try { + EntryLogManagerBase entryLogManagerBase = ((EntryLogManagerBase) entryLogger.getEntryLogManager()); - /* - * addentries to the ledgers - */ - for (int j = 0; j < numEntries; j++) { + int numOfActiveLedgers = 10; + int numEntries = 10; + long[][] positions = new long[numOfActiveLedgers][]; for (int i = 0; i < numOfActiveLedgers; i++) { - positions[i][j] = entryLogger.addEntry((long) i, generateEntry(i, j)); - long entryLogId = (positions[i][j] >> 32L); - /** - * - * Though EntryLogFilePreAllocation is enabled, Since things are not done concurrently here, - * entryLogIds will be sequential. - */ - Assert.assertEquals("EntryLogId for ledger: " + i, i, entryLogId); + positions[i] = new long[numEntries]; } - } - /* - * read the entries which are written - */ - for (int j = 0; j < numEntries; j++) { - for (int i = 0; i < numOfActiveLedgers; i++) { - String expectedValue = "ledger-" + i + "-" + j; - ByteBuf buf = entryLogger.readEntry(i, j, positions[i][j]); - long ledgerId = buf.readLong(); - long entryId = buf.readLong(); - byte[] data = new byte[buf.readableBytes()]; - buf.readBytes(data); - assertEquals("LedgerId ", i, ledgerId); - assertEquals("EntryId ", j, entryId); - assertEquals("Entry Data ", expectedValue, new String(data)); + /* + * addentries to the ledgers + */ + for (int j = 0; j < numEntries; j++) { + for (int i = 0; i < numOfActiveLedgers; i++) { + positions[i][j] = entryLogger.addEntry((long) i, generateEntry(i, j)); + long entryLogId = (positions[i][j] >> 32L); + /** + * + * Though EntryLogFilePreAllocation is enabled, Since things are not done concurrently here, + * entryLogIds will be sequential. + */ + Assert.assertEquals("EntryLogId for ledger: " + i, i, entryLogId); + } } - } - for (long i = 0; i < numOfActiveLedgers; i++) { - entryLogManagerBase.createNewLog(i); - } + /* + * read the entries which are written + */ + for (int j = 0; j < numEntries; j++) { + for (int i = 0; i < numOfActiveLedgers; i++) { + String expectedValue = "ledger-" + i + "-" + j; + ByteBuf buf = entryLogger.readEntry(i, j, positions[i][j]); + try { + long ledgerId = buf.readLong(); + long entryId = buf.readLong(); + byte[] data = new byte[buf.readableBytes()]; + buf.readBytes(data); + assertEquals("LedgerId ", i, ledgerId); + assertEquals("EntryId ", j, entryId); + assertEquals("Entry Data ", expectedValue, new String(data)); + } finally { + ReferenceCountUtil.release(buf); + } + } + } - entryLogManagerBase.flushRotatedLogs(); + for (long i = 0; i < numOfActiveLedgers; i++) { + entryLogManagerBase.createNewLog(i); + } - // reading after flush of rotatedlogs - for (int j = 0; j < numEntries; j++) { - for (int i = 0; i < numOfActiveLedgers; i++) { - String expectedValue = "ledger-" + i + "-" + j; - ByteBuf buf = entryLogger.readEntry(i, j, positions[i][j]); - long ledgerId = buf.readLong(); - long entryId = buf.readLong(); - byte[] data = new byte[buf.readableBytes()]; - buf.readBytes(data); - assertEquals("LedgerId ", i, ledgerId); - assertEquals("EntryId ", j, entryId); - assertEquals("Entry Data ", expectedValue, new String(data)); + entryLogManagerBase.flushRotatedLogs(); + + // reading after flush of rotatedlogs + for (int j = 0; j < numEntries; j++) { + for (int i = 0; i < numOfActiveLedgers; i++) { + String expectedValue = "ledger-" + i + "-" + j; + ByteBuf buf = entryLogger.readEntry(i, j, positions[i][j]); + try { + long ledgerId = buf.readLong(); + long entryId = buf.readLong(); + byte[] data = new byte[buf.readableBytes()]; + buf.readBytes(data); + assertEquals("LedgerId ", i, ledgerId); + assertEquals("EntryId ", j, entryId); + assertEquals("Entry Data ", expectedValue, new String(data)); + } finally { + ReferenceCountUtil.release(buf); + } + } } + } finally { + entryLogger.close(); } } @@ -1664,9 +1764,11 @@ class ReadTask implements Callable { @Override public Boolean call() throws IOException { + ByteBuf expectedByteBuf = null; + ByteBuf actualByteBuf = null; try { - ByteBuf expectedByteBuf = generateEntry(ledgerId, entryId); - ByteBuf actualByteBuf = entryLogger.readEntry(ledgerId, entryId, position); + expectedByteBuf = generateEntry(ledgerId, entryId); + actualByteBuf = entryLogger.readEntry(ledgerId, entryId, position); if (!expectedByteBuf.equals(actualByteBuf)) { LOG.error("Expected Entry: {} Actual Entry: {}", expectedByteBuf.toString(Charset.defaultCharset()), actualByteBuf.toString(Charset.defaultCharset())); @@ -1677,6 +1779,9 @@ public Boolean call() throws IOException { LOG.error("Got Exception for GetEntry call. LedgerId: " + ledgerId + " entryId: " + entryId, e); throw new IOException("Got Exception for GetEntry call. LedgerId: " + ledgerId + " entryId: " + entryId, e); + } finally { + ReferenceCountUtil.release(actualByteBuf); + ReferenceCountUtil.release(expectedByteBuf); } return true; } @@ -1695,51 +1800,59 @@ public void testConcurrentReadCallsAfterEntryLogsAreRotated() throws Exception { new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - int numOfActiveLedgers = 15; - int numEntries = 2000; - final AtomicLongArray positions = new AtomicLongArray(numOfActiveLedgers * numEntries); - EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) - entryLogger.getEntryLogManager(); + ExecutorService executor = null; + try { + int numOfActiveLedgers = 15; + int numEntries = 2000; + final AtomicLongArray positions = new AtomicLongArray(numOfActiveLedgers * numEntries); + EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) + entryLogger.getEntryLogManager(); - for (int i = 0; i < numOfActiveLedgers; i++) { - for (int j = 0; j < numEntries; j++) { - positions.set(i * numEntries + j, entryLogger.addEntry((long) i, generateEntry(i, j))); - long entryLogId = (positions.get(i * numEntries + j) >> 32L); - /** - * - * Though EntryLogFilePreAllocation is enabled, Since things are not done concurrently here, entryLogIds - * will be sequential. - */ - Assert.assertEquals("EntryLogId for ledger: " + i, i, entryLogId); + for (int i = 0; i < numOfActiveLedgers; i++) { + for (int j = 0; j < numEntries; j++) { + positions.set(i * numEntries + j, entryLogger.addEntry((long) i, generateEntry(i, j))); + long entryLogId = (positions.get(i * numEntries + j) >> 32L); + /** + * + * Though EntryLogFilePreAllocation is enabled, Since things are not done concurrently here, + * entryLogIds will be sequential. + */ + Assert.assertEquals("EntryLogId for ledger: " + i, i, entryLogId); + } } - } - for (long i = 0; i < numOfActiveLedgers; i++) { - entryLogManager.createNewLog(i); - } - entryLogManager.flushRotatedLogs(); + for (long i = 0; i < numOfActiveLedgers; i++) { + entryLogManager.createNewLog(i); + } + entryLogManager.flushRotatedLogs(); - // reading after flush of rotatedlogs - ArrayList readTasks = new ArrayList(); - for (int i = 0; i < numOfActiveLedgers; i++) { - for (int j = 0; j < numEntries; j++) { - readTasks.add(new ReadTask(i, j, positions.get(i * numEntries + j), entryLogger)); + // reading after flush of rotatedlogs + ArrayList readTasks = new ArrayList(); + for (int i = 0; i < numOfActiveLedgers; i++) { + for (int j = 0; j < numEntries; j++) { + readTasks.add(new ReadTask(i, j, positions.get(i * numEntries + j), entryLogger)); + } } - } - ExecutorService executor = Executors.newFixedThreadPool(40); - executor.invokeAll(readTasks).forEach((future) -> { - try { - future.get(); - } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); - LOG.error("Read/Flush task failed because of InterruptedException", ie); - Assert.fail("Read/Flush task interrupted"); - } catch (Exception ex) { - LOG.error("Read/Flush task failed because of exception", ex); - Assert.fail("Read/Flush task failed " + ex.getMessage()); + executor = Executors.newFixedThreadPool(40); + executor.invokeAll(readTasks).forEach((future) -> { + try { + future.get(); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + LOG.error("Read/Flush task failed because of InterruptedException", ie); + Assert.fail("Read/Flush task interrupted"); + } catch (Exception ex) { + LOG.error("Read/Flush task failed because of exception", ex); + Assert.fail("Read/Flush task failed " + ex.getMessage()); + } + }); + } finally { + if (executor != null) { + executor.shutdownNow(); } - }); + entryLogger.close(); + } } /** @@ -1774,70 +1887,74 @@ public void testEntryLoggerAddEntryWhenLedgerDirsAreFull() throws Exception { new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger entryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) - entryLogger.getEntryLogManager(); - Assert.assertEquals("EntryLogManager class type", EntryLogManagerForEntryLogPerLedger.class, - entryLogManager.getClass()); + try { + EntryLogManagerForEntryLogPerLedger entryLogManager = (EntryLogManagerForEntryLogPerLedger) + entryLogger.getEntryLogManager(); + Assert.assertEquals("EntryLogManager class type", EntryLogManagerForEntryLogPerLedger.class, + entryLogManager.getClass()); - entryLogger.addEntry(0L, generateEntry(0, 1)); - entryLogger.addEntry(1L, generateEntry(1, 1)); - entryLogger.addEntry(2L, generateEntry(2, 1)); + entryLogger.addEntry(0L, generateEntry(0, 1)); + entryLogger.addEntry(1L, generateEntry(1, 1)); + entryLogger.addEntry(2L, generateEntry(2, 1)); - File ledgerDirForLedger0 = entryLogManager.getCurrentLogForLedger(0L).getLogFile().getParentFile(); - File ledgerDirForLedger1 = entryLogManager.getCurrentLogForLedger(1L).getLogFile().getParentFile(); - File ledgerDirForLedger2 = entryLogManager.getCurrentLogForLedger(2L).getLogFile().getParentFile(); + File ledgerDirForLedger0 = entryLogManager.getCurrentLogForLedger(0L).getLogFile().getParentFile(); + File ledgerDirForLedger1 = entryLogManager.getCurrentLogForLedger(1L).getLogFile().getParentFile(); + File ledgerDirForLedger2 = entryLogManager.getCurrentLogForLedger(2L).getLogFile().getParentFile(); - Set ledgerDirsSet = new HashSet(); - ledgerDirsSet.add(ledgerDirForLedger0); - ledgerDirsSet.add(ledgerDirForLedger1); - ledgerDirsSet.add(ledgerDirForLedger2); + Set ledgerDirsSet = new HashSet(); + ledgerDirsSet.add(ledgerDirForLedger0); + ledgerDirsSet.add(ledgerDirForLedger1); + ledgerDirsSet.add(ledgerDirForLedger2); - /* - * since there are 3 ledgerdirs, entrylogs for all the 3 ledgers should be in different ledgerdirs. - */ - Assert.assertEquals("Current active LedgerDirs size", 3, ledgerDirs.size()); - Assert.assertEquals("Number of rotated logchannels", 0, entryLogManager.getRotatedLogChannels().size()); + /* + * since there are 3 ledgerdirs, entrylogs for all the 3 ledgers should be in different ledgerdirs. + */ + Assert.assertEquals("Current active LedgerDirs size", 3, ledgerDirs.size()); + Assert.assertEquals("Number of rotated logchannels", 0, entryLogManager.getRotatedLogChannels().size()); - /* - * ledgerDirForLedger0 is added to filledDirs, for ledger0 new entrylog should not be created in - * ledgerDirForLedger0 - */ - ledgerDirsManager.addToFilledDirs(ledgerDirForLedger0); - addEntryAndValidateFolders(entryLogger, entryLogManager, 2, ledgerDirForLedger0, false, ledgerDirForLedger1, - ledgerDirForLedger2); - Assert.assertEquals("Number of rotated logchannels", 1, entryLogManager.getRotatedLogChannels().size()); + /* + * ledgerDirForLedger0 is added to filledDirs, for ledger0 new entrylog should not be created in + * ledgerDirForLedger0 + */ + ledgerDirsManager.addToFilledDirs(ledgerDirForLedger0); + addEntryAndValidateFolders(entryLogger, entryLogManager, 2, ledgerDirForLedger0, false, ledgerDirForLedger1, + ledgerDirForLedger2); + Assert.assertEquals("Number of rotated logchannels", 1, entryLogManager.getRotatedLogChannels().size()); - /* - * ledgerDirForLedger1 is also added to filledDirs, so for all the ledgers new entryLogs should be in - * ledgerDirForLedger2 - */ - ledgerDirsManager.addToFilledDirs(ledgerDirForLedger1); - addEntryAndValidateFolders(entryLogger, entryLogManager, 3, ledgerDirForLedger2, true, ledgerDirForLedger2, - ledgerDirForLedger2); - Assert.assertTrue("Number of rotated logchannels", (2 <= entryLogManager.getRotatedLogChannels().size()) - && (entryLogManager.getRotatedLogChannels().size() <= 3)); - int numOfRotatedLogChannels = entryLogManager.getRotatedLogChannels().size(); + /* + * ledgerDirForLedger1 is also added to filledDirs, so for all the ledgers new entryLogs should be in + * ledgerDirForLedger2 + */ + ledgerDirsManager.addToFilledDirs(ledgerDirForLedger1); + addEntryAndValidateFolders(entryLogger, entryLogManager, 3, ledgerDirForLedger2, true, ledgerDirForLedger2, + ledgerDirForLedger2); + Assert.assertTrue("Number of rotated logchannels", (2 <= entryLogManager.getRotatedLogChannels().size()) + && (entryLogManager.getRotatedLogChannels().size() <= 3)); + int numOfRotatedLogChannels = entryLogManager.getRotatedLogChannels().size(); - /* - * since ledgerDirForLedger2 is added to filleddirs, all the dirs are full. If all the dirs are full then it - * will continue to use current entrylogs for new entries instead of creating new one. So for all the ledgers - * ledgerdirs should be same as before - ledgerDirForLedger2 - */ - ledgerDirsManager.addToFilledDirs(ledgerDirForLedger2); - addEntryAndValidateFolders(entryLogger, entryLogManager, 4, ledgerDirForLedger2, true, ledgerDirForLedger2, - ledgerDirForLedger2); - Assert.assertEquals("Number of rotated logchannels", numOfRotatedLogChannels, - entryLogManager.getRotatedLogChannels().size()); + /* + * since ledgerDirForLedger2 is added to filleddirs, all the dirs are full. If all the dirs are full then it + * will continue to use current entrylogs for new entries instead of creating new one. So for all the ledgers + * ledgerdirs should be same as before - ledgerDirForLedger2 + */ + ledgerDirsManager.addToFilledDirs(ledgerDirForLedger2); + addEntryAndValidateFolders(entryLogger, entryLogManager, 4, ledgerDirForLedger2, true, ledgerDirForLedger2, + ledgerDirForLedger2); + Assert.assertEquals("Number of rotated logchannels", numOfRotatedLogChannels, + entryLogManager.getRotatedLogChannels().size()); - /* - * ledgerDirForLedger1 is added back to writableDirs, so new entrylog for all the ledgers should be created in - * ledgerDirForLedger1 - */ - ledgerDirsManager.addToWritableDirs(ledgerDirForLedger1, true); - addEntryAndValidateFolders(entryLogger, entryLogManager, 4, ledgerDirForLedger1, true, ledgerDirForLedger1, - ledgerDirForLedger1); - Assert.assertEquals("Number of rotated logchannels", numOfRotatedLogChannels + 3, - entryLogManager.getRotatedLogChannels().size()); + /* + * ledgerDirForLedger1 is added back to writableDirs, so new entrylog for all the ledgers should be created + * in ledgerDirForLedger1 + */ + ledgerDirsManager.addToWritableDirs(ledgerDirForLedger1, true); + addEntryAndValidateFolders(entryLogger, entryLogManager, 4, ledgerDirForLedger1, true, ledgerDirForLedger1, + ledgerDirForLedger1); + Assert.assertEquals("Number of rotated logchannels", numOfRotatedLogChannels + 3, + entryLogManager.getRotatedLogChannels().size()); + } finally { + entryLogger.close(); + } } /* @@ -1893,75 +2010,87 @@ public void testSwappingEntryLogManager(boolean initialEntryLogPerLedgerEnabled, new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); DefaultEntryLogger defaultEntryLogger = new DefaultEntryLogger(conf, ledgerDirsManager); - EntryLogManagerBase entryLogManager = (EntryLogManagerBase) defaultEntryLogger.getEntryLogManager(); - Assert.assertEquals( - "EntryLogManager class type", initialEntryLogPerLedgerEnabled - ? EntryLogManagerForEntryLogPerLedger.class : EntryLogManagerForSingleEntryLog.class, - entryLogManager.getClass()); - - int numOfActiveLedgers = 10; - int numEntries = 10; - long[][] positions = new long[numOfActiveLedgers][]; - for (int i = 0; i < numOfActiveLedgers; i++) { - positions[i] = new long[numEntries]; - } - - /* - * addentries to the ledgers - */ - for (int j = 0; j < numEntries; j++) { + DefaultEntryLogger newEntryLogger = null; + try { + EntryLogManagerBase entryLogManager = (EntryLogManagerBase) defaultEntryLogger.getEntryLogManager(); + Assert.assertEquals( + "EntryLogManager class type", initialEntryLogPerLedgerEnabled + ? EntryLogManagerForEntryLogPerLedger.class : EntryLogManagerForSingleEntryLog.class, + entryLogManager.getClass()); + + int numOfActiveLedgers = 10; + int numEntries = 10; + long[][] positions = new long[numOfActiveLedgers][]; for (int i = 0; i < numOfActiveLedgers; i++) { - positions[i][j] = defaultEntryLogger.addEntry((long) i, generateEntry(i, j)); - long entryLogId = (positions[i][j] >> 32L); - if (initialEntryLogPerLedgerEnabled) { - Assert.assertEquals("EntryLogId for ledger: " + i, i, entryLogId); - } else { - Assert.assertEquals("EntryLogId for ledger: " + i, 0, entryLogId); + positions[i] = new long[numEntries]; + } + + /* + * addentries to the ledgers + */ + for (int j = 0; j < numEntries; j++) { + for (int i = 0; i < numOfActiveLedgers; i++) { + positions[i][j] = defaultEntryLogger.addEntry((long) i, generateEntry(i, j)); + long entryLogId = (positions[i][j] >> 32L); + if (initialEntryLogPerLedgerEnabled) { + Assert.assertEquals("EntryLogId for ledger: " + i, i, entryLogId); + } else { + Assert.assertEquals("EntryLogId for ledger: " + i, 0, entryLogId); + } } } - } - for (long i = 0; i < numOfActiveLedgers; i++) { - entryLogManager.createNewLog(i); - } + for (long i = 0; i < numOfActiveLedgers; i++) { + entryLogManager.createNewLog(i); + } - /** - * since new entrylog is created for all the ledgers, the previous - * entrylogs must be rotated and with the following flushRotatedLogs - * call they should be forcewritten and file should be closed. - */ - entryLogManager.flushRotatedLogs(); + /** + * since new entrylog is created for all the ledgers, the previous + * entrylogs must be rotated and with the following flushRotatedLogs + * call they should be forcewritten and file should be closed. + */ + entryLogManager.flushRotatedLogs(); - /* - * new entrylogger and entryLogManager are created with - * 'laterEntryLogPerLedgerEnabled' conf - */ - conf.setEntryLogPerLedgerEnabled(laterEntryLogPerLedgerEnabled); - LedgerDirsManager newLedgerDirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(), - new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); - DefaultEntryLogger newEntryLogger = new DefaultEntryLogger(conf, newLedgerDirsManager); - EntryLogManager newEntryLogManager = newEntryLogger.getEntryLogManager(); - Assert.assertEquals("EntryLogManager class type", - laterEntryLogPerLedgerEnabled ? EntryLogManagerForEntryLogPerLedger.class - : EntryLogManagerForSingleEntryLog.class, - newEntryLogManager.getClass()); + /* + * new entrylogger and entryLogManager are created with + * 'laterEntryLogPerLedgerEnabled' conf + */ + conf.setEntryLogPerLedgerEnabled(laterEntryLogPerLedgerEnabled); + LedgerDirsManager newLedgerDirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(), + new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold())); + newEntryLogger = new DefaultEntryLogger(conf, newLedgerDirsManager); + EntryLogManager newEntryLogManager = newEntryLogger.getEntryLogManager(); + Assert.assertEquals("EntryLogManager class type", + laterEntryLogPerLedgerEnabled ? EntryLogManagerForEntryLogPerLedger.class + : EntryLogManagerForSingleEntryLog.class, + newEntryLogManager.getClass()); - /* - * read the entries (which are written with previous entrylogger) with - * new entrylogger - */ - for (int j = 0; j < numEntries; j++) { - for (int i = 0; i < numOfActiveLedgers; i++) { - String expectedValue = "ledger-" + i + "-" + j; - ByteBuf buf = newEntryLogger.readEntry(i, j, positions[i][j]); - long ledgerId = buf.readLong(); - long entryId = buf.readLong(); - byte[] data = new byte[buf.readableBytes()]; - buf.readBytes(data); - assertEquals("LedgerId ", i, ledgerId); - assertEquals("EntryId ", j, entryId); - assertEquals("Entry Data ", expectedValue, new String(data)); + /* + * read the entries (which are written with previous entrylogger) with + * new entrylogger + */ + for (int j = 0; j < numEntries; j++) { + for (int i = 0; i < numOfActiveLedgers; i++) { + String expectedValue = "ledger-" + i + "-" + j; + ByteBuf buf = newEntryLogger.readEntry(i, j, positions[i][j]); + try { + long ledgerId = buf.readLong(); + long entryId = buf.readLong(); + byte[] data = new byte[buf.readableBytes()]; + buf.readBytes(data); + assertEquals("LedgerId ", i, ledgerId); + assertEquals("EntryId ", j, entryId); + assertEquals("Entry Data ", expectedValue, new String(data)); + } finally { + ReferenceCountUtil.release(buf); + } + } + } + } finally { + if (newEntryLogger != null) { + newEntryLogger.close(); } + defaultEntryLogger.close(); } } diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java index 448f3737875..e0e50e94210 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java @@ -163,7 +163,7 @@ public void simple() throws Exception { // Read from write cache assertTrue(storage.entryExists(4, 1)); ByteBuf res = storage.getEntry(4, 1); - assertEquals(entry, res); + assertByteBufEqualsAndRelease(entry, res); storage.flush(); @@ -172,7 +172,7 @@ public void simple() throws Exception { // Read from db assertTrue(storage.entryExists(4, 1)); res = storage.getEntry(4, 1); - assertEquals(entry, res); + assertByteBufEqualsAndRelease(entry, res); try { storage.getEntry(4, 2); @@ -191,7 +191,7 @@ public void simple() throws Exception { // Read last entry in ledger res = storage.getEntry(4, BookieProtocol.LAST_ADD_CONFIRMED); - assertEquals(entry2, res); + assertByteBufEqualsAndRelease(entry2, res); // Read last add confirmed in ledger assertEquals(1L, storage.getLastAddConfirmed(4)); @@ -211,7 +211,7 @@ public void simple() throws Exception { storage.addEntry(entry4); res = storage.getEntry(4, 4); - assertEquals(entry4, res); + assertByteBufEqualsAndRelease(entry4, res); assertEquals(3, storage.getLastAddConfirmed(4)); @@ -260,7 +260,7 @@ public void testBookieCompaction() throws Exception { ByteBuf res = storage.getEntry(4, 3); System.out.println("res: " + ByteBufUtil.hexDump(res)); System.out.println("newEntry3: " + ByteBufUtil.hexDump(newEntry3)); - assertEquals(newEntry3, res); + assertByteBufEqualsAndRelease(newEntry3, res); } @Test @@ -385,7 +385,7 @@ public void testRewritingEntries() throws Exception { storage.flush(); ByteBuf response = storage.getEntry(1, 1); - assertEquals(newEntry1, response); + assertByteBufEqualsAndRelease(newEntry1, response); } @Test @@ -407,7 +407,7 @@ public void testEntriesOutOfOrder() throws Exception { } ByteBuf res = storage.getEntry(1, 2); - assertEquals(entry2, res); + assertByteBufEqualsAndRelease(entry2, res); ByteBuf entry1 = Unpooled.buffer(1024); entry1.writeLong(1); // ledger id @@ -417,18 +417,18 @@ public void testEntriesOutOfOrder() throws Exception { storage.addEntry(entry1); res = storage.getEntry(1, 1); - assertEquals(entry1, res); + assertByteBufEqualsAndRelease(entry1, res); res = storage.getEntry(1, 2); - assertEquals(entry2, res); + assertByteBufEqualsAndRelease(entry2, res); storage.flush(); res = storage.getEntry(1, 1); - assertEquals(entry1, res); + assertByteBufEqualsAndRelease(entry1, res); res = storage.getEntry(1, 2); - assertEquals(entry2, res); + assertByteBufEqualsAndRelease(entry2, res); } @Test @@ -528,8 +528,8 @@ public void testAddEntriesAfterDelete() throws Exception { storage.addEntry(entry0); storage.addEntry(entry1); - assertEquals(entry0, storage.getEntry(1, 0)); - assertEquals(entry1, storage.getEntry(1, 1)); + assertByteBufEqualsAndRelease(entry0, storage.getEntry(1, 0)); + assertByteBufEqualsAndRelease(entry1, storage.getEntry(1, 1)); storage.flush(); } @@ -547,10 +547,13 @@ public void testLimboStateSucceedsWhenInLimboButHasEntry() throws Exception { storage.flush(); storage.setLimboState(1); + ByteBuf result = null; try { - storage.getEntry(1, 0); + result = storage.getEntry(1, 0); } catch (BookieException.DataUnknownException e) { fail("Should have been able to read entry"); + } finally { + ReferenceCountUtil.release(result); } } @@ -567,24 +570,30 @@ public void testLimboStateThrowsInLimboWhenNoEntry() throws Exception { storage.flush(); storage.setLimboState(1); + ByteBuf result = null; try { - storage.getEntry(1, 1); + result = storage.getEntry(1, 1); } catch (NoEntryException nee) { fail("Shouldn't have seen NoEntryException"); } catch (BookieException.DataUnknownException e) { // expected + } finally { + ReferenceCountUtil.release(result); } storage.shutdown(); Bookie restartedBookie = new TestBookieImpl(conf); DbLedgerStorage restartedStorage = (DbLedgerStorage) restartedBookie.getLedgerStorage(); try { + result = null; try { - restartedStorage.getEntry(1, 1); + result = restartedStorage.getEntry(1, 1); } catch (NoEntryException nee) { fail("Shouldn't have seen NoEntryException"); } catch (BookieException.DataUnknownException e) { // expected + } finally { + ReferenceCountUtil.release(result); } } finally { restartedStorage.shutdown(); @@ -606,21 +615,27 @@ public void testLimboStateThrowsNoEntryExceptionWhenLimboCleared() throws Except storage.flush(); storage.setLimboState(1); + ByteBuf result = null; try { - storage.getEntry(1, 1); + result = storage.getEntry(1, 1); } catch (NoEntryException nee) { fail("Shouldn't have seen NoEntryException"); } catch (BookieException.DataUnknownException e) { // expected + } finally { + ReferenceCountUtil.release(result); } storage.clearLimboState(1); + result = null; try { - storage.getEntry(1, 1); + result = storage.getEntry(1, 1); } catch (NoEntryException nee) { // expected } catch (BookieException.DataUnknownException e) { fail("Should have seen NoEntryException"); + } finally { + ReferenceCountUtil.release(result); } } @@ -689,7 +704,7 @@ public void testHasEntry() throws Exception { assertFalse(storage.entryExists(ledgerId, 1)); // pull entry into readcache - storage.getEntry(ledgerId, 0); + ReferenceCountUtil.release(storage.getEntry(ledgerId, 0)); // should come from read cache assertTrue(storage.entryExists(ledgerId, 0)); @@ -840,6 +855,14 @@ private LogMark readLogMark(File file) throws IOException { return mark; } + private static void assertByteBufEqualsAndRelease(ByteBuf expected, ByteBuf actual) { + try { + assertEquals(expected, actual); + } finally { + ReferenceCountUtil.release(actual); + } + } + @Test public void testSingleLedgerDirectoryCheckpoint() throws Exception { int gcWaitTime = 1000; From cc086d7a98b76280aee60ff04a7f191845854415 Mon Sep 17 00:00:00 2001 From: yangxianjungree <714696209@qq.com> Date: Wed, 5 Aug 2026 10:36:47 +0800 Subject: [PATCH 9/9] fix: make entrylog failure shutdown cleanup idempotent (cherry picked from commit 90885008f7330fc852a340139b4ec594c6d3e33a) --- .../bookkeeper/bookie/DefaultEntryLogger.java | 6 ++++++ .../bookkeeper/bookie/EntryLoggerAllocator.java | 9 ++++++++- .../org/apache/bookkeeper/bookie/SyncThread.java | 9 ++++++++- .../apache/bookkeeper/bookie/BookieImplTest.java | 6 ++++++ .../bookkeeper/bookie/DefaultEntryLogTest.java | 13 +++++++++++++ .../apache/bookkeeper/bookie/SyncThreadTest.java | 14 ++++++++++++++ ...DbLedgerStorageEntryLogFlushFailureE2ETest.java | 1 + .../storage/ldb/FailOnFlushDbLedgerStorage.java | 5 +---- 8 files changed, 57 insertions(+), 6 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java index 32ece69f606..900d1da621a 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java @@ -56,6 +56,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.Pattern; import org.apache.bookkeeper.bookie.LedgerDirsManager.LedgerDirsListener; import org.apache.bookkeeper.bookie.storage.CompactionEntryLog; @@ -241,6 +242,7 @@ private static void writeFully(FileChannel fileChannel, ByteBuffer buffer, long final EntryLoggerAllocator entryLoggerAllocator; private final EntryLogManager entryLogManager; + private final AtomicBoolean closed = new AtomicBoolean(false); private final CopyOnWriteArrayList listeners = new CopyOnWriteArrayList(); @@ -1214,6 +1216,10 @@ public boolean accept(long ledgerId) { */ @Override public void close() { + if (!closed.compareAndSet(false, true)) { + LOG.debug("EntryLogger is already stopped"); + return; + } // since logChannel is buffered channel, do flush when shutting down LOG.info("Stopping EntryLogger"); try { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java index 832539dcc29..24c5a64404d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLoggerAllocator.java @@ -41,6 +41,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import lombok.extern.slf4j.Slf4j; @@ -240,7 +241,13 @@ void setLastLogId(File dir, long logId) throws IOException { */ void stop() { // wait until the preallocation finished. - allocatorExecutor.execute(this::closePreAllocateLog); + if (!allocatorExecutor.isShutdown()) { + try { + allocatorExecutor.execute(this::closePreAllocateLog); + } catch (RejectedExecutionException e) { + log.debug("Skipping preallocated entry log cleanup because allocator is stopping", e); + } + } allocatorExecutor.shutdown(); try { if (!allocatorExecutor.awaitTermination(5, TimeUnit.SECONDS)) { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java index 1de3f5a068d..28da210afba 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/SyncThread.java @@ -26,6 +26,7 @@ import java.io.IOException; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import lombok.AccessLevel; @@ -235,7 +236,13 @@ public void disableCheckpoint() { // shutdown sync thread void shutdown() throws InterruptedException { log.info("Shutting down SyncThread"); - requestFlush(); + if (!executor.isShutdown()) { + try { + requestFlush(); + } catch (RejectedExecutionException e) { + log.debug("Skipping final flush because SyncThread executor is shutting down", e); + } + } executor.shutdown(); long start = System.currentTimeMillis(); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java index 7ad6a51e48b..b66934d5b5f 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieImplTest.java @@ -147,6 +147,9 @@ public void testBackgroundEntryLogFlushFailureShutsDownBookie() throws Exception BookieImpl bookie = new TestBookieImpl(resources) { @Override int shutdown(int exitCode) { + if (exitCode == ExitCode.BOOKIE_EXCEPTION) { + FailOnFlushDbLedgerStorage.resetFailure(); + } int result = super.shutdown(exitCode); if (exitCode == ExitCode.BOOKIE_EXCEPTION) { shutdownLatch.countDown(); @@ -182,6 +185,9 @@ public void testStartupEntryLogFlushFailureStopsBookieBeforeRunning() throws Exc BookieImpl bookie = new TestBookieImpl(resources) { @Override int shutdown(int exitCode) { + if (exitCode == ExitCode.BOOKIE_EXCEPTION) { + FailOnFlushDbLedgerStorage.resetFailure(); + } int result = super.shutdown(exitCode); if (exitCode == ExitCode.BOOKIE_EXCEPTION) { shutdownLatch.countDown(); diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java index 64fb678b7c7..3b0da105b83 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/DefaultEntryLogTest.java @@ -506,6 +506,19 @@ public void testPreAllocateLog() throws Exception { assertNull(entryLogger.getEntryLoggerAllocator().getPreallocationFuture()); } + @Test + public void testCloseIsIdempotentWithPreAllocation() throws Exception { + entryLogger.close(); + + conf.setEntryLogFilePreAllocationEnabled(true); + entryLogger = new DefaultEntryLogger(conf, dirsMgr); + ((EntryLogManagerBase) entryLogger.getEntryLogManager()).createNewLog(DefaultEntryLogger.UNASSIGNED_LEDGERID); + assertNotNull(entryLogger.getEntryLoggerAllocator().getPreallocationFuture()); + + entryLogger.close(); + entryLogger.close(); + } + /** * Test the getEntryLogsSet() method. */ diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java index 98857c616c3..526a93fbe8b 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/SyncThreadTest.java @@ -147,6 +147,20 @@ public void checkpoint(Checkpoint checkpoint) assertFalse("Shouldn't have failed anywhere", failedSomewhere.get()); } + @Test + public void testSyncThreadShutdownIsIdempotent() throws Exception { + int flushInterval = 100; + ServerConfiguration conf = TestBKConfiguration.newServerConfiguration(); + conf.setFlushInterval(flushInterval); + CheckpointSource checkpointSource = new DummyCheckpointSource(); + LedgerDirsListener listener = new LedgerDirsListener() {}; + LedgerStorage storage = new DummyLedgerStorage(); + + final SyncThread t = new SyncThread(conf, listener, storage, checkpointSource, NullStatsLogger.INSTANCE); + t.shutdown(); + t.shutdown(); + } + /** * Test that sync thread suspension works. * i.e. when we suspend the syncthread, nothing diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java index ce093ecd367..e529113a798 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageEntryLogFlushFailureE2ETest.java @@ -65,6 +65,7 @@ public void testClientWriteFailsAndBookieShutsDownAfterEntryLogFlushFailure() th } assertNotNull("Client should observe a write failure after the entry log flush failure", clientFailure); + FailOnFlushDbLedgerStorage.resetFailure(); Awaitility.await().atMost(10, TimeUnit.SECONDS).untilAsserted(() -> assertFalse("Bookie should be shut down after entry log flush failure", bookie.isRunning())); } finally { diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java index 19d97fe8da2..cc6286de042 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/FailOnFlushDbLedgerStorage.java @@ -32,7 +32,6 @@ public class FailOnFlushDbLedgerStorage extends DbLedgerStorage { private static final AtomicBoolean failNextFlushWithEntryLogWriteException = new AtomicBoolean(false); - private static final AtomicBoolean entryLogFlushFailed = new AtomicBoolean(false); public static void injectFailureOnNextFlush() { failNextFlushWithEntryLogWriteException.set(true); @@ -40,7 +39,6 @@ public static void injectFailureOnNextFlush() { public static void resetFailure() { failNextFlushWithEntryLogWriteException.set(false); - entryLogFlushFailed.set(false); } @Override @@ -66,8 +64,7 @@ private static class FailOnFlushSingleDirectoryDbLedgerStorage extends SingleDir @Override public void flush() throws IOException { - if (entryLogFlushFailed.get() || failNextFlushWithEntryLogWriteException.compareAndSet(true, false)) { - entryLogFlushFailed.set(true); + if (failNextFlushWithEntryLogWriteException.compareAndSet(true, false)) { throw new EntryLogWriteException("entry log flush failed", new IOException("injected")); } super.flush();