eolivelli commented on a change in pull request #2868:
URL: https://github.com/apache/bookkeeper/pull/2868#discussion_r739100313
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCache.java
##########
@@ -59,6 +63,8 @@
private ByteBufAllocator allocator;
private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+ private Boolean isStorageShutdown;
Review comment:
"boolean" and not "Boolean"
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java
##########
@@ -238,6 +238,17 @@ public void testBookieCompaction() throws Exception {
assertEquals(newEntry3, res);
}
+ @Test
+ public void testReadsOpAfterShutdown() throws Exception {
+ SingleDirectoryDbLedgerStorage singleDirStorage = ((DbLedgerStorage)
storage).getLedgerStorageList().get(0);
+ singleDirStorage.shutdown();
+ try {
+ ByteBuf res = singleDirStorage.getEntry(4, 3);
+ } catch (IOException e) {
+ // This will pass the test since it is expected to throw
IOException
Review comment:
please add an assertion
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java
##########
@@ -238,6 +238,17 @@ public void testBookieCompaction() throws Exception {
assertEquals(newEntry3, res);
}
+ @Test
+ public void testReadsOpAfterShutdown() throws Exception {
+ SingleDirectoryDbLedgerStorage singleDirStorage = ((DbLedgerStorage)
storage).getLedgerStorageList().get(0);
+ singleDirStorage.shutdown();
+ try {
+ ByteBuf res = singleDirStorage.getEntry(4, 3);
Review comment:
please add "fail()" here, otherwise the test will pass in any case
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageTest.java
##########
@@ -238,6 +238,17 @@ public void testBookieCompaction() throws Exception {
assertEquals(newEntry3, res);
}
+ @Test
+ public void testReadsOpAfterShutdown() throws Exception {
+ SingleDirectoryDbLedgerStorage singleDirStorage = ((DbLedgerStorage)
storage).getLedgerStorageList().get(0);
+ singleDirStorage.shutdown();
Review comment:
`shutdown` should be idempotent if called twice
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/ReadCache.java
##########
@@ -59,6 +63,8 @@
private ByteBufAllocator allocator;
private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock();
+ private Boolean isStorageShutdown;
Review comment:
please add "volatile"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
