You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bookkeeper.apache.org by yo...@apache.org on 2022/08/01 13:44:46 UTC
[bookkeeper] branch branch-4.15 updated (e536d23947 -> 067dd4e614)
This is an automated email from the ASF dual-hosted git repository.
yong pushed a change to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
from e536d23947 Switch back ordered executor to LinkedBlockingQueue (#3384)
new 12c3325191 [conf] minorCompactionInterval should be greater than gcWaitTime (#2116)
new 688460b670 Ledger replicate supports throttle (#2778)
new aaf496d006 issue #2879 : let bookie quit if journal thread exit (#2887)
new f9e4a3b5b6 Issue #2908: Replace unsafe NoEntryException with IOException (#2909)
new 7f53f0b0e2 Make sure the LedgerHandle close callback can be completed when encounter exception (#2913)
new 308646bbfd Fix the infinite waiting for shutdown due to throttler limit (#2942)
new d58fd7adcb Fix wrong ledger id parse radix for index relocation file in IndexPersistenceMgr (#2944)
new b715cf77b9 Autorecovery to rereplicate empty ledgers (#3239)
new c0d970c2c7 [ISSUE 2637] Fix jvm_memory_direct_bytes_used metrics when using jdk11+ (#3252)
new bd25e6bb9b Fix the 3141 revert issue (#3283)
new db3ee4e9d9 validate diskUsageThreshold and diskUsageLwmThreshold (#3285)
new ee9a765fd7 Close journal channel in testJunkEndedJournal (#3307)
new 84ff7d599d Consider consider Bookie ID when validating the Cookie. (#3308)
new 40af8ee4a4 Fix maven javadoc generate (#3317)
new 02ca217605 [Client] Deduplicate error log for SSLException (#3320)
new 2d40a7c3b6 avoid init WriteSet when waitForWriteSetMs < 0. (#3325)
new 067dd4e614 Fix the build issue
The 17 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.github/workflows/pr-validation.yml | 3 +-
bookkeeper-common/pom.xml | 25 ++++
bookkeeper-server/pom.xml | 6 +
.../bookkeeper/bookie/AbstractLogCompactor.java | 32 +++++-
.../org/apache/bookkeeper/bookie/BookieImpl.java | 14 ++-
.../org/apache/bookkeeper/bookie/EntryLogger.java | 17 +--
.../bookkeeper/bookie/GarbageCollectorThread.java | 6 +-
.../bookkeeper/bookie/IndexPersistenceMgr.java | 5 +-
.../java/org/apache/bookkeeper/bookie/Journal.java | 12 ++
...package-info.java => JournalAliveListener.java} | 12 +-
.../bookkeeper/bookie/LedgerDirsMonitor.java | 9 ++
.../bookkeeper/bookie/LegacyCookieValidation.java | 37 +++---
.../bookie/storage/ldb/EntryLocationIndex.java | 1 +
.../apache/bookkeeper/client/BookKeeperAdmin.java | 2 +-
.../apache/bookkeeper/client/LedgerChecker.java | 9 +-
.../org/apache/bookkeeper/client/LedgerHandle.java | 35 +++---
.../apache/bookkeeper/client/LedgerHandleAdv.java | 12 +-
.../bookkeeper/conf/ClientConfiguration.java | 24 ++++
.../bookkeeper/conf/ServerConfiguration.java | 30 +++++
.../bookkeeper/proto/PerChannelBookieClient.java | 1 +
.../org/apache/bookkeeper/stats/package-info.java | 23 ----
.../bookkeeper/bookie/BookieJournalTest.java | 1 +
.../bookie/BookieMultipleJournalsTest.java | 41 +++++++
.../apache/bookkeeper/bookie/CompactionTest.java | 56 +++++++++
.../org/apache/bookkeeper/bookie/CookieTest.java | 24 ++++
.../bookkeeper/bookie/IndexPersistenceMgrTest.java | 127 +++++++++++++++++++--
.../apache/bookkeeper/bookie/LedgerCacheTest.java | 9 +-
.../bookkeeper/bookie/LedgerDirsManagerTest.java | 29 +++++
.../bookkeeper/bookie/datainteg/WriteSetsTest.java | 1 +
.../bookkeeper/client/BookieDecommissionTest.java | 107 +++++++++++++++--
.../bookkeeper/client/TestLedgerChecker.java | 24 ++--
.../bookkeeper/conf/TestServerConfiguration.java | 22 +++-
.../replication/BookieAutoRecoveryTest.java | 1 +
.../bookkeeper/test/BookKeeperClusterTestCase.java | 22 ++--
.../prometheus/PrometheusMetricsProvider.java | 30 ++---
.../prometheus/TestPrometheusMetricsProvider.java | 40 +++++++
bookkeeper-stats/pom.xml | 8 +-
.../apache/bookkeeper/stats/AlertStatsLogger.java | 0
conf/bk_server.conf | 2 +
pom.xml | 48 ++++++++
site3/website/scripts/javadoc-gen.sh | 2 +-
stream/distributedlog/pom.xml | 3 +-
42 files changed, 753 insertions(+), 159 deletions(-)
copy bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/{package-info.java => JournalAliveListener.java} (88%)
delete mode 100644 bookkeeper-server/src/main/java/org/apache/bookkeeper/stats/package-info.java
rename {bookkeeper-server => bookkeeper-stats}/src/main/java/org/apache/bookkeeper/stats/AlertStatsLogger.java (100%)
[bookkeeper] 06/17: Fix the infinite waiting for shutdown due to throttler limit (#2942)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 308646bbfd95c3858d352fbf9c3fc56ffe2eec68
Author: wenbingshen <ol...@gmail.com>
AuthorDate: Thu Jul 28 11:41:52 2022 +0800
Fix the infinite waiting for shutdown due to throttler limit (#2942)
Descriptions of the changes in this PR:
If the compactor is limited, the shutdown priority should be higher than waiting for RateLimiter.acquire.
According to @hangc0276 suggestion, when processing the shutdown logic of `GarbageCollectorThread`, we should check the status of the `newScanner.process` method. If the status is false, throw an `IOException` and stop compact immediately.
Master Issue: #2941
(cherry picked from commit 442e3bbad384fba9b09f8ea774e28562a8d5c6d7)
---
.../bookkeeper/bookie/AbstractLogCompactor.java | 32 ++++++++++++-
.../bookkeeper/bookie/GarbageCollectorThread.java | 2 +
.../apache/bookkeeper/bookie/CompactionTest.java | 56 ++++++++++++++++++++++
3 files changed, 88 insertions(+), 2 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/AbstractLogCompactor.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/AbstractLogCompactor.java
index 57ec8978cc..56d4ff5cb7 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/AbstractLogCompactor.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/AbstractLogCompactor.java
@@ -23,6 +23,9 @@ package org.apache.bookkeeper.bookie;
import com.google.common.util.concurrent.RateLimiter;
+import java.io.IOException;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
import org.apache.bookkeeper.conf.ServerConfiguration;
/**
@@ -60,6 +63,7 @@ public abstract class AbstractLogCompactor {
static class Throttler {
private final RateLimiter rateLimiter;
private final boolean isThrottleByBytes;
+ private final AtomicBoolean cancelled = new AtomicBoolean(false);
Throttler(ServerConfiguration conf) {
this.isThrottleByBytes = conf.getIsThrottleByBytes();
@@ -68,8 +72,32 @@ public abstract class AbstractLogCompactor {
}
// acquire. if bybytes: bytes of this entry; if byentries: 1.
- void acquire(int permits) {
- rateLimiter.acquire(this.isThrottleByBytes ? permits : 1);
+ boolean tryAcquire(int permits, long timeout, TimeUnit unit) {
+ return rateLimiter.tryAcquire(this.isThrottleByBytes ? permits : 1, timeout, unit);
+ }
+
+ // GC thread will check the status for the rate limiter
+ // If the compactor is being stopped by other threads,
+ // and the GC thread is still limited, the compact task will be stopped.
+ public void acquire(int permits) throws IOException {
+ long timeout = 100;
+ long start = System.currentTimeMillis();
+ while (!tryAcquire(permits, timeout, TimeUnit.MILLISECONDS)) {
+ if (cancelled.get()) {
+ throw new IOException("Failed to get permits takes "
+ + (System.currentTimeMillis() - start)
+ + " ms may be compactor has been shutting down");
+ }
+ try {
+ TimeUnit.MILLISECONDS.sleep(timeout);
+ } catch (InterruptedException e) {
+ // ignore
+ }
+ }
+ }
+
+ public void cancelledAcquire() {
+ cancelled.set(true);
}
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
index b98cd2a8f0..6e3d00c3d4 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
@@ -611,6 +611,8 @@ public class GarbageCollectorThread extends SafeRunnable {
}
LOG.info("Shutting down GarbageCollectorThread");
+ throttler.cancelledAcquire();
+ compactor.throttler.cancelledAcquire();
while (!compacting.compareAndSet(false, true)) {
// Wait till the thread stops compacting
Thread.sleep(100);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
index 765d8256a7..d39e6bf134 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CompactionTest.java
@@ -54,6 +54,7 @@ import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.function.Supplier;
import org.apache.bookkeeper.bookie.BookieException.EntryLogMetadataMapException;
import org.apache.bookkeeper.bookie.LedgerDirsManager.NoWritableLedgerDirException;
@@ -1389,6 +1390,61 @@ public abstract class CompactionTest extends BookKeeperClusterTestCase {
storage.getEntry(1, 1); // entry should exist
}
+ @Test
+ public void testCancelledCompactionWhenShuttingDown() throws Exception {
+ // prepare data
+ LedgerHandle[] lhs = prepareData(3, false);
+
+ // change compaction in low throughput
+ // restart bookies
+ restartBookies(c -> {
+ c.setIsThrottleByBytes(true);
+ c.setCompactionRateByBytes(ENTRY_SIZE / 1000);
+ c.setMinorCompactionThreshold(0.2f);
+ c.setMajorCompactionThreshold(0.5f);
+ return c;
+ });
+
+ // remove ledger2 and ledger3
+ // so entry log 1 and 2 would have ledger1 entries left
+ bkc.deleteLedger(lhs[1].getId());
+ bkc.deleteLedger(lhs[2].getId());
+ LOG.info("Finished deleting the ledgers contains most entries.");
+
+ getGCThread().triggerGC(true, false, false);
+ getGCThread().throttler.cancelledAcquire();
+ waitUntilTrue(() -> {
+ try {
+ return getGCThread().compacting.get();
+ } catch (Exception e) {
+ fail("Get GC thread failed");
+ }
+ return null;
+ }, () -> "Not attempting to complete", 10000, 200);
+
+ getGCThread().shutdown();
+ // after garbage collection shutdown, compaction should be cancelled when acquire permits
+ // and GC running flag should be false.
+ assertFalse(getGCThread().running);
+
+ }
+
+ private void waitUntilTrue(Supplier<Boolean> condition,
+ Supplier<String> msg,
+ long waitTime,
+ long pause) throws InterruptedException {
+ long startTime = System.currentTimeMillis();
+ while (true) {
+ if (condition.get()) {
+ return;
+ }
+ if (System.currentTimeMillis() > startTime + waitTime) {
+ fail(msg.get());
+ }
+ Thread.sleep(Math.min(waitTime, pause));
+ }
+ }
+
private LedgerManager getLedgerManager(final Set<Long> ledgers) {
LedgerManager manager = new LedgerManager() {
@Override
[bookkeeper] 07/17: Fix wrong ledger id parse radix for index relocation file in IndexPersistenceMgr (#2944)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit d58fd7adcbb6d5b4e7e39d9208c60be977f7fbef
Author: Kezhu Wang <ke...@gmail.com>
AuthorDate: Thu Jul 28 10:07:03 2022 +0800
Fix wrong ledger id parse radix for index relocation file in IndexPersistenceMgr (#2944)
Descriptions of the changes in this PR:
### Motivation
1. `IndexPersistenceMgr` fails to start after crashing index file movement.
### Changes
1. Add test to assert index file relocation
2. Fix wrong ledger id parse radix for index relocation file in IndexPersistenceMgr
Master Issue: None
(cherry picked from commit eef34477befc27e01f2e4531677236427ed859ef)
---
.../bookkeeper/bookie/IndexPersistenceMgr.java | 5 +-
.../bookkeeper/bookie/IndexPersistenceMgrTest.java | 127 +++++++++++++++++++--
2 files changed, 120 insertions(+), 12 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
index ec349f3123..71afd81f61 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/IndexPersistenceMgr.java
@@ -274,8 +274,9 @@ public class IndexPersistenceMgr {
// name is the HexString representation of the
// ledgerId.
String ledgerIdInHex = index.getName().replace(RLOC, "").replace(IDX, "");
+ long ledgerId = Long.parseLong(ledgerIdInHex, 16);
if (index.getName().endsWith(RLOC)) {
- if (findIndexFile(Long.parseLong(ledgerIdInHex)) != null) {
+ if (findIndexFile(ledgerId) != null) {
if (!index.delete()) {
LOG.warn("Deleting the rloc file " + index + " failed");
}
@@ -288,7 +289,7 @@ public class IndexPersistenceMgr {
}
}
}
- activeLedgers.put(Long.parseLong(ledgerIdInHex, 16), true);
+ activeLedgers.put(ledgerId, true);
}
}
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/IndexPersistenceMgrTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/IndexPersistenceMgrTest.java
index 898ef535b4..105c9c8b65 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/IndexPersistenceMgrTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/IndexPersistenceMgrTest.java
@@ -22,10 +22,13 @@ package org.apache.bookkeeper.bookie;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.spy;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufUtil;
@@ -59,7 +62,7 @@ import org.junit.Test;
public class IndexPersistenceMgrTest {
ServerConfiguration conf;
- File journalDir, ledgerDir;
+ File journalDir, ledgerDir1, ledgerDir2;
LedgerDirsManager ledgerDirsManager;
LedgerDirsMonitor ledgerMonitor;
@@ -68,17 +71,21 @@ public class IndexPersistenceMgrTest {
journalDir = File.createTempFile("IndexPersistenceMgr", "Journal");
journalDir.delete();
journalDir.mkdir();
- ledgerDir = File.createTempFile("IndexPersistenceMgr", "Ledger");
- ledgerDir.delete();
- ledgerDir.mkdir();
+ ledgerDir1 = File.createTempFile("IndexPersistenceMgr", "Ledger1");
+ ledgerDir1.delete();
+ ledgerDir1.mkdir();
+ ledgerDir2 = File.createTempFile("IndexPersistenceMgr", "Ledger2");
+ ledgerDir2.delete();
+ ledgerDir2.mkdir();
// Create current directories
BookieImpl.getCurrentDirectory(journalDir).mkdir();
- BookieImpl.getCurrentDirectory(ledgerDir).mkdir();
+ BookieImpl.getCurrentDirectory(ledgerDir1).mkdir();
+ BookieImpl.getCurrentDirectory(ledgerDir2).mkdir();
conf = new ServerConfiguration();
conf.setMetadataServiceUri(null);
conf.setJournalDirName(journalDir.getPath());
- conf.setLedgerDirNames(new String[] { ledgerDir.getPath() });
+ conf.setLedgerDirNames(new String[] { ledgerDir1.getPath(), ledgerDir2.getPath() });
ledgerDirsManager = new LedgerDirsManager(conf, conf.getLedgerDirs(),
new DiskChecker(conf.getDiskUsageThreshold(), conf.getDiskUsageWarnThreshold()));
@@ -92,7 +99,8 @@ public class IndexPersistenceMgrTest {
public void tearDown() throws Exception {
ledgerMonitor.shutdown();
FileUtils.deleteDirectory(journalDir);
- FileUtils.deleteDirectory(ledgerDir);
+ FileUtils.deleteDirectory(ledgerDir1);
+ FileUtils.deleteDirectory(ledgerDir2);
}
private IndexPersistenceMgr createIndexPersistenceManager(int openFileLimit) throws Exception {
@@ -338,12 +346,111 @@ public class IndexPersistenceMgrTest {
}
}
+ @Test
+ public void testIndexFileRelocation() throws Exception {
+ final long ledgerId = Integer.MAX_VALUE;
+ final String ledgerName = IndexPersistenceMgr.getLedgerName(ledgerId);
+
+ IndexPersistenceMgr indexPersistenceMgr = createIndexPersistenceManager(1);
+ preCreateFileInfoForLedgerInDir1(ledgerId, FileInfo.V1);
+
+ ledgerDirsManager.addToFilledDirs(BookieImpl.getCurrentDirectory(ledgerDir1));
+ indexPersistenceMgr.flushLedgerHeader(ledgerId);
+
+ File expectedIndexFile = new File(BookieImpl.getCurrentDirectory(ledgerDir2), ledgerName);
+ CachedFileInfo fileInfo = indexPersistenceMgr.getFileInfo(ledgerId, null);
+ assertTrue(fileInfo.isSameFile(expectedIndexFile));
+ assertFalse(fileInfo.isDeleted());
+
+ indexPersistenceMgr.close();
+
+ // Test startup after clean shutdown.
+ //
+ // Index file should stay in original location.
+ IndexPersistenceMgr indexPersistenceMgr2 = createIndexPersistenceManager(1);
+ CachedFileInfo fileInfo2 = indexPersistenceMgr2.getFileInfo(ledgerId, null);
+ assertTrue(fileInfo2.isSameFile(expectedIndexFile));
+ indexPersistenceMgr2.close();
+ }
+
+ @Test
+ public void testIndexFileRelocationCrashBeforeOriginalFileDeleted() throws Exception {
+ final long ledgerId = Integer.MAX_VALUE;
+ final String ledgerName = IndexPersistenceMgr.getLedgerName(ledgerId);
+ final String reason = "crash before original file deleted";
+
+ try {
+ IndexPersistenceMgr indexPersistenceMgr = createIndexPersistenceManager(1);
+ preCreateFileInfoForLedgerInDir1(ledgerId, FileInfo.V1);
+
+ CachedFileInfo fileInfo = spy(indexPersistenceMgr.getFileInfo(ledgerId, null));
+ doAnswer(invocation -> {
+ throw new RuntimeException(reason);
+ }).when(fileInfo).delete();
+ indexPersistenceMgr.readFileInfoCache.put(ledgerId, fileInfo);
+
+ ledgerDirsManager.addToFilledDirs(BookieImpl.getCurrentDirectory(ledgerDir1));
+ indexPersistenceMgr.flushLedgerHeader(ledgerId);
+ fail("should fail due to " + reason);
+ } catch (RuntimeException ex) {
+ assertEquals(reason, ex.getMessage());
+ }
+
+ // Test startup after:
+ // 1. relocation file created.
+ // 2. crashed with possible corrupted relocation file.
+ //
+ // Index file should stay in original location in this case.
+ IndexPersistenceMgr indexPersistenceMgr2 = createIndexPersistenceManager(1);
+ File expectedIndexFile = new File(BookieImpl.getCurrentDirectory(ledgerDir1), ledgerName);
+ CachedFileInfo fileInfo2 = indexPersistenceMgr2.getFileInfo(ledgerId, null);
+ assertTrue(fileInfo2.isSameFile(expectedIndexFile));
+ indexPersistenceMgr2.close();
+ }
+
+ @Test
+ public void testIndexFileRelocationCrashAfterOriginalFileDeleted() throws Exception {
+ final long ledgerId = Integer.MAX_VALUE;
+ final String ledgerName = IndexPersistenceMgr.getLedgerName(ledgerId);
+ final String reason = "crash after original file deleted";
+
+ try {
+ IndexPersistenceMgr indexPersistenceMgr = createIndexPersistenceManager(1);
+ preCreateFileInfoForLedgerInDir1(ledgerId, FileInfo.V1);
+
+ CachedFileInfo fileInfo = spy(indexPersistenceMgr.getFileInfo(ledgerId, null));
+ doAnswer(invocation -> {
+ invocation.callRealMethod();
+ throw new RuntimeException(reason);
+ }).when(fileInfo).delete();
+ indexPersistenceMgr.readFileInfoCache.put(ledgerId, fileInfo);
+
+ ledgerDirsManager.addToFilledDirs(BookieImpl.getCurrentDirectory(ledgerDir1));
+ indexPersistenceMgr.flushLedgerHeader(ledgerId);
+ fail("should fail due to " + reason);
+ } catch (RuntimeException ex) {
+ assertEquals(reason, ex.getMessage());
+ }
+
+ // Test startup after:
+ // 1. relocation file created, filled and synced.
+ // 2. original index file deleted.
+ // 3. crashed.
+ //
+ // Index file should stay in new location in this case.
+ IndexPersistenceMgr indexPersistenceMgr2 = createIndexPersistenceManager(1);
+ File expectedIndexFile = new File(BookieImpl.getCurrentDirectory(ledgerDir2), ledgerName);
+ CachedFileInfo fileInfo2 = indexPersistenceMgr2.getFileInfo(ledgerId, null);
+ assertTrue(fileInfo2.isSameFile(expectedIndexFile));
+ indexPersistenceMgr2.close();
+ }
+
void validateFileInfo(IndexPersistenceMgr indexPersistenceMgr, long ledgerId, int headerVersion)
throws IOException, GeneralSecurityException {
BookKeeper.DigestType digestType = BookKeeper.DigestType.CRC32;
boolean getUseV2WireProtocol = true;
- preCreateFileInfoForLedger(ledgerId, headerVersion);
+ preCreateFileInfoForLedgerInDir1(ledgerId, headerVersion);
DigestManager digestManager = DigestManager.instantiate(ledgerId, masterKey,
BookKeeper.DigestType.toProtoDigestType(digestType), UnpooledByteBufAllocator.DEFAULT,
getUseV2WireProtocol);
@@ -389,8 +496,8 @@ public class IndexPersistenceMgrTest {
}
}
- void preCreateFileInfoForLedger(long ledgerId, int headerVersion) throws IOException {
- File ledgerCurDir = BookieImpl.getCurrentDirectory(ledgerDir);
+ void preCreateFileInfoForLedgerInDir1(long ledgerId, int headerVersion) throws IOException {
+ File ledgerCurDir = BookieImpl.getCurrentDirectory(ledgerDir1);
String ledgerName = IndexPersistenceMgr.getLedgerName(ledgerId);
File indexFile = new File(ledgerCurDir, ledgerName);
indexFile.getParentFile().mkdirs();
[bookkeeper] 10/17: Fix the 3141 revert issue (#3283)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit bd25e6bb9b28e936ac698170779b3ed7ddc5737c
Author: xiaolong ran <xi...@tencent.com>
AuthorDate: Tue Jul 26 23:22:07 2022 +0800
Fix the 3141 revert issue (#3283)
Signed-off-by: xiaolongran <xi...@tencent.com>
### Motivation
In #3144, we reverted the changes of #2686, but after the revert, the self-increment behavior of deletedEntries was also removed, resulting in deletedEntries No assignment, always 0.
In #2686
<img width="1501" alt="image" src="https://user-images.githubusercontent.com/20965307/169231903-1a0bee03-f602-4c61-9c98-6b832f75648f.png">
In #3144
<img width="1352" alt="image" src="https://user-images.githubusercontent.com/20965307/169232028-658a1182-d8c5-4cfa-8f39-2ed7416ee508.png">
### Changes
- Add `++deletedEntries` for removeOffsetFromDeletedLedgers.
(cherry picked from commit 39a9c281a6ab329ba1aa8fb232ce81c2ad6719cc)
---
.../org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java | 1 +
1 file changed, 1 insertion(+)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java
index 41e021ac8c..468f268aab 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java
@@ -243,6 +243,7 @@ public class EntryLocationIndex implements Closeable {
}
batch.remove(keyToDelete.array);
++deletedEntriesInBatch;
+ ++deletedEntries;
}
if (deletedEntriesInBatch > DELETE_ENTRIES_BATCH_SIZE) {
[bookkeeper] 15/17: [Client] Deduplicate error log for SSLException (#3320)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 02ca2176057553013dceb73598e156379f52e9ca
Author: Michael Marshall <mm...@apache.org>
AuthorDate: Thu Jun 9 00:08:07 2022 -0500
[Client] Deduplicate error log for SSLException (#3320)
### Motivation
While testing #3310, I noticed that the `PerChannelBookieClient#exceptionCaught` logic contains redundant logs when the exception is an `SSLException`. This PR removes a redundant log from client.
Based on reading through the rest of the method, this should be a trivial change with no other side effects. My one question is how closing a channel and closing the context differ. Technically, returning early skips the `ctx.close()`, which could change the behavior.
### Changes
* Return early to prevent a redundant log
(cherry picked from commit bd827978ca7c72ad74ba5efc014f59aea3e6706c)
---
.../main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java | 1 +
1 file changed, 1 insertion(+)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
index 169307123c..0592378731 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
@@ -1297,6 +1297,7 @@ public class PerChannelBookieClient extends ChannelInboundHandlerAdapter {
if (c != null) {
closeChannel(c);
}
+ return;
}
if (cause instanceof IOException) {
[bookkeeper] 04/17: Issue #2908: Replace unsafe NoEntryException with IOException (#2909)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit f9e4a3b5b68f7505d7ed472d79bfaf973efa55ea
Author: Jack Vanlightly <va...@gmail.com>
AuthorDate: Mon Aug 1 09:11:35 2022 +0200
Issue #2908: Replace unsafe NoEntryException with IOException (#2909)
* Replace unsafe NoEntryException with IOException
Throwing a NoEntryException from the entry logger
for an entry that the index says should exist is
unsafe. It can cause ledger truncation during ledger
recovery. It only takes a single false NoSuchEntry
response to trigger truncation.
NoEntryException should only be thrown from inside
ledger storage if the entry is not found in the index.
* fix CI
Co-authored-by: chenhang <ch...@apache.org>
(cherry picked from commit 966b865973714c1bde596004bc4e5c1daa5a6d00)
---
.../java/org/apache/bookkeeper/bookie/EntryLogger.java | 17 ++++-------------
.../org/apache/bookkeeper/bookie/LedgerCacheTest.java | 9 +++++++--
2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
index 859f3e588b..d69e434ef5 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/EntryLogger.java
@@ -850,27 +850,18 @@ public class EntryLogger {
if (validateEntry) {
validateEntry(ledgerId, entryId, entryLogId, pos, sizeBuff);
}
- } catch (EntryLookupException.MissingEntryException entryLookupError) {
- throw new Bookie.NoEntryException("Short read from entrylog " + entryLogId,
- ledgerId, entryId);
} catch (EntryLookupException e) {
- throw new IOException(e.toString());
+ throw new IOException("Bad entry read from log file id: " + entryLogId, e);
}
ByteBuf data = allocator.buffer(entrySize, entrySize);
int rc = readFromLogChannel(entryLogId, fc, data, pos);
if (rc != entrySize) {
- // Note that throwing NoEntryException here instead of IOException is not
- // without risk. If all bookies in a quorum throw this same exception
- // the client will assume that it has reached the end of the ledger.
- // However, this may not be the case, as a very specific error condition
- // could have occurred, where the length of the entry was corrupted on all
- // replicas. However, the chance of this happening is very very low, so
- // returning NoEntryException is mostly safe.
data.release();
- throw new Bookie.NoEntryException("Short read for " + ledgerId + "@"
+ throw new IOException("Bad entry read from log file id: " + entryLogId,
+ new EntryLookupException("Short read for " + ledgerId + "@"
+ entryId + " in " + entryLogId + "@"
- + pos + "(" + rc + "!=" + entrySize + ")", ledgerId, entryId);
+ + pos + "(" + rc + "!=" + entrySize + ")"));
}
data.writerIndex(entrySize);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
index ba8e1b96d0..15947977c7 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
@@ -352,8 +352,13 @@ public class LedgerCacheTest {
// this is fine, means the ledger was written to the index cache, but not
// the entry log
} catch (IOException ioe) {
- LOG.info("Shouldn't have received IOException", ioe);
- fail("Shouldn't throw IOException, should say that entry is not found");
+ if (ioe.getCause() instanceof DefaultEntryLogger.EntryLookupException) {
+ // this is fine, means the ledger was not fully written to
+ // the entry log
+ } else {
+ LOG.info("Shouldn't have received IOException for entry {}", i, ioe);
+ fail("Shouldn't throw IOException, should say that entry is not found");
+ }
}
}
}
[bookkeeper] 17/17: Fix the build issue
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 067dd4e61457fd500e36a1a1fad03cb500484fd1
Author: Yong Zhang <zh...@gmail.com>
AuthorDate: Mon Aug 1 21:44:26 2022 +0800
Fix the build issue
---
bookkeeper-server/pom.xml | 5 +++++
.../test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java | 2 +-
pom.xml | 7 +++++++
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/bookkeeper-server/pom.xml b/bookkeeper-server/pom.xml
index 454311e195..10ae6281fd 100644
--- a/bookkeeper-server/pom.xml
+++ b/bookkeeper-server/pom.xml
@@ -191,6 +191,11 @@
<version>${project.parent.version}</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.awaitility</groupId>
+ <artifactId>awaitility</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
<plugins>
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
index 15947977c7..3350b88510 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerCacheTest.java
@@ -352,7 +352,7 @@ public class LedgerCacheTest {
// this is fine, means the ledger was written to the index cache, but not
// the entry log
} catch (IOException ioe) {
- if (ioe.getCause() instanceof DefaultEntryLogger.EntryLookupException) {
+ if (ioe.getCause() instanceof EntryLogger.EntryLookupException) {
// this is fine, means the ledger was not fully written to
// the entry log
} else {
diff --git a/pom.xml b/pom.xml
index 7b420b76da..aa42a9abce 100644
--- a/pom.xml
+++ b/pom.xml
@@ -211,6 +211,7 @@
<forkCount.variable>1</forkCount.variable>
<servlet-api.version>4.0.0</servlet-api.version>
<rxjava.version>3.0.1</rxjava.version>
+ <awaitility.version>4.0.3</awaitility.version>
</properties>
<!-- dependency definitions -->
@@ -751,6 +752,12 @@
<artifactId>jsoup</artifactId>
<version>${jsoup.version}</version>
</dependency>
+ <dependency>
+ <groupId>org.awaitility</groupId>
+ <artifactId>awaitility</artifactId>
+ <version>${awaitility.version}</version>
+ <scope>test</scope>
+ </dependency>
<!-- benchmark dependencies -->
<dependency>
[bookkeeper] 05/17: Make sure the LedgerHandle close callback can be completed when encounter exception (#2913)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 7f53f0b0e272b253d79ee66413e746e39fedf45c
Author: Penghui Li <pe...@apache.org>
AuthorDate: Fri Jul 29 14:56:48 2022 +0800
Make sure the LedgerHandle close callback can be completed when encounter exception (#2913)
* Make sure the LedgerHandle close callback can be completed when encounter exception.
(cherry picked from commit 05ca058444e83a349d3a002b8d893817691f36b0)
---
.../src/main/java/org/apache/bookkeeper/client/LedgerHandle.java | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
index 4d9090e589..bead7d1072 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
@@ -571,7 +571,12 @@ public class LedgerHandle implements WriteHandle {
// error out all pending adds during closing, the callbacks shouldn't be
// running under any bk locks.
- errorOutPendingAdds(rc, pendingAdds);
+ try {
+ errorOutPendingAdds(rc, pendingAdds);
+ } catch (Throwable e) {
+ closePromise.completeExceptionally(e);
+ return;
+ }
if (prevHandleState != HandleState.CLOSED) {
if (LOG.isDebugEnabled()) {
[bookkeeper] 12/17: Close journal channel in testJunkEndedJournal (#3307)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit ee9a765fd71cf271940dd6273ffd7ef54c0d86b0
Author: ZhangJian He <sh...@gmail.com>
AuthorDate: Mon Jun 6 13:41:47 2022 +0800
Close journal channel in testJunkEndedJournal (#3307)
(cherry picked from commit d3229c5cdb73c53de86d6e4b5a44a9eea9a5852d)
---
.../src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java | 1 +
1 file changed, 1 insertion(+)
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
index 7ddb9b0f26..e5a1d78d8c 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieJournalTest.java
@@ -539,6 +539,7 @@ public class BookieJournalTest {
JournalChannel jc = writeV2Journal(BookieImpl.getCurrentDirectory(journalDir), 0);
jc.getBufferedChannel().write(Unpooled.wrappedBuffer("JunkJunkJunk".getBytes()));
jc.getBufferedChannel().flushAndForceWrite(false);
+ jc.close();
writeIndexFileForLedger(ledgerDir, 1, "testPasswd".getBytes());
[bookkeeper] 08/17: Autorecovery to rereplicate empty ledgers (#3239)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit b715cf77b9ad1d8ece828a7799384124331147a3
Author: Andrey Yegorov <86...@users.noreply.github.com>
AuthorDate: Tue Jun 21 02:44:38 2022 -0700
Autorecovery to rereplicate empty ledgers (#3239)
(cherry picked from commit eadbdd4b6bfeef9924a3ff2c59fc3718cf3dc06b)
---
.../apache/bookkeeper/client/BookKeeperAdmin.java | 2 +-
.../apache/bookkeeper/client/LedgerChecker.java | 9 +-
.../bookkeeper/client/BookieDecommissionTest.java | 107 +++++++++++++++++++--
.../bookkeeper/client/TestLedgerChecker.java | 24 ++---
.../replication/BookieAutoRecoveryTest.java | 1 +
.../bookkeeper/test/BookKeeperClusterTestCase.java | 22 ++---
6 files changed, 132 insertions(+), 33 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
index cb3057a503..54cb566a30 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
@@ -1649,7 +1649,7 @@ public class BookKeeperAdmin implements AutoCloseable {
LOG.debug("Ledger: {} has been deleted", ledgerId);
return false;
} else {
- LOG.error("Got exception while trying to read LedgerMeatadata of " + ledgerId, e);
+ LOG.error("Got exception while trying to read LedgerMetadata of " + ledgerId, e);
throw new RuntimeException(e);
}
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java
index 87cbd5fa72..f3a49d31a6 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerChecker.java
@@ -40,7 +40,6 @@ import org.apache.bookkeeper.proto.BookkeeperInternalCallbacks.ReadEntryCallback
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-
/**
* A utility class to check the complete ledger and finds the UnderReplicated fragments if any.
*
@@ -238,7 +237,13 @@ public class LedgerChecker {
if (lastStored != LedgerHandle.INVALID_ENTRY_ID) {
throw new InvalidFragmentException();
}
- cb.operationComplete(BKException.Code.OK, fragment);
+
+ if (bookieWatcher.isBookieUnavailable(fragment.getAddress(bookieIndex))) {
+ // fragment is on this bookie, but already know it's unavailable, so skip the call
+ cb.operationComplete(BKException.Code.BookieHandleNotAvailableException, fragment);
+ } else {
+ cb.operationComplete(BKException.Code.OK, fragment);
+ }
} else if (bookieWatcher.isBookieUnavailable(fragment.getAddress(bookieIndex))) {
// fragment is on this bookie, but already know it's unavailable, so skip the call
cb.operationComplete(BKException.Code.BookieHandleNotAvailableException, fragment);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java
index d395ddf771..00127a2801 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieDecommissionTest.java
@@ -19,8 +19,11 @@
package org.apache.bookkeeper.client;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.fail;
import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
import lombok.extern.slf4j.Slf4j;
import org.apache.bookkeeper.bookie.BookieImpl;
import org.apache.bookkeeper.client.BKException.BKIllegalOpException;
@@ -29,6 +32,7 @@ import org.apache.bookkeeper.common.testing.annotations.FlakyTest;
import org.apache.bookkeeper.conf.ServerConfiguration;
import org.apache.bookkeeper.meta.UnderreplicatedLedger;
import org.apache.bookkeeper.meta.ZkLedgerUnderreplicationManager;
+import org.apache.bookkeeper.net.BookieId;
import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
import org.junit.Test;
@@ -44,19 +48,23 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
public BookieDecommissionTest() {
super(NUM_OF_BOOKIES, 480);
- baseConf.setOpenLedgerRereplicationGracePeriod(String.valueOf(30000));
+ baseConf.setOpenLedgerRereplicationGracePeriod(String.valueOf(1000));
setAutoRecoveryEnabled(true);
}
@FlakyTest("https://github.com/apache/bookkeeper/issues/502")
+ @Test
public void testDecommissionBookie() throws Exception {
ZkLedgerUnderreplicationManager urLedgerMgr = new ZkLedgerUnderreplicationManager(baseClientConf, zkc);
BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
+ List<Long> ledgerIds = new LinkedList<>();
+
int numOfLedgers = 2 * NUM_OF_BOOKIES;
int numOfEntries = 2 * NUM_OF_BOOKIES;
for (int i = 0; i < numOfLedgers; i++) {
LedgerHandle lh = bkc.createLedger(3, 2, digestType, PASSWORD.getBytes());
+ ledgerIds.add(lh.getId());
for (int j = 0; j < numOfEntries; j++) {
lh.addEntry("entry".getBytes());
}
@@ -67,6 +75,7 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
*/
for (int i = 0; i < numOfLedgers; i++) {
LedgerHandle emptylh = bkc.createLedger(3, 2, digestType, PASSWORD.getBytes());
+ ledgerIds.add(emptylh.getId());
emptylh.close();
}
@@ -88,7 +97,7 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
*/
bkAdmin.decommissionBookie(BookieImpl.getBookieId(killedBookieConf));
bkAdmin.triggerAudit();
- Thread.sleep(500);
+ Thread.sleep(5000);
Iterator<UnderreplicatedLedger> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
if (ledgersToRereplicate.hasNext()) {
while (ledgersToRereplicate.hasNext()) {
@@ -101,7 +110,7 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
killedBookieConf = killBookie(0);
bkAdmin.decommissionBookie(BookieImpl.getBookieId(killedBookieConf));
bkAdmin.triggerAudit();
- Thread.sleep(500);
+ Thread.sleep(5000);
ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
if (ledgersToRereplicate.hasNext()) {
while (ledgersToRereplicate.hasNext()) {
@@ -111,6 +120,10 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
fail("There are not supposed to be any underreplicatedledgers");
}
bkAdmin.close();
+
+ for (Long id: ledgerIds) {
+ verifyNoFragmentsOnBookie(id, BookieImpl.getBookieId(killedBookieConf));
+ }
}
@Test
@@ -130,11 +143,16 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
lh4.addEntry(j, "data".getBytes());
}
+ // avoiding autorecovery fencing the ledger
+ servers.forEach(srv -> srv.stopAutoRecovery());
+
startNewBookie();
assertEquals("Number of Available Bookies", NUM_OF_BOOKIES + 1, bkAdmin.getAvailableBookies().size());
- ServerConfiguration killedBookieConf = killBookie(0);
+ BookieId killedBookieId = getBookie(0);
+ log.warn("Killing bookie {}", killedBookieId);
+ killBookie(0);
/*
* since one of the bookie is killed, ensemble change happens when next
@@ -152,16 +170,24 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
lh1.close();
lh2.close();
+ servers.forEach(srv -> {
+ try {
+ srv.startAutoRecovery();
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ });
+
/*
* If the last fragment of the ledger is underreplicated and if the
* ledger is not closed then it will remain underreplicated for
- * openLedgerRereplicationGracePeriod (by default 30 secs). For more
+ * openLedgerRereplicationGracePeriod (by default 30 secs, 1 in the test). For more
* info. Check BOOKKEEPER-237 and BOOKKEEPER-325. But later
* ReplicationWorker will fence the ledger.
*/
- bkAdmin.decommissionBookie(BookieImpl.getBookieId(killedBookieConf));
+ bkAdmin.decommissionBookie(killedBookieId);
bkAdmin.triggerAudit();
- Thread.sleep(500);
+ Thread.sleep(5000);
Iterator<UnderreplicatedLedger> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
if (ledgersToRereplicate.hasNext()) {
while (ledgersToRereplicate.hasNext()) {
@@ -171,6 +197,73 @@ public class BookieDecommissionTest extends BookKeeperClusterTestCase {
fail("There are not supposed to be any underreplicatedledgers");
}
bkAdmin.close();
+
+ verifyNoFragmentsOnBookie(1L, killedBookieId);
+ verifyNoFragmentsOnBookie(2L, killedBookieId);
+ verifyNoFragmentsOnBookie(3L, killedBookieId);
+ verifyNoFragmentsOnBookie(4L, killedBookieId);
+ }
+
+ @Test
+ public void testDecommissionForEmptyLedgers() throws Exception {
+ ZkLedgerUnderreplicationManager urLedgerMgr = new ZkLedgerUnderreplicationManager(baseClientConf, zkc);
+ BookKeeperAdmin bkAdmin = new BookKeeperAdmin(zkUtil.getZooKeeperConnectString());
+
+ LedgerHandle lh1 = bkc.createLedgerAdv(1L, numBookies, numBookies - 1, numBookies - 1,
+ digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh2 = bkc.createLedgerAdv(2L, numBookies, numBookies - 1, numBookies - 1,
+ digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh3 = bkc.createLedgerAdv(3L, numBookies, numBookies - 1, numBookies - 1,
+ digestType, PASSWORD.getBytes(), null);
+ LedgerHandle lh4 = bkc.createLedgerAdv(4L, numBookies, numBookies - 1, numBookies - 1,
+ digestType, PASSWORD.getBytes(), null);
+
+ lh1.close();
+ lh2.close();
+
+ startNewBookie();
+
+ assertEquals("Number of Available Bookies", NUM_OF_BOOKIES + 1, bkAdmin.getAvailableBookies().size());
+
+ BookieId killedBookieId = getBookie(0);
+ log.warn("Killing bookie {}", killedBookieId);
+ killBookie(0);
+ assertEquals("Number of Available Bookies", NUM_OF_BOOKIES, bkAdmin.getAvailableBookies().size());
+
+ bkAdmin.decommissionBookie(killedBookieId);
+ bkAdmin.triggerAudit();
+ Thread.sleep(5000);
+ Iterator<UnderreplicatedLedger> ledgersToRereplicate = urLedgerMgr.listLedgersToRereplicate(null);
+ if (ledgersToRereplicate.hasNext()) {
+ while (ledgersToRereplicate.hasNext()) {
+ long ledgerId = ledgersToRereplicate.next().getLedgerId();
+ log.error("Ledger: {} is underreplicated which is not expected. {}",
+ ledgerId, ledgersToRereplicate.next().getReplicaList());
+ }
+ fail("There are not supposed to be any underreplicatedledgers");
+ }
+ bkAdmin.close();
+
+ verifyNoFragmentsOnBookie(1L, killedBookieId);
+ verifyNoFragmentsOnBookie(2L, killedBookieId);
+ verifyNoFragmentsOnBookie(3L, killedBookieId);
+ verifyNoFragmentsOnBookie(4L, killedBookieId);
+
+ lh3.close();
+ lh4.close();
+ }
+
+ private void verifyNoFragmentsOnBookie(long ledgerId, BookieId bookieId) throws BKException, InterruptedException {
+ LedgerHandle lh = bkc.openLedgerNoRecovery(ledgerId, digestType, PASSWORD.getBytes());
+ log.error("Ledger {} metadata: {}", ledgerId, lh.getLedgerMetadata());
+
+ lh.getLedgerMetadata().getAllEnsembles().forEach((num, bookies) -> {
+ bookies.forEach(id -> {
+ assertNotEquals(bookieId, id);
+ });
+ });
+
+ lh.close();
}
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java
index 9da4cb3fd7..d78d1599f9 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestLedgerChecker.java
@@ -169,7 +169,7 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
LOG.info("unreplicated fragment: {}", r);
}
- assertEquals("Should not have any missing fragment", 0, result.size());
+ assertEquals("Empty fragment should be considered missing", 1, result.size());
}
/**
@@ -203,7 +203,7 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
LOG.info("unreplicated fragment: {}", r);
}
- assertEquals("There should be 1 fragments", 1, result.size());
+ assertEquals("Empty fragment should be considered missing", 2, result.size());
assertEquals("There should be 2 failed bookies in the fragment",
2, result.iterator().next().getBookiesIndexes().size());
}
@@ -314,8 +314,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
Set<LedgerFragment> result = getUnderReplicatedFragments(lh);
assertNotNull("Result shouldn't be null", result);
assertEquals("There should be 1 fragments.", 1, result.size());
- assertEquals("There should be 2 failed bookies in the fragment",
- 2, result.iterator().next().getBookiesIndexes().size());
+ assertEquals("There should be 3 failed bookies in the fragment",
+ 3, result.iterator().next().getBookiesIndexes().size());
}
/**
@@ -421,8 +421,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
Set<LedgerFragment> result = getUnderReplicatedFragments(lh1);
assertNotNull("Result shouldn't be null", result);
- assertEquals("There should be 0 fragment. But returned fragments are "
- + result, 0, result.size());
+ assertEquals("Empty fragment should be considered missing"
+ + result, 1, result.size());
}
/**
@@ -450,8 +450,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
Set<LedgerFragment> result = getUnderReplicatedFragments(lh1);
assertNotNull("Result shouldn't be null", result);
- assertEquals("There should be 0 fragment. But returned fragments are "
- + result, 0, result.size());
+ assertEquals("Empty fragment should be considered missing"
+ + result, 1, result.size());
lh1.close();
// kill bookie 1
@@ -469,8 +469,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
assertNotNull("Result shouldn't be null", result);
assertEquals("There should be 1 fragment. But returned fragments are "
+ result, 1, result.size());
- assertEquals("There should be 1 failed bookies in the fragment",
- 1, result.iterator().next().getBookiesIndexes().size());
+ assertEquals("There should be 2 failed bookies in the fragment",
+ 2, result.iterator().next().getBookiesIndexes().size());
lh1.close();
// kill bookie 0
@@ -488,8 +488,8 @@ public class TestLedgerChecker extends BookKeeperClusterTestCase {
assertNotNull("Result shouldn't be null", result);
assertEquals("There should be 1 fragment. But returned fragments are "
+ result, 1, result.size());
- assertEquals("There should be 2 failed bookies in the fragment",
- 2, result.iterator().next().getBookiesIndexes().size());
+ assertEquals("There should be 3 failed bookies in the fragment",
+ 3, result.iterator().next().getBookiesIndexes().size());
lh1.close();
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieAutoRecoveryTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieAutoRecoveryTest.java
index a8e89dcf0c..a19be543a4 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieAutoRecoveryTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/BookieAutoRecoveryTest.java
@@ -383,6 +383,7 @@ public class BookieAutoRecoveryTest extends BookKeeperClusterTestCase {
LOG.info("Killing last bookie, {}, in ensemble {}", replicaToKill,
lh.getLedgerMetadata().getAllEnsembles().get(0L));
killBookie(replicaToKill);
+ startNewBookie();
getAuditor(10, TimeUnit.SECONDS).submitAuditTask().get(); // ensure auditor runs
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java
index 1b14f17255..cb1cdaa4e8 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookKeeperClusterTestCase.java
@@ -108,7 +108,7 @@ public abstract class BookKeeperClusterTestCase {
// BookKeeper related variables
protected final TmpDirs tmpDirs = new TmpDirs();
- private final List<ServerTester> servers = new LinkedList<>();
+ protected final List<ServerTester> servers = new LinkedList<>();
protected int numBookies;
protected BookKeeperTestClient bkc;
@@ -835,7 +835,7 @@ public abstract class BookKeeperClusterTestCase {
private AutoRecoveryMain autoRecovery;
- ServerTester(ServerConfiguration conf) throws Exception {
+ public ServerTester(ServerConfiguration conf) throws Exception {
this.conf = conf;
provider = new TestStatsProvider();
@@ -879,7 +879,7 @@ public abstract class BookKeeperClusterTestCase {
autoRecovery = null;
}
- ServerTester(ServerConfiguration conf, Bookie b) throws Exception {
+ public ServerTester(ServerConfiguration conf, Bookie b) throws Exception {
this.conf = conf;
provider = new TestStatsProvider();
@@ -897,20 +897,20 @@ public abstract class BookKeeperClusterTestCase {
autoRecovery = null;
}
- void startAutoRecovery() throws Exception {
+ public void startAutoRecovery() throws Exception {
LOG.debug("Starting Auditor Recovery for the bookie: {}", address);
autoRecovery = new AutoRecoveryMain(conf);
autoRecovery.start();
}
- void stopAutoRecovery() {
+ public void stopAutoRecovery() {
if (autoRecovery != null) {
LOG.debug("Shutdown Auditor Recovery for the bookie: {}", address);
autoRecovery.shutdown();
}
}
- Auditor getAuditor() {
+ public Auditor getAuditor() {
if (autoRecovery != null) {
return autoRecovery.getAuditor();
} else {
@@ -918,7 +918,7 @@ public abstract class BookKeeperClusterTestCase {
}
}
- ReplicationWorker getReplicationWorker() {
+ public ReplicationWorker getReplicationWorker() {
if (autoRecovery != null) {
return autoRecovery.getReplicationWorker();
} else {
@@ -926,7 +926,7 @@ public abstract class BookKeeperClusterTestCase {
}
}
- ServerConfiguration getConfiguration() {
+ public ServerConfiguration getConfiguration() {
return conf;
}
@@ -934,15 +934,15 @@ public abstract class BookKeeperClusterTestCase {
return server;
}
- TestStatsProvider getStatsProvider() {
+ public TestStatsProvider getStatsProvider() {
return provider;
}
- BookieSocketAddress getAddress() {
+ public BookieSocketAddress getAddress() {
return address;
}
- void shutdown() throws Exception {
+ public void shutdown() throws Exception {
server.shutdown();
if (ledgerManager != null) {
[bookkeeper] 13/17: Consider consider Bookie ID when validating the Cookie. (#3308)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 84ff7d599da98cd172b30ed9c51083b0d01d8f8d
Author: Raúl Gracia <ra...@emc.com>
AuthorDate: Tue Jun 7 12:53:54 2022 +0200
Consider consider Bookie ID when validating the Cookie. (#3308)
Signed-off-by: Raúl Gracia <ra...@emc.com>
(cherry picked from commit b477f8d506ea493f211dfc96e40d647f5254d3ca)
---
.../bookkeeper/bookie/LegacyCookieValidation.java | 37 ++++++++++++----------
.../org/apache/bookkeeper/bookie/CookieTest.java | 24 ++++++++++++++
2 files changed, 45 insertions(+), 16 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LegacyCookieValidation.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LegacyCookieValidation.java
index 481e34294c..803d48e1aa 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LegacyCookieValidation.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LegacyCookieValidation.java
@@ -136,23 +136,28 @@ public class LegacyCookieValidation implements CookieValidation {
// we are checking all possibilities here, so we don't need to fail if we can only get
// loopback address. it will fail anyway when the bookie attempts to listen on loopback address.
try {
- // ip address
- addresses.add(BookieImpl.getBookieAddress(
- new ServerConfiguration(conf)
- .setUseHostNameAsBookieID(false)
- .setAdvertisedAddress(null)
- .setAllowLoopback(true)
- ).toBookieId());
- // host name
- addresses.add(BookieImpl.getBookieAddress(
- new ServerConfiguration(conf)
- .setUseHostNameAsBookieID(true)
- .setAdvertisedAddress(null)
- .setAllowLoopback(true)
- ).toBookieId());
- // advertised address
- if (null != conf.getAdvertisedAddress()) {
+ if (null != conf.getBookieId()) {
+ // If BookieID is configured, it takes precedence over default network information used as id.
addresses.add(BookieImpl.getBookieId(conf));
+ } else {
+ // ip address
+ addresses.add(BookieImpl.getBookieAddress(
+ new ServerConfiguration(conf)
+ .setUseHostNameAsBookieID(false)
+ .setAdvertisedAddress(null)
+ .setAllowLoopback(true)
+ ).toBookieId());
+ // host name
+ addresses.add(BookieImpl.getBookieAddress(
+ new ServerConfiguration(conf)
+ .setUseHostNameAsBookieID(true)
+ .setAdvertisedAddress(null)
+ .setAllowLoopback(true)
+ ).toBookieId());
+ // advertised address
+ if (null != conf.getAdvertisedAddress()) {
+ addresses.add(BookieImpl.getBookieAddress(conf).toBookieId());
+ }
}
} catch (UnknownHostException e) {
throw new BookieException.UnknownBookieIdException(e);
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
index 0bbdeab9ff..5dd353bfad 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/CookieTest.java
@@ -36,6 +36,7 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
+import java.util.Random;
import java.util.Set;
import org.apache.bookkeeper.bookie.BookieException.InvalidCookieException;
import org.apache.bookkeeper.client.BookKeeperAdmin;
@@ -52,6 +53,7 @@ import org.apache.bookkeeper.versioning.LongVersion;
import org.apache.bookkeeper.versioning.Version;
import org.apache.bookkeeper.versioning.Versioned;
import org.apache.commons.io.FileUtils;
+import org.junit.Assert;
import org.junit.Test;
import org.slf4j.Logger;
@@ -755,4 +757,26 @@ public class CookieTest extends BookKeeperClusterTestCase {
Cookie cookie = zkCookie.getValue();
cookie.deleteFromRegistrationManager(rm, conf, zkCookie.getVersion());
}
+
+ /**
+ * Tests that custom Bookie Id is properly set in the Cookie (via {@link LegacyCookieValidation}).
+ */
+ @Test
+ public void testBookieIdSetting() throws Exception {
+ final String customBookieId = "myCustomBookieId" + new Random().nextInt();
+ ServerConfiguration conf = TestBKConfiguration.newServerConfiguration();
+ conf.setJournalDirName(newDirectory())
+ .setLedgerDirNames(new String[] { newDirectory() , newDirectory() })
+ .setBookiePort(bookiePort)
+ .setBookieId(customBookieId)
+ .setMetadataServiceUri(zkUtil.getMetadataServiceUri());
+ validateConfig(conf);
+ Versioned<Cookie> zkCookie = Cookie.readFromRegistrationManager(rm, conf);
+ Version version1 = zkCookie.getVersion();
+ assertTrue("Invalid type expected ZkVersion type",
+ version1 instanceof LongVersion);
+ Cookie cookie = zkCookie.getValue();
+ cookie.writeToRegistrationManager(rm, conf, version1);
+ Assert.assertTrue(cookie.toString().contains(customBookieId));
+ }
}
[bookkeeper] 03/17: issue #2879 : let bookie quit if journal thread exit (#2887)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit aaf496d006fc70a527dfe47b4c7048de4d327580
Author: AloysZhang <lo...@gmail.com>
AuthorDate: Sun Jul 31 11:30:45 2022 +0800
issue #2879 : let bookie quit if journal thread exit (#2887)
Descriptions of the changes in this PR:
fix #2879
This pull request let bookie quit when there's journal thread exit
### Motivation
As described in #2879, now if a bookie has multi journal directories means it has multi journal thread. Once a journal thread exits, the bookie will be unhealthy due to the block of all bookie-io threads, and then the bookie will not work but progress is still alive.
This pull request tries to fix this problem.
### Changes
check the journal thread alive in a fixed interval, let bookie quit once there's a journal thread exit
(cherry picked from commit 67208fb74181faa640e793cd5757712fd9b5d9d5)
---
.../org/apache/bookkeeper/bookie/BookieImpl.java | 14 +++++---
.../java/org/apache/bookkeeper/bookie/Journal.java | 12 +++++++
.../bookkeeper/bookie/JournalAliveListener.java | 28 +++++++++++++++
.../bookie/BookieMultipleJournalsTest.java | 41 ++++++++++++++++++++++
.../bookkeeper/bookie/datainteg/WriteSetsTest.java | 1 +
5 files changed, 91 insertions(+), 5 deletions(-)
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 9bc084ee92..dd5eeb49b7 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
@@ -50,6 +50,7 @@ import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.apache.bookkeeper.bookie.BookieException.DiskPartitionDuplicationException;
@@ -425,11 +426,13 @@ public class BookieImpl extends BookieCriticalThread implements Bookie {
}
}
+ JournalAliveListener journalAliveListener =
+ () -> BookieImpl.this.triggerBookieShutdown(ExitCode.BOOKIE_EXCEPTION);
// instantiate the journals
journals = Lists.newArrayList();
for (int i = 0; i < journalDirectories.size(); i++) {
journals.add(new Journal(i, journalDirectories.get(i),
- conf, ledgerDirsManager, statsLogger.scope(JOURNAL_SCOPE), allocator));
+ conf, ledgerDirsManager, statsLogger.scope(JOURNAL_SCOPE), allocator, journalAliveListener));
}
this.entryLogPerLedgerEnabled = conf.isEntryLogPerLedgerEnabled();
@@ -828,12 +831,13 @@ public class BookieImpl extends BookieCriticalThread implements Bookie {
public int shutdown() {
return shutdown(ExitCode.OK);
}
-
// internal shutdown method to let shutdown bookie gracefully
// when encountering exception
- synchronized int shutdown(int exitCode) {
+ ReentrantLock lock = new ReentrantLock(true);
+ int shutdown(int exitCode) {
+ lock.lock();
try {
- if (isRunning()) { // avoid shutdown twice
+ if (isRunning()) {
// the exitCode only set when first shutdown usually due to exception found
LOG.info("Shutting down Bookie-{} with exitCode {}",
conf.getBookiePort(), exitCode);
@@ -854,7 +858,6 @@ public class BookieImpl extends BookieCriticalThread implements Bookie {
for (Journal journal : journals) {
journal.shutdown();
}
- this.join();
// Shutdown the EntryLogger which has the GarbageCollector Thread running
ledgerStorage.shutdown();
@@ -871,6 +874,7 @@ public class BookieImpl extends BookieCriticalThread implements Bookie {
LOG.error("Got Exception while trying to shutdown Bookie", e);
throw e;
} finally {
+ lock.unlock();
// setting running to false here, so watch thread
// in bookie server know it only after bookie shut down
stateManager.close();
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
index a4c91e9483..193b557312 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Journal.java
@@ -687,6 +687,8 @@ public class Journal extends BookieCriticalThread implements CheckpointSource {
// Expose Stats
private final JournalStats journalStats;
+ private JournalAliveListener journalAliveListener;
+
public Journal(int journalIndex, File journalDirectory, ServerConfiguration conf,
LedgerDirsManager ledgerDirsManager) {
this(journalIndex, journalDirectory, conf, ledgerDirsManager, NullStatsLogger.INSTANCE,
@@ -767,6 +769,13 @@ public class Journal extends BookieCriticalThread implements CheckpointSource {
() -> memoryLimitController.currentUsage());
}
+ public Journal(int journalIndex, File journalDirectory, ServerConfiguration conf,
+ LedgerDirsManager ledgerDirsManager, StatsLogger statsLogger,
+ ByteBufAllocator allocator, JournalAliveListener journalAliveListener) {
+ this(journalIndex, journalDirectory, conf, ledgerDirsManager, statsLogger, allocator);
+ this.journalAliveListener = journalAliveListener;
+ }
+
JournalStats getJournalStats() {
return this.journalStats;
}
@@ -1227,6 +1236,9 @@ public class Journal extends BookieCriticalThread implements CheckpointSource {
// close will flush the file system cache making any previous
// cached writes durable so this is fine as well.
IOUtils.close(LOG, bc);
+ if (journalAliveListener != null) {
+ journalAliveListener.onJournalExit();
+ }
}
LOG.info("Journal exited loop!");
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalAliveListener.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalAliveListener.java
new file mode 100644
index 0000000000..ef73edc0ea
--- /dev/null
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/JournalAliveListener.java
@@ -0,0 +1,28 @@
+/*
+ *
+ * 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;
+
+/**
+ * Listener for journal alive.
+ * */
+public interface JournalAliveListener {
+ void onJournalExit();
+}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieMultipleJournalsTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieMultipleJournalsTest.java
index bc30246637..a6a9a67e70 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieMultipleJournalsTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/BookieMultipleJournalsTest.java
@@ -21,8 +21,10 @@
package org.apache.bookkeeper.bookie;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import java.io.File;
+import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
@@ -31,7 +33,9 @@ import org.apache.bookkeeper.client.BookKeeper.DigestType;
import org.apache.bookkeeper.client.LedgerEntry;
import org.apache.bookkeeper.client.LedgerHandle;
import org.apache.bookkeeper.conf.ServerConfiguration;
+import org.apache.bookkeeper.proto.BookieServer;
import org.apache.bookkeeper.test.BookKeeperClusterTestCase;
+import org.awaitility.Awaitility;
import org.junit.Test;
/**
@@ -57,6 +61,43 @@ public class BookieMultipleJournalsTest extends BookKeeperClusterTestCase {
return conf;
}
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testJournalExit() throws Exception {
+
+ LedgerHandle ledgerHandle = bkc.createLedger(1, 1, DigestType.CRC32, new byte[0]);
+ for (int i = 0; i < 10; i++) {
+ ledgerHandle.addEntry(("entry-" + i).getBytes());
+ }
+
+ BookieServer bookieServer = serverByIndex(0);
+ BookieImpl bookie = (BookieImpl) bookieServer.getBookie();
+ Field journalList = bookie.getClass().getDeclaredField("journals");
+ journalList.setAccessible(true);
+ List<Journal> journals = (List<Journal>) journalList.get(bookie);
+ journals.get(0).interrupt();
+ Awaitility.await().untilAsserted(() -> assertFalse(bookie.isRunning()));
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testJournalExitAndShutdown() throws Exception {
+
+ LedgerHandle ledgerHandle = bkc.createLedger(1, 1, DigestType.CRC32, new byte[0]);
+ for (int i = 0; i < 10; i++) {
+ ledgerHandle.addEntry(("entry-" + i).getBytes());
+ }
+
+ BookieServer bookieServer = serverByIndex(0);
+ BookieImpl bookie = (BookieImpl) bookieServer.getBookie();
+ Field journalList = bookie.getClass().getDeclaredField("journals");
+ journalList.setAccessible(true);
+ List<Journal> journals = (List<Journal>) journalList.get(bookie);
+ journals.get(0).interrupt();
+ bookie.shutdown(ExitCode.OK);
+ Awaitility.await().untilAsserted(() -> assertFalse(bookie.isRunning()));
+ }
+
@Test
public void testMultipleWritesAndBookieRestart() throws Exception {
// Creates few ledgers so that writes are spread across all journals
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/WriteSetsTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/WriteSetsTest.java
index 1a82b0bde0..139351b950 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/WriteSetsTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/datainteg/WriteSetsTest.java
@@ -158,6 +158,7 @@ public class WriteSetsTest {
}
}
+ @SuppressWarnings("deprecation")
private static void assertContentsMatch(ImmutableList<Integer> writeSet,
DistributionSchedule.WriteSet distWriteSet)
throws Exception {
[bookkeeper] 09/17: [ISSUE 2637] Fix jvm_memory_direct_bytes_used metrics when using jdk11+ (#3252)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit c0d970c2c7a5044b5cc11dc4b2c155a87b61d11e
Author: ZhangJian He <sh...@gmail.com>
AuthorDate: Fri Jul 29 18:38:27 2022 +0800
[ISSUE 2637] Fix jvm_memory_direct_bytes_used metrics when using jdk11+ (#3252)
Fix #2637 #3247
### Motivation
The mertics about `jvm_memory_direct_bytes_used` is acquired by netty's `PlatformDependent#DIRECT_MEMORY_COUNTER`. Which can only acquired the memory used by netty.
### Changes
- use `java.nio.Bits#RESERVED_MEMORY` for jvm direct memory metrics.
- add tests to ensure `jvm_memory_direct_bytes_max` and `jvm_memory_direct_bytes_used` gets value.
(cherry picked from commit cefe9d44dad3558cd4cc30547007c3c41b9bfbec)
---
.../prometheus/PrometheusMetricsProvider.java | 30 ++++++----------
.../prometheus/TestPrometheusMetricsProvider.java | 40 ++++++++++++++++++++++
2 files changed, 50 insertions(+), 20 deletions(-)
diff --git a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java
index 974d914e3e..ab81992531 100644
--- a/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java
+++ b/bookkeeper-stats-providers/prometheus-metrics-provider/src/main/java/org/apache/bookkeeper/stats/prometheus/PrometheusMetricsProvider.java
@@ -30,19 +30,19 @@ import io.prometheus.client.hotspot.GarbageCollectorExports;
import io.prometheus.client.hotspot.MemoryPoolsExports;
import io.prometheus.client.hotspot.StandardExports;
import io.prometheus.client.hotspot.ThreadExports;
-
import java.io.IOException;
import java.io.Writer;
-import java.lang.reflect.Field;
+import java.lang.management.BufferPoolMXBean;
+import java.lang.management.ManagementFactory;
import java.net.InetSocketAddress;
import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicLong;
-
import org.apache.bookkeeper.stats.StatsLogger;
import org.apache.bookkeeper.stats.StatsProvider;
import org.apache.bookkeeper.stats.ThreadRegistry;
@@ -130,7 +130,7 @@ public class PrometheusMetricsProvider implements StatsProvider {
registerMetrics(Gauge.build("jvm_memory_direct_bytes_used", "-").create().setChild(new Child() {
@Override
public double get() {
- return directMemoryUsage != null ? directMemoryUsage.longValue() : Double.NaN;
+ return poolMxBeanOp.isPresent() ? poolMxBeanOp.get().getMemoryUsed() : Double.NaN;
}
}));
@@ -215,21 +215,11 @@ public class PrometheusMetricsProvider implements StatsProvider {
private static final Logger log = LoggerFactory.getLogger(PrometheusMetricsProvider.class);
- /*
- * Try to get Netty counter of used direct memory. This will be correct, unlike the JVM values.
- */
- private static final AtomicLong directMemoryUsage;
- static {
- AtomicLong tmpDirectMemoryUsage = null;
+ private static final Optional<BufferPoolMXBean> poolMxBeanOp;
- try {
- Field field = PlatformDependent.class.getDeclaredField("DIRECT_MEMORY_COUNTER");
- field.setAccessible(true);
- tmpDirectMemoryUsage = (AtomicLong) field.get(null);
- } catch (Throwable t) {
- log.warn("Failed to access netty DIRECT_MEMORY_COUNTER field {}", t.getMessage());
- }
-
- directMemoryUsage = tmpDirectMemoryUsage;
+ static {
+ List<BufferPoolMXBean> platformMXBeans = ManagementFactory.getPlatformMXBeans(BufferPoolMXBean.class);
+ poolMxBeanOp = platformMXBeans.stream()
+ .filter(bufferPoolMXBean -> bufferPoolMXBean.getName().equals("direct")).findAny();
}
}
diff --git a/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusMetricsProvider.java b/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusMetricsProvider.java
index df954e64d9..999be26cbb 100644
--- a/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusMetricsProvider.java
+++ b/bookkeeper-stats-providers/prometheus-metrics-provider/src/test/java/org/apache/bookkeeper/stats/prometheus/TestPrometheusMetricsProvider.java
@@ -21,11 +21,16 @@ import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
+import java.io.StringWriter;
+import java.nio.ByteBuffer;
import java.util.Collections;
+import java.util.HashMap;
+
import lombok.Cleanup;
import org.apache.bookkeeper.stats.Counter;
import org.apache.bookkeeper.stats.StatsLogger;
import org.apache.commons.configuration.PropertiesConfiguration;
+import org.junit.Assert;
import org.junit.Test;
/**
@@ -111,4 +116,39 @@ public class TestPrometheusMetricsProvider {
assertEquals(1, provider.counters.size());
}
+ @Test
+ public void testJvmDirectMemoryMetrics() throws Exception {
+ PropertiesConfiguration config = new PropertiesConfiguration();
+ config.setProperty(PrometheusMetricsProvider.PROMETHEUS_STATS_HTTP_ENABLE, true);
+ config.setProperty(PrometheusMetricsProvider.PROMETHEUS_STATS_HTTP_PORT, 0);
+ config.setProperty(PrometheusMetricsProvider.PROMETHEUS_STATS_HTTP_ADDRESS, "127.0.0.1");
+ ByteBuffer byteBuffer = ByteBuffer.allocateDirect(25);
+ PrometheusMetricsProvider provider = new PrometheusMetricsProvider();
+ try {
+ provider.start(config);
+ assertNotNull(provider.server);
+ StringWriter writer = new StringWriter();
+ provider.writeAllMetrics(writer);
+ String s = writer.toString();
+ String[] split = s.split(System.lineSeparator());
+ HashMap<String, String> map = new HashMap<>();
+ for (String str : split) {
+ String[] aux = str.split(" ");
+ map.put(aux[0], aux[1]);
+ }
+ String directBytesMax = map.get("jvm_memory_direct_bytes_max{}");
+ Assert.assertNotNull(directBytesMax);
+ Assert.assertNotEquals("Nan", directBytesMax);
+ Assert.assertNotEquals("-1", directBytesMax);
+ String directBytesUsed = map.get("jvm_memory_direct_bytes_used{}");
+ Assert.assertNotNull(directBytesUsed);
+ Assert.assertNotEquals("Nan", directBytesUsed);
+ Assert.assertTrue(Double.parseDouble(directBytesUsed) > 25);
+ // ensure byteBuffer doesn't gc
+ byteBuffer.clear();
+ } finally {
+ provider.stop();
+ }
+ }
+
}
[bookkeeper] 01/17: [conf] minorCompactionInterval should be greater than gcWaitTime (#2116)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 12c332519124040eeea08be38b700d906a400361
Author: Penghui Li <pe...@apache.org>
AuthorDate: Fri Jul 29 20:11:21 2022 +0800
[conf] minorCompactionInterval should be greater than gcWaitTime (#2116)
(cherry picked from commit 6520a45724abcf8f4fe4566bc58b67f8aea659a0)
---
.../bookkeeper/bookie/GarbageCollectorThread.java | 4 ++--
.../bookkeeper/conf/ServerConfiguration.java | 7 +++++++
.../bookkeeper/conf/TestServerConfiguration.java | 22 +++++++++++++++++++++-
conf/bk_server.conf | 2 ++
4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
index 87a947731a..b98cd2a8f0 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
@@ -224,7 +224,7 @@ public class GarbageCollectorThread extends SafeRunnable {
throw new IOException("Invalid minor compaction threshold "
+ minorCompactionThreshold);
}
- if (minorCompactionInterval <= gcWaitTime) {
+ if (minorCompactionInterval < gcWaitTime) {
throw new IOException("Too short minor compaction interval : "
+ minorCompactionInterval);
}
@@ -245,7 +245,7 @@ public class GarbageCollectorThread extends SafeRunnable {
throw new IOException("Invalid major compaction threshold "
+ majorCompactionThreshold);
}
- if (majorCompactionInterval <= gcWaitTime) {
+ if (majorCompactionInterval < gcWaitTime) {
throw new IOException("Too short major compaction interval : "
+ majorCompactionInterval);
}
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
index 9ce055610d..09a1c76cc5 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
@@ -54,6 +54,7 @@ import org.apache.commons.lang3.StringUtils;
*/
public class ServerConfiguration extends AbstractConfiguration<ServerConfiguration> {
+ private static final int SECOND = 1000;
// Ledger Storage Settings
private static final ConfigKeyGroup GROUP_LEDGER_STORAGE = ConfigKeyGroup.builder("ledgerstorage")
@@ -3090,6 +3091,12 @@ public class ServerConfiguration extends AbstractConfiguration<ServerConfigurati
throw new ConfigurationException("For persisiting explicitLac, journalFormatVersionToWrite should be >= 6"
+ "and FileInfoFormatVersionToWrite should be >= 1");
}
+ if (getMinorCompactionInterval() * SECOND < getGcWaitTime()) {
+ throw new ConfigurationException("minorCompactionInterval should be >= gcWaitTime.");
+ }
+ if (getMajorCompactionInterval() * SECOND < getGcWaitTime()) {
+ throw new ConfigurationException("majorCompactionInterval should be >= gcWaitTime.");
+ }
}
/**
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/TestServerConfiguration.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/TestServerConfiguration.java
index 16901117bb..04ac87818f 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/TestServerConfiguration.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/TestServerConfiguration.java
@@ -155,7 +155,7 @@ public class TestServerConfiguration {
}
@Test
- public void testCompactionSettings() {
+ public void testCompactionSettings() throws ConfigurationException {
ServerConfiguration conf = new ServerConfiguration();
long major, minor;
@@ -199,6 +199,26 @@ public class TestServerConfiguration {
Assert.assertEquals(900, minor);
Assert.assertEquals(21700, major);
+ conf.setMinorCompactionInterval(500);
+ try {
+ conf.validate();
+ fail();
+ } catch (ConfigurationException ignore) {
+ }
+
+ conf.setMinorCompactionInterval(600);
+ conf.validate();
+
+ conf.setMajorCompactionInterval(550);
+ try {
+ conf.validate();
+ fail();
+ } catch (ConfigurationException ignore) {
+ }
+
+ conf.setMajorCompactionInterval(600);
+ conf.validate();
+
// Default Values
double majorThreshold, minorThreshold;
majorThreshold = conf.getMajorCompactionThreshold();
diff --git a/conf/bk_server.conf b/conf/bk_server.conf
index cf76c2a4aa..4aeec71264 100755
--- a/conf/bk_server.conf
+++ b/conf/bk_server.conf
@@ -535,6 +535,7 @@ ledgerDirectories=/tmp/bk-data
# Interval to run minor compaction, in seconds
# If it is set to less than zero, the minor compaction is disabled.
+# Note: should be greater than gcWaitTime.
# minorCompactionInterval=3600
# Maximum milliseconds to run minor Compaction. Defaults to -1 run indefinitely.
@@ -560,6 +561,7 @@ ledgerDirectories=/tmp/bk-data
# Interval to run major compaction, in seconds
# If it is set to less than zero, the major compaction is disabled.
+# Note: should be greater than gcWaitTime.
# majorCompactionInterval=86400
# Maximum milliseconds to run major Compaction. Defaults to -1 run indefinitely.
[bookkeeper] 02/17: Ledger replicate supports throttle (#2778)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 688460b670683adefc0a9e7629f40d1d48c5eb44
Author: gaozhangmin <ga...@qq.com>
AuthorDate: Mon Dec 13 19:55:02 2021 +0800
Ledger replicate supports throttle (#2778)
Ledger replicating puts heavy loads on cluster.
Now, ledger replicate only supports split fragments into small pieces.
But, throttling is not supported.
Add a confiuration `replicationRateByBytes `
support throttling read rate in bytes.
Also bookkeeper shell recover command supports throttle.
(cherry picked from commit a2d73416667a94597d507f9a27b1561d39366c98)
---
.../bookkeeper/conf/ClientConfiguration.java | 24 ++++++++++++++++++++++
.../bookkeeper/conf/ServerConfiguration.java | 23 +++++++++++++++++++++
2 files changed, 47 insertions(+)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java
index 935445174f..57f0323196 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java
@@ -200,6 +200,30 @@ public class ClientConfiguration extends AbstractConfiguration<ClientConfigurati
protected static final String CLIENT_CONNECT_BOOKIE_UNAVAILABLE_LOG_THROTTLING =
"clientConnectBookieUnavailableLogThrottling";
+ protected static final String REPLICATION_RATE_BY_BYTES = "replicationRateByBytes";
+
+ /**
+ * Get the bytes rate of re-replication.
+ * Default value is -1 which it means entries will replicated without any throttling activity.
+ *
+ * @return bytes rate of re-replication.
+ */
+ public int getReplicationRateByBytes() {
+ return getInt(REPLICATION_RATE_BY_BYTES, -1);
+ }
+
+ /**
+ * Set the bytes rate of re-replication.
+ *
+ * @param rate bytes rate of re-replication.
+ *
+ * @return ClientConfiguration
+ */
+ public ClientConfiguration setReplicationRateByBytes(int rate) {
+ this.setProperty(REPLICATION_RATE_BY_BYTES, rate);
+ return this;
+ }
+
/**
* Construct a default client-side configuration.
*/
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
index 09a1c76cc5..6327599808 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
@@ -216,6 +216,7 @@ public class ServerConfiguration extends AbstractConfiguration<ServerConfigurati
"auditorAcquireConcurrentOpenLedgerOperationsTimeOutMSec";
protected static final String IN_FLIGHT_READ_ENTRY_NUM_IN_LEDGER_CHECKER = "inFlightReadEntryNumInLedgerChecker";
+ protected static final String REPLICATION_RATE_BY_BYTES = "replicationRateByBytes";
// Worker Thread parameters.
protected static final String NUM_ADD_WORKER_THREADS = "numAddWorkerThreads";
@@ -3973,4 +3974,26 @@ public class ServerConfiguration extends AbstractConfiguration<ServerConfigurati
this.setProperty(LEDGER_METADATA_ROCKSDB_CONF, ledgerMetadataRocksdbConf);
return this;
}
+
+ /**
+ * Get the bytes rate of re-replication.
+ * Default value is -1 which it means entries will replicated without any throttling activity.
+ *
+ * @return bytes rate of re-replication.
+ */
+ public int getReplicationRateByBytes() {
+ return getInt(REPLICATION_RATE_BY_BYTES, -1);
+ }
+
+ /**
+ * Set the rate of re-replication.
+ *
+ * @param rate bytes rate of re-replication.
+ *
+ * @return ServerConfiguration
+ */
+ public ServerConfiguration setReplicationRateByBytes(int rate) {
+ setProperty(REPLICATION_RATE_BY_BYTES, rate);
+ return this;
+ }
}
[bookkeeper] 11/17: validate diskUsageThreshold and diskUsageLwmThreshold (#3285)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit db3ee4e9d99036f41b07157a68202c39fdd5b129
Author: wenbingshen <ol...@gmail.com>
AuthorDate: Tue Jul 26 17:24:18 2022 +0800
validate diskUsageThreshold and diskUsageLwmThreshold (#3285)
### Motivation
When `diskUsageThreshold < diskUsageLwmThreshold`, the bookie can be started normally. When the disk usage reaches `diskUsageThreshold` , bookie will automatically switch to `ReadOnly` mode. The `LedgerDirsMonitor` then switches the bookie back to `read-write` mode since the disk usage is less than `diskUsageLwmThreshold`, the bookie will switch state back and forth frequently.
### Changes
When creating `LedgerDirsMonitor`, we need to validate `diskUsageThreshold` and `diskUsageLwmThreshold` first.
(cherry picked from commit f181325b9fff32d1b26af4b049a5c343081c97a5)
---
.../bookkeeper/bookie/LedgerDirsMonitor.java | 9 +++++++
.../bookkeeper/bookie/LedgerDirsManagerTest.java | 29 ++++++++++++++++++++++
2 files changed, 38 insertions(+)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsMonitor.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsMonitor.java
index 2b7c90152d..f565119395 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsMonitor.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsMonitor.java
@@ -59,6 +59,7 @@ class LedgerDirsMonitor {
public LedgerDirsMonitor(final ServerConfiguration conf,
final DiskChecker diskChecker,
final List<LedgerDirsManager> dirsManagers) {
+ validateThreshold(conf.getDiskUsageThreshold(), conf.getDiskLowWaterMarkUsageThreshold());
this.interval = conf.getDiskCheckInterval();
this.minUsableSizeForHighPriorityWrites = conf.getMinUsableSizeForHighPriorityWrites();
this.conf = conf;
@@ -229,5 +230,13 @@ class LedgerDirsMonitor {
}
ldm.getWritableLedgerDirs();
}
+
+ private void validateThreshold(float diskSpaceThreshold, float diskSpaceLwmThreshold) {
+ if (diskSpaceThreshold <= 0 || diskSpaceThreshold >= 1 || diskSpaceLwmThreshold - diskSpaceThreshold > 1e-6) {
+ throw new IllegalArgumentException("Disk space threashold: "
+ + diskSpaceThreshold + " and lwm threshold: " + diskSpaceLwmThreshold
+ + " are not valid. Should be > 0 and < 1 and diskSpaceThreshold >= diskSpaceLwmThreshold");
+ }
+ }
}
diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerDirsManagerTest.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerDirsManagerTest.java
index 31b6da37a3..7e0e84f7fd 100644
--- a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerDirsManagerTest.java
+++ b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/LedgerDirsManagerTest.java
@@ -428,6 +428,35 @@ public class LedgerDirsManagerTest {
verifyUsage(curDir1, nospace + 0.05f, curDir2, nospace + 0.05f, mockLedgerDirsListener, true);
}
+ @Test
+ public void testValidateLwmThreshold() {
+ final ServerConfiguration configuration = TestBKConfiguration.newServerConfiguration();
+ // check failed because diskSpaceThreshold < diskSpaceLwmThreshold
+ configuration.setDiskUsageThreshold(0.65f);
+ configuration.setDiskLowWaterMarkUsageThreshold(0.90f);
+ try {
+ new LedgerDirsMonitor(configuration, mockDiskChecker, Collections.singletonList(dirsManager));
+ fail("diskSpaceThreshold < diskSpaceLwmThreshold, should be failed.");
+ } catch (Exception e) {
+ assertTrue(e.getMessage().contains("diskSpaceThreshold >= diskSpaceLwmThreshold"));
+ }
+
+ // check failed because diskSpaceThreshold = 0 and diskUsageLwmThreshold = 1
+ configuration.setDiskUsageThreshold(0f);
+ configuration.setDiskLowWaterMarkUsageThreshold(1f);
+ try {
+ new LedgerDirsMonitor(configuration, mockDiskChecker, Collections.singletonList(dirsManager));
+ fail("diskSpaceThreshold = 0 and diskUsageLwmThreshold = 1, should be failed.");
+ } catch (Exception e) {
+ assertTrue(e.getMessage().contains("Should be > 0 and < 1"));
+ }
+
+ // check succeeded
+ configuration.setDiskUsageThreshold(0.95f);
+ configuration.setDiskLowWaterMarkUsageThreshold(0.90f);
+ new LedgerDirsMonitor(configuration, mockDiskChecker, Collections.singletonList(dirsManager));
+ }
+
private void setUsageAndThenVerify(File dir1, float dir1Usage, File dir2, float dir2Usage,
MockDiskChecker mockDiskChecker, MockLedgerDirsListener mockLedgerDirsListener, boolean verifyReadOnly)
throws InterruptedException {
[bookkeeper] 16/17: avoid init WriteSet when waitForWriteSetMs < 0. (#3325)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 2d40a7c3b608c7fa66909307570f274a7a47e24c
Author: Yan Zhao <ho...@apache.org>
AuthorDate: Tue Jul 26 17:18:16 2022 +0800
avoid init WriteSet when waitForWriteSetMs < 0. (#3325)
### Motivation
Avoid init WriteSet when waitForWriteSetMs < 0.
And LedgerHandleAdv didn't recycle WriteSet.
(cherry picked from commit cb70194be62ba8110a9dba316f53b9760b6a8fc0)
---
.../org/apache/bookkeeper/client/LedgerHandle.java | 28 ++++++++++++----------
.../apache/bookkeeper/client/LedgerHandleAdv.java | 12 +++++++---
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
index bead7d1072..316cc7d32d 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandle.java
@@ -883,13 +883,15 @@ public class LedgerHandle implements WriteHandle {
// Naturally one of the solutions would be to submit smaller batches and in this case
// current implementation will prevent next batch from starting when bookie is
// unresponsive thus helpful enough.
- DistributionSchedule.WriteSet ws = distributionSchedule.getWriteSet(firstEntry);
- try {
- if (!waitForWritable(ws, ws.size() - 1, clientCtx.getConf().waitForWriteSetMs)) {
- op.allowFailFastOnUnwritableChannel();
+ if (clientCtx.getConf().waitForWriteSetMs >= 0) {
+ DistributionSchedule.WriteSet ws = distributionSchedule.getWriteSet(firstEntry);
+ try {
+ if (!waitForWritable(ws, ws.size() - 1, clientCtx.getConf().waitForWriteSetMs)) {
+ op.allowFailFastOnUnwritableChannel();
+ }
+ } finally {
+ ws.recycle();
}
- } finally {
- ws.recycle();
}
if (isHandleWritable()) {
@@ -1348,13 +1350,15 @@ public class LedgerHandle implements WriteHandle {
return;
}
- DistributionSchedule.WriteSet ws = distributionSchedule.getWriteSet(op.getEntryId());
- try {
- if (!waitForWritable(ws, 0, clientCtx.getConf().waitForWriteSetMs)) {
- op.allowFailFastOnUnwritableChannel();
+ if (clientCtx.getConf().waitForWriteSetMs >= 0) {
+ DistributionSchedule.WriteSet ws = distributionSchedule.getWriteSet(op.getEntryId());
+ try {
+ if (!waitForWritable(ws, 0, clientCtx.getConf().waitForWriteSetMs)) {
+ op.allowFailFastOnUnwritableChannel();
+ }
+ } finally {
+ ws.recycle();
}
- } finally {
- ws.recycle();
}
try {
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandleAdv.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandleAdv.java
index d755061727..1320fc0fd2 100644
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandleAdv.java
+++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerHandleAdv.java
@@ -263,9 +263,15 @@ public class LedgerHandleAdv extends LedgerHandle implements WriteAdvHandle {
return;
}
- if (!waitForWritable(distributionSchedule.getWriteSet(op.getEntryId()),
- 0, clientCtx.getConf().waitForWriteSetMs)) {
- op.allowFailFastOnUnwritableChannel();
+ if (clientCtx.getConf().waitForWriteSetMs >= 0) {
+ DistributionSchedule.WriteSet ws = distributionSchedule.getWriteSet(op.getEntryId());
+ try {
+ if (!waitForWritable(ws, 0, clientCtx.getConf().waitForWriteSetMs)) {
+ op.allowFailFastOnUnwritableChannel();
+ }
+ } finally {
+ ws.recycle();
+ }
}
try {
[bookkeeper] 14/17: Fix maven javadoc generate (#3317)
Posted by yo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
yong pushed a commit to branch branch-4.15
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git
commit 40af8ee4a4dbab6a341d27b0595718ef6cf2f416
Author: ZhangJian He <sh...@gmail.com>
AuthorDate: Tue Jun 7 18:54:02 2022 +0800
Fix maven javadoc generate (#3317)
Currently, the maven javadoc generate is broken both on master and branch-4.14
- fix the javadoc generate, the gradle script has already use `delombok`
- delete unneeded `package-info` file
- delete unused dependency `spotbugs-annotations`, it's not worth to open other pr(IMO)
(cherry picked from commit 9f3a815902171467ad6ca863d186449049d78448)
---
.github/workflows/pr-validation.yml | 3 +-
bookkeeper-common/pom.xml | 25 +++++++++++++
bookkeeper-server/pom.xml | 1 +
.../org/apache/bookkeeper/stats/package-info.java | 23 ------------
bookkeeper-stats/pom.xml | 8 +----
.../apache/bookkeeper/stats/AlertStatsLogger.java | 0
pom.xml | 41 ++++++++++++++++++++++
site3/website/scripts/javadoc-gen.sh | 2 +-
stream/distributedlog/pom.xml | 3 +-
9 files changed, 72 insertions(+), 34 deletions(-)
diff --git a/.github/workflows/pr-validation.yml b/.github/workflows/pr-validation.yml
index eb5075e0b2..632dc5c31b 100644
--- a/.github/workflows/pr-validation.yml
+++ b/.github/workflows/pr-validation.yml
@@ -57,6 +57,5 @@ jobs:
- name: Check license files
run: dev/check-all-licenses
- # keeping on gradle, `mvn site javadoc:javadoc`, to figure out later
- name: Generate Javadoc
- run: ./gradlew generateApiJavadoc
+ run: mvn clean -B -nsu -am -pl bookkeeper-common,bookkeeper-server,:bookkeeper-stats-api,:bookkeeper-stats-providers,:codahale-metrics-provider,:prometheus-metrics-provider install javadoc:aggregate -DskipTests -Pdelombok
diff --git a/bookkeeper-common/pom.xml b/bookkeeper-common/pom.xml
index dd928868cc..936cbf2d31 100644
--- a/bookkeeper-common/pom.xml
+++ b/bookkeeper-common/pom.xml
@@ -112,6 +112,31 @@
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <version>${maven-javadoc-plugin.version}</version>
+ <configuration>
+ <sourcepath>${src.dir}</sourcepath>
+ <!-- Avoid for missing javadoc comments to be marked as errors -->
+ <doclint>none</doclint>
+ <subpackages>org.apache.bookkeeper.common.annotation</subpackages>
+ <groups>
+ <group>
+ <title>Bookkeeper Client</title>
+ <packages>org.apache.bookkeeper.common.annotation*</packages>
+ </group>
+ </groups>
+ </configuration>
+ <executions>
+ <execution>
+ <id>attach-javadocs</id>
+ <goals>
+ <goal>jar</goal>
+ </goals>
+ </execution>
+ </executions>
+ </plugin>
</plugins>
</build>
</project>
diff --git a/bookkeeper-server/pom.xml b/bookkeeper-server/pom.xml
index 51d7066b42..454311e195 100644
--- a/bookkeeper-server/pom.xml
+++ b/bookkeeper-server/pom.xml
@@ -254,6 +254,7 @@
<artifactId>maven-javadoc-plugin</artifactId>
<version>${maven-javadoc-plugin.version}</version>
<configuration>
+ <sourcepath>${src.dir}</sourcepath>
<!-- Avoid for missing javadoc comments to be marked as errors -->
<doclint>none</doclint>
<subpackages>org.apache.bookkeeper.client:org.apache.bookkeeper.conf:org.apache.bookkeeper.feature</subpackages>
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/stats/package-info.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/stats/package-info.java
deleted file mode 100644
index df77c581c3..0000000000
--- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/stats/package-info.java
+++ /dev/null
@@ -1,23 +0,0 @@
-/*
- * 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.
- */
-
-/**
- * The bookkeeper stats related classes.
- */
-package org.apache.bookkeeper.stats;
\ No newline at end of file
diff --git a/bookkeeper-stats/pom.xml b/bookkeeper-stats/pom.xml
index 862ed6f90e..390b35a0c4 100644
--- a/bookkeeper-stats/pom.xml
+++ b/bookkeeper-stats/pom.xml
@@ -27,7 +27,6 @@
<name>Apache BookKeeper :: Stats API</name>
<url>http://maven.apache.org</url>
<properties>
- <spotbugs-annotations.version>4.6.0</spotbugs-annotations.version>
</properties>
<build>
<plugins>
@@ -36,6 +35,7 @@
<artifactId>maven-javadoc-plugin</artifactId>
<version>${maven-javadoc-plugin.version}</version>
<configuration>
+ <sourcepath>${src.dir}</sourcepath>
<!-- Avoid for missing javadoc comments to be marked as errors -->
<doclint>none</doclint>
<subpackages>org.apache.bookkeeper.stats</subpackages>
@@ -58,11 +58,5 @@
</plugins>
</build>
<dependencies>
- <dependency>
- <groupId>com.github.spotbugs</groupId>
- <artifactId>spotbugs-annotations</artifactId>
- <version>${spotbugs-annotations.version}</version>
- <scope>provided</scope>
- </dependency>
</dependencies>
</project>
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/stats/AlertStatsLogger.java b/bookkeeper-stats/src/main/java/org/apache/bookkeeper/stats/AlertStatsLogger.java
similarity index 100%
rename from bookkeeper-server/src/main/java/org/apache/bookkeeper/stats/AlertStatsLogger.java
rename to bookkeeper-stats/src/main/java/org/apache/bookkeeper/stats/AlertStatsLogger.java
diff --git a/pom.xml b/pom.xml
index dd14ecf7c0..7b420b76da 100644
--- a/pom.xml
+++ b/pom.xml
@@ -113,6 +113,7 @@
<javac.target>1.8</javac.target>
<redirectTestOutputToFile>true</redirectTestOutputToFile>
<testRetryCount>2</testRetryCount>
+ <src.dir>src/main/java</src.dir>
<test.additional.args />
<!-- dependencies -->
@@ -187,6 +188,7 @@
<exec-maven-plugin.version>1.6.0</exec-maven-plugin.version>
<license-maven-plugin.version>1.6</license-maven-plugin.version>
<jacoco-maven-plugin.version>0.8.0</jacoco-maven-plugin.version>
+ <lombok-maven-plugin.version>1.18.20.0</lombok-maven-plugin.version>
<maven-antrun-plugin.version>1.8</maven-antrun-plugin.version>
<maven-assembly-plugin.version>3.1.0</maven-assembly-plugin.version>
<maven-checkstyle-plugin.version>3.0.0</maven-checkstyle-plugin.version>
@@ -831,6 +833,7 @@
</dependencies>
<build>
+ <sourceDirectory>${src.dir}</sourceDirectory>
<extensions>
<extension>
<groupId>kr.motd.maven</groupId>
@@ -1151,6 +1154,44 @@
</plugins>
</reporting>
</profile>
+ <profile>
+ <id>delombok</id>
+ <properties>
+ <src.dir>${project.build.directory}/generated-sources/delombok</src.dir>
+ </properties>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.projectlombok</groupId>
+ <artifactId>lombok-maven-plugin</artifactId>
+ <version>${lombok-maven-plugin.version}</version>
+ <inherited>true</inherited>
+ <dependencies>
+ <dependency>
+ <groupId>org.projectlombok</groupId>
+ <artifactId>lombok</artifactId>
+ <version>${lombok.version}</version>
+ </dependency>
+ </dependencies>
+ <executions>
+ <execution>
+ <phase>generate-sources</phase>
+ <goals>
+ <goal>delombok</goal>
+ </goals>
+ <configuration>
+ <sourceDirectory>${project.basedir}/src/main/java</sourceDirectory>
+ <outputDirectory>${project.build.directory}/generated-sources/delombok</outputDirectory>
+ <formatPreferences>
+ <pretty/>
+ </formatPreferences>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ </plugins>
+ </build>
+ </profile>
<!-- the profiles below are only for development purpose -->
<profile>
<id>dev</id>
diff --git a/site3/website/scripts/javadoc-gen.sh b/site3/website/scripts/javadoc-gen.sh
index efe088e255..d516acd070 100755
--- a/site3/website/scripts/javadoc-gen.sh
+++ b/site3/website/scripts/javadoc-gen.sh
@@ -34,7 +34,7 @@ function build_javadoc() {
if [[ "$use_gradle" == "true" ]]; then
./gradlew generateApiJavadoc
else
- mvn clean install javadoc:aggregate -DskipTests
+ mvn clean -B -nsu -am -pl bookkeeper-common,bookkeeper-server,:bookkeeper-stats-api,:bookkeeper-stats-providers,:codahale-metrics-provider,:prometheus-metrics-provider install javadoc:aggregate -DskipTests -Pdelombok
fi
mv $javadoc_gen_dir $javadoc_dest_dir
diff --git a/stream/distributedlog/pom.xml b/stream/distributedlog/pom.xml
index 151161743f..99fa4fd501 100644
--- a/stream/distributedlog/pom.xml
+++ b/stream/distributedlog/pom.xml
@@ -47,8 +47,9 @@
<artifactId>maven-javadoc-plugin</artifactId>
<version>${maven-javadoc-plugin.version}</version>
<configuration>
+ <sourcepath>${src.dir}</sourcepath>
+ <notimestamp>true</notimestamp>
<!-- Avoid for missing javadoc comments to be marked as errors -->
- <additionalparam>-notimestamp</additionalparam>
<doclint>none</doclint>
<groups>
<group>