eolivelli commented on a change in pull request #2802:
URL: https://github.com/apache/bookkeeper/pull/2802#discussion_r713912547
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/replication/Auditor.java
##########
@@ -165,6 +165,7 @@
private final AtomicInteger
numLedgersFoundHavingLessThanWQReplicasOfAnEntry;
private final long underreplicatedLedgerRecoveryGracePeriod;
private final int zkOpTimeoutMs;
+ private final Semaphore semaphore;
Review comment:
what about `openLedgerNoRecoverySemaphore` ?
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorPeriodicCheckTest.java
##########
@@ -365,6 +366,36 @@ public void run() {
}
}
+ @Test
+ public void testGetLedgerFromZookeeperThrottled() throws Exception {
+ for (AuditorElector e : auditorElectors.values()) {
+ e.shutdown();
+ }
+
+ final int numberLedgers = 100;
+ // write ledgers into bookkeeper cluster
+ for (int i = 0; i < numberLedgers; ++i) {
+ LedgerHandle lh = bkc.createLedger(3, 3, DigestType.CRC32,
"passwd".getBytes());
+ for (int j = 0; j < 5; j++) {
+ lh.addEntry("testdata".getBytes());
+ }
+ lh.close();
+ }
+
+ // create auditor and call `checkAllLedgers`
+ ServerConfiguration configuration = confByIndex(0);
+ configuration.setAuditorGetLedgerSemaphore(5);
+ try (final Auditor auditor = new Auditor(
+ BookieImpl.getBookieId(configuration).toString(),
+ configuration, NullStatsLogger.INSTANCE)) {
+ auditor.checkAllLedgers();
Review comment:
is there any way to verify that the operation completed without errors
or at least that we called asyncOpenLedgerNoRecovery the expected number of
times ?
(with PowerMock should be easy)
##########
File path:
bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
##########
@@ -204,6 +204,7 @@
protected static final String UNDERREPLICATED_LEDGER_RECOVERY_GRACE_PERIOD
=
"underreplicatedLedgerRecoveryGracePeriod";
protected static final String AUDITOR_REPLICAS_CHECK_INTERVAL =
"auditorReplicasCheckInterval";
+ protected static final String AUDITOR_GET_LEDGER_SEMAPHORE =
"auditorGetLedgerSemaphore";
Review comment:
what about `auditorMaxNumberOfConcurrentOpenLedgerOperations` ?
##########
File path:
bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/AuditorPeriodicCheckTest.java
##########
@@ -365,6 +366,36 @@ public void run() {
}
}
+ @Test
+ public void testGetLedgerFromZookeeperThrottled() throws Exception {
+ for (AuditorElector e : auditorElectors.values()) {
+ e.shutdown();
+ }
+
+ final int numberLedgers = 100;
+ // write ledgers into bookkeeper cluster
+ for (int i = 0; i < numberLedgers; ++i) {
+ LedgerHandle lh = bkc.createLedger(3, 3, DigestType.CRC32,
"passwd".getBytes());
Review comment:
nit: you can use try-with-resources here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
