Skip to content

Commit dc2bb1d

Browse files
authored
Enable reorder read sequence for bk client by default (#4139)
### Motivation <!-- Explain here the context, and why you're making that change. What is the problem you're trying to solve. --> If one ledger's ensemble is [bk0, bk1] and bk0 is down, the bookie client may send a read request to bk0 first then fail with the following errors, and resend the read request to bk1 in the end. ``` 2023-10-19T18:33:52,042 - ERROR - [BookKeeperClientWorker-OrderedExecutor-3-0:PerChannelBookieClient@563] - Cannot connect to 192.168.31.216:3181 as endpoint resolution failed (probably bookie is down) err org.apache.bookkeeper.proto.BookieAddressResolver$BookieIdNotResolvedException: Cannot resolve bookieId 192.168.31.216:3181, bookie does not exist or it is not running 2023-10-19T18:33:52,042 - INFO - [BookKeeperClientWorker-OrderedExecutor-3-0:DefaultBookieAddressResolver@77] - Cannot resolve 192.168.31.216:3181, bookie is unknown org.apache.bookkeeper.client.BKException$BKBookieHandleNotAvailableException: Bookie handle is not available 2023-10-19T18:33:52,042 - INFO - [BookKeeperClientWorker-OrderedExecutor-3-0:PendingReadOp$LedgerEntryRequest@223] - Error: Bookie handle is not available while reading L6 E40 from bookie: 192.168.31.216:3181 ``` One of the related issues is in the auto-recovery decommission and there is one PR in the BookKeeper repo: #4113 However, the bookie client already knows the bk0 is down and we should send the read request to bk1 first. So we can reorder the read request based on the known bookie list. If one bookie is lost, it will reorder the lost bookie to the end of the read list. ### Modifications <!-- Describe the modifications you've done. --> Enable the `reorderReadSequence` by default for auto-recovery.
1 parent 1eceb5d commit dc2bb1d

File tree

4 files changed

+11
-5
lines changed

4 files changed

+11
-5
lines changed

bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1148,7 +1148,7 @@ public ClientConfiguration setRecoveryReadBatchSize(int batchSize) {
11481148
* @return true if reorder read sequence is enabled, otherwise false.
11491149
*/
11501150
public boolean isReorderReadSequenceEnabled() {
1151-
return getBoolean(REORDER_READ_SEQUENCE_ENABLED, false);
1151+
return getBoolean(REORDER_READ_SEQUENCE_ENABLED, true);
11521152
}
11531153

11541154
/**

bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public abstract class MockBookKeeperTestCase {
9696
protected BookieClient bookieClient;
9797
protected LedgerManager ledgerManager;
9898
protected LedgerIdGenerator ledgerIdGenerator;
99+
protected EnsemblePlacementPolicy placementPolicy;
99100

100101
private BookieWatcher bookieWatcher;
101102

@@ -152,6 +153,7 @@ public void setup() throws Exception {
152153
scheduler = OrderedScheduler.newSchedulerBuilder().numThreads(4).name("bk-test").build();
153154
executor = OrderedExecutor.newBuilder().build();
154155
bookieWatcher = mock(BookieWatcher.class);
156+
placementPolicy = new DefaultEnsemblePlacementPolicy();
155157

156158
bookieClient = mock(BookieClient.class);
157159
ledgerManager = mock(LedgerManager.class);
@@ -194,7 +196,7 @@ public BookieWatcher getBookieWatcher() {
194196

195197
@Override
196198
public EnsemblePlacementPolicy getPlacementPolicy() {
197-
return null;
199+
return placementPolicy;
198200
}
199201

200202
@Override

bookkeeper-server/src/test/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOpTest.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public class ReadLastConfirmedAndEntryOpTest {
8585
private ScheduledExecutorService scheduler;
8686
private OrderedScheduler orderedScheduler;
8787
private ClientInternalConf internalConf;
88-
private EnsemblePlacementPolicy mockPlacementPolicy;
88+
private EnsemblePlacementPolicy placementPolicy;
8989
private LedgerMetadata ledgerMetadata;
9090
private DistributionSchedule distributionSchedule;
9191
private DigestManager digestManager;
@@ -121,10 +121,11 @@ public void setup() throws Exception {
121121
.build();
122122

123123
this.mockBookieClient = mock(BookieClient.class);
124-
this.mockPlacementPolicy = mock(EnsemblePlacementPolicy.class);
124+
//this.mockPlacementPolicy = mock(EnsemblePlacementPolicy.class);
125+
this.placementPolicy = new DefaultEnsemblePlacementPolicy();
125126
this.mockClientCtx = mock(ClientContext.class);
126127
when(mockClientCtx.getBookieClient()).thenReturn(mockBookieClient);
127-
when(mockClientCtx.getPlacementPolicy()).thenReturn(mockPlacementPolicy);
128+
when(mockClientCtx.getPlacementPolicy()).thenReturn(placementPolicy);
128129
when(mockClientCtx.getConf()).thenReturn(internalConf);
129130
when(mockClientCtx.getScheduler()).thenReturn(orderedScheduler);
130131
when(mockClientCtx.getMainWorkerPool()).thenReturn(orderedScheduler);

conf/bk_server.conf

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,9 @@ statsProviderClass=org.apache.bookkeeper.stats.prometheus.PrometheusMetricsProvi
10571057
# Enable/disable having read operations for a ledger to be sticky to a single bookie.
10581058
stickyReadSEnabled=true
10591059

1060+
# Enable/disable reordering read sequence on reading entries.
1061+
reorderReadSequenceEnabled=true
1062+
10601063
# The grace period, in milliseconds, that the replication worker waits before fencing and
10611064
# replicating a ledger fragment that's still being written to upon bookie failure.
10621065
# openLedgerRereplicationGracePeriod=30000

0 commit comments

Comments
 (0)