diff --git a/.github/workflows/compatibility-check-java11.yml b/.github/workflows/compatibility-check-java11.yml index 3f1306820fb..243e8efb1c1 100644 --- a/.github/workflows/compatibility-check-java11.yml +++ b/.github/workflows/compatibility-check-java11.yml @@ -46,6 +46,10 @@ jobs: with: java-version: 1.11 - name: Build with gradle - run: ./gradlew test -x bookkeeper-server:test -x tests:integration:cluster:test -x tests:integration:smoke:test -x tests:integration:standalone:test -Dtestlogger.theme=plain - -PexcludeTests="*.apache.distributedlog.*, *.apache.bookkeeper.statelib.*, *.apache.bookkeeper.clients.*, *.apache.bookkeeper.common.*, *.apache.bookkeeper.stream.*" + run: | + ./gradlew test -x bookkeeper-server:test + -x tests:integration:cluster:test + -x tests:integration:smoke:test + -x tests:integration:standalone:test + -Dtestlogger.theme=plain -PexcludeTests="*.apache.distributedlog.*, *.apache.bookkeeper.statelib.*, *.apache.bookkeeper.clients.*, *.apache.bookkeeper.common.*, *.apache.bookkeeper.stream.*" diff --git a/.github/workflows/compatibility-check-java8.yml b/.github/workflows/compatibility-check-java8.yml index 8caa43c8740..239a85080b1 100644 --- a/.github/workflows/compatibility-check-java8.yml +++ b/.github/workflows/compatibility-check-java8.yml @@ -46,8 +46,9 @@ jobs: with: java-version: 1.8 - name: Build with gradle - run: ./gradlew test -x bookkeeper-server:test - -x tests:integration:cluster:test - -x tests:integration:smoke:test - -x tests:integration:standalone:test -Dtestlogger.theme=plain - -PexcludeTests="*.apache.distributedlog.*, *.apache.bookkeeper.statelib.*, *.apache.bookkeeper.clients.*, *.apache.bookkeeper.common.*, *.apache.bookkeeper.stream.*" + run: | + ./gradlew test -x bookkeeper-server:test + -x tests:integration:cluster:test + -x tests:integration:smoke:test + -x tests:integration:standalone:test -Dtestlogger.theme=plain + -PexcludeTests="*.apache.distributedlog.*, *.apache.bookkeeper.statelib.*, *.apache.bookkeeper.clients.*, *.apache.bookkeeper.common.*, *.apache.bookkeeper.stream.*" diff --git a/bookkeeper-benchmark/build.gradle b/bookkeeper-benchmark/build.gradle index 6f428f697de..3dec69e5229 100644 --- a/bookkeeper-benchmark/build.gradle +++ b/bookkeeper-benchmark/build.gradle @@ -42,6 +42,8 @@ dependencies { implementation depLibs.nettyTransportNativeEpoll implementation depLibs.zookeeper implementation depLibs.log4j12api + implementation depLibs.log4jCore + implementation depLibs.log4jSlf4jImpl testImplementation project(':bookkeeper-server') testImplementation project(path: ':bookkeeper-common', configuration: 'testArtifacts') @@ -63,7 +65,6 @@ test { maxFailures = 10 maxRetries = 3 } - maxHeapSize = '4G' forkEvery = 1 jvmArgs("-Djunit.timeout.test=600000", diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCache.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCache.java index 986c741a010..702aa34854e 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCache.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCache.java @@ -27,6 +27,7 @@ import io.netty.buffer.Unpooled; import java.io.Closeable; +import java.io.IOException; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; @@ -35,6 +36,7 @@ import org.apache.bookkeeper.util.collections.ConcurrentLongLongPairHashMap; import org.apache.bookkeeper.util.collections.ConcurrentLongLongPairHashMap.LongPair; + /** * Read cache implementation. * @@ -59,6 +61,7 @@ public class ReadCache implements Closeable { private ByteBufAllocator allocator; private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock(); + private volatile boolean isStorageShutdown; public ReadCache(ByteBufAllocator allocator, long maxCacheSize) { this(allocator, maxCacheSize, DEFAULT_MAX_SEGMENT_SIZE); @@ -76,18 +79,32 @@ public ReadCache(ByteBufAllocator allocator, long maxCacheSize, int maxSegmentSi cacheSegments.add(Unpooled.directBuffer(segmentSize, segmentSize)); cacheIndexes.add(new ConcurrentLongLongPairHashMap(4096, 2 * Runtime.getRuntime().availableProcessors())); } + isStorageShutdown = false; } @Override public void close() { - cacheSegments.forEach(ByteBuf::release); + lock.writeLock().lock(); + try { + isStorageShutdown = true; + cacheSegments.forEach(ByteBuf::release); + } finally { + lock.writeLock().unlock(); + } } - public void put(long ledgerId, long entryId, ByteBuf entry) { + public void put(long ledgerId, long entryId, ByteBuf entry) throws IOException { int entrySize = entry.readableBytes(); int alignedSize = align64(entrySize); lock.readLock().lock(); + if (isStorageShutdown) { + // This is a hack. Ideally, we should be returning from here as a no-op or, + // the higher layers must no-op read/ writes after shutdown() + // Since the other lower layers like locationIndex, logEntry also need to handle the shutdown scenario, + // the IOException is thrown from here so that it bypasses other lower layers. + throw new IOException("Read cache has shutdown, not allowing put cache operation"); + } try { int offset = currentSegmentOffset.getAndAdd(alignedSize); @@ -104,10 +121,16 @@ public void put(long ledgerId, long entryId, ByteBuf entry) { lock.readLock().unlock(); } - // We could not insert in segment, we to get the write lock and roll-over to + // We could not insert in segment, we need to get the write lock and roll-over to // next segment lock.writeLock().lock(); - + if (isStorageShutdown) { + // This is a hack. Ideally, we should be returning from here as a no-op or, + // the higher layers must no-op read/ writes after shutdown() + // Since the other lower layers like locationIndex, logEntry also need to handle the shutdown scenario, + // the IOException is thrown from here so that it bypasses other lower layers. + throw new IOException("Read cache has shutdown, not allowing put cache operation"); + } try { int offset = currentSegmentOffset.getAndAdd(entrySize); if (offset + entrySize > segmentSize) { @@ -126,9 +149,16 @@ public void put(long ledgerId, long entryId, ByteBuf entry) { } } - public ByteBuf get(long ledgerId, long entryId) { + public ByteBuf get(long ledgerId, long entryId) throws IOException { lock.readLock().lock(); + if (isStorageShutdown) { + // This is a hack. Ideally, we should be returning from here as a no-op or, + // the higher layers must no-op read/ writes after shutdown() + // Since the other lower layers like locationIndex, logEntry also need to handle the shutdown scenario, + // the IOException is thrown from here so that it bypasses other lower layers. + throw new IOException("Read cache has shutdown, not allowing get cache operation"); + } try { // We need to check all the segments, starting from the current one and looking // backward to minimize the 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 f2a3bec153d..7c7eb2c2675 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 @@ -52,6 +52,7 @@ public class DbLedgerStorageTest { private DbLedgerStorage storage; private File tmpDir; private LedgerDirsManager ledgerDirsManager; + private boolean isStorageShutdown; @Before public void setup() throws Exception { @@ -70,11 +71,14 @@ public void setup() throws Exception { ledgerDirsManager = bookie.getLedgerDirsManager(); storage = (DbLedgerStorage) bookie.getLedgerStorage(); + isStorageShutdown = false; } @After public void teardown() throws Exception { - storage.shutdown(); + if (!isStorageShutdown) { + storage.shutdown(); + } tmpDir.delete(); } @@ -238,6 +242,20 @@ public void testBookieCompaction() throws Exception { assertEquals(newEntry3, res); } + @Test(expected = IOException.class) + public void testReadsOpAfterShutdown() throws Exception { + SingleDirectoryDbLedgerStorage singleDirStorage = ((DbLedgerStorage) storage).getLedgerStorageList().get(0); + ByteBuf entry1 = Unpooled.buffer(64); + entry1.writeLong(1); // ledger id + entry1.writeLong(1); // entry id + entry1.writeBytes("e".getBytes()); + singleDirStorage.addEntry(entry1); + entry1.release(); + singleDirStorage.shutdown(); + isStorageShutdown = true; + singleDirStorage.getEntry(1, 1); + } + @Test public void doubleDirectory() throws Exception { int gcWaitTime = 1000; diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCacheTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCacheTest.java index 337140c2dd9..52718075f42 100644 --- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCacheTest.java +++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCacheTest.java @@ -26,6 +26,7 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; import io.netty.buffer.UnpooledByteBufAllocator; +import java.io.IOException; import org.junit.Test; @@ -35,7 +36,7 @@ public class ReadCacheTest { @Test - public void simple() { + public void simple() throws IOException { ReadCache cache = new ReadCache(UnpooledByteBufAllocator.DEFAULT, 10 * 1024); assertEquals(0, cache.count()); @@ -72,7 +73,7 @@ public void simple() { } @Test - public void emptyCache() { + public void emptyCache() throws IOException { ReadCache cache = new ReadCache(UnpooledByteBufAllocator.DEFAULT, 10 * 1024); assertEquals(0, cache.count()); @@ -83,7 +84,7 @@ public void emptyCache() { } @Test - public void multipleSegments() { + public void multipleSegments() throws IOException{ // Test with multiple smaller segments ReadCache cache = new ReadCache(UnpooledByteBufAllocator.DEFAULT, 10 * 1024, 2 * 1024); diff --git a/shaded/distributedlog-core-shaded/build.gradle b/shaded/distributedlog-core-shaded/build.gradle index 2bafc330aaa..e5c83b21e5a 100644 --- a/shaded/distributedlog-core-shaded/build.gradle +++ b/shaded/distributedlog-core-shaded/build.gradle @@ -53,7 +53,7 @@ shadowJar { relocate 'org.rocksdb', 'dlshade.org.rocksdb' relocate 'com.scurrilous.circe', 'dlshade.com.scurrilous.circe' relocate 'org.apache.bookkeeper', 'dlshade.org.apache.bookkeeper' - relocate 'org.apache.distributedlog', 'dhshade.org.apache.distributedlog' + relocate 'org.apache.distributedlog', 'dlshade.org.apache.distributedlog' archiveBaseName.set("distributedlog-core-shaded") archiveClassifier.set("") } diff --git a/stream/bk-grpc-name-resolver/build.gradle b/stream/bk-grpc-name-resolver/build.gradle index 950bb61724a..488f32a3310 100644 --- a/stream/bk-grpc-name-resolver/build.gradle +++ b/stream/bk-grpc-name-resolver/build.gradle @@ -40,6 +40,8 @@ dependencies { testImplementation project(path: ':bookkeeper-server', configuration: 'testArtifacts') testImplementation depLibs.junit testImplementation depLibs.zookeeper + testImplementation depLibs.zookeeperTest + testImplementation depLibs.log4jSlf4jImpl testCompileOnly depLibs.lombok testAnnotationProcessor depLibs.lombok diff --git a/tests/integration/cluster/build.gradle b/tests/integration/cluster/build.gradle index 4eafefd1b97..f29fedfec84 100644 --- a/tests/integration/cluster/build.gradle +++ b/tests/integration/cluster/build.gradle @@ -38,6 +38,7 @@ dependencies { testCompileOnly depLibs.lombok testImplementation depLibs.log4j12api + testImplementation depLibs.log4jCore testImplementation depLibs.testcontainers testImplementation depLibs.commonsConfiguration testImplementation depLibs.googleHTTPClient diff --git a/tests/shaded/distributedlog-core-shaded-test/src/test/java/org/apache/bookkeeper/tests/shaded/DistributedLogCoreShadedJarTest.java b/tests/shaded/distributedlog-core-shaded-test/src/test/java/org/apache/bookkeeper/tests/shaded/DistributedLogCoreShadedJarTest.java index 6432c303a28..6632c617e73 100644 --- a/tests/shaded/distributedlog-core-shaded-test/src/test/java/org/apache/bookkeeper/tests/shaded/DistributedLogCoreShadedJarTest.java +++ b/tests/shaded/distributedlog-core-shaded-test/src/test/java/org/apache/bookkeeper/tests/shaded/DistributedLogCoreShadedJarTest.java @@ -59,7 +59,7 @@ public void testProtobufIsShaded() throws Exception { @Test public void testProtobufShadedPath() throws Exception { - Class.forName("dlshade.com.google.protobuf.Message"); + Class.forName("org.apache.bookkeeper.shaded.com.google.protobuf.Message"); } @Test(expected = ClassNotFoundException.class) @@ -69,7 +69,7 @@ public void testGuavaIsShaded() throws Exception { @Test public void testGuavaShadedPath() throws Exception { - Class.forName("dlshade.com.google.common.cache.Cache"); + Class.forName("org.apache.bookkeeper.shaded.com.google.common.cache.Cache"); assertTrue(true); } @@ -119,19 +119,19 @@ public void testCirceChecksumShade() throws Exception { @Test public void testDistributedLogCommon() throws Exception { - Class.forName("org.apache.distributedlog.common.concurrent.AsyncSemaphore"); + Class.forName("dlshade.org.apache.distributedlog.common.concurrent.AsyncSemaphore"); assertTrue(true); } @Test public void testDistributedLogProto() throws Exception { - Class.forName("org.apache.distributedlog.DLSN"); + Class.forName("dlshade.org.apache.distributedlog.DLSN"); assertTrue(true); } @Test public void testDistributedLogCore() throws Exception { - Class.forName("org.apache.distributedlog.api.AsyncLogReader"); + Class.forName("dlshade.org.apache.distributedlog.api.AsyncLogReader"); assertTrue(true); }