Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable reorder read sequence for bk client by default #4139

Merged
merged 4 commits into from
Feb 1, 2024
Merged

Enable reorder read sequence for bk client by default #4139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a151f8a
enable reorder read sequence for bk client by default
hangc0276 Dec 1, 2023
0668143
address comments
hangc0276 Dec 4, 2023
01669df
Fix failed test
hangc0276 Jan 29, 2024
40c8923
Fix failed test
hangc0276 Jan 31, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ClientConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1148,7 +1148,7 @@ public ClientConfiguration setRecoveryReadBatchSize(int batchSize) {
* @return true if reorder read sequence is enabled, otherwise false.
*/
public boolean isReorderReadSequenceEnabled() {
return getBoolean(REORDER_READ_SEQUENCE_ENABLED, false);
return getBoolean(REORDER_READ_SEQUENCE_ENABLED, true);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion bookkeeper-server/src/test/java/org/apache/bookkeeper/client/MockBookKeeperTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public abstract class MockBookKeeperTestCase {
protected BookieClient bookieClient;
protected LedgerManager ledgerManager;
protected LedgerIdGenerator ledgerIdGenerator;
protected EnsemblePlacementPolicy placementPolicy;

private BookieWatcher bookieWatcher;

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

bookieClient = mock(BookieClient.class);
ledgerManager = mock(LedgerManager.class);
Expand Down Expand Up @@ -194,7 +196,7 @@ public BookieWatcher getBookieWatcher() {

@Override
public EnsemblePlacementPolicy getPlacementPolicy() {
return null;
return placementPolicy;
}

@Override
Expand Down
7 changes: 4 additions & 3 deletions ...er-server/src/test/java/org/apache/bookkeeper/client/ReadLastConfirmedAndEntryOpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public class ReadLastConfirmedAndEntryOpTest {
private ScheduledExecutorService scheduler;
private OrderedScheduler orderedScheduler;
private ClientInternalConf internalConf;
private EnsemblePlacementPolicy mockPlacementPolicy;
private EnsemblePlacementPolicy placementPolicy;
private LedgerMetadata ledgerMetadata;
private DistributionSchedule distributionSchedule;
private DigestManager digestManager;
Expand Down Expand Up @@ -121,10 +121,11 @@ public void setup() throws Exception {
.build();

this.mockBookieClient = mock(BookieClient.class);
this.mockPlacementPolicy = mock(EnsemblePlacementPolicy.class);
//this.mockPlacementPolicy = mock(EnsemblePlacementPolicy.class);
this.placementPolicy = new DefaultEnsemblePlacementPolicy();
this.mockClientCtx = mock(ClientContext.class);
when(mockClientCtx.getBookieClient()).thenReturn(mockBookieClient);
when(mockClientCtx.getPlacementPolicy()).thenReturn(mockPlacementPolicy);
when(mockClientCtx.getPlacementPolicy()).thenReturn(placementPolicy);
when(mockClientCtx.getConf()).thenReturn(internalConf);
when(mockClientCtx.getScheduler()).thenReturn(orderedScheduler);
when(mockClientCtx.getMainWorkerPool()).thenReturn(orderedScheduler);
Expand Down
3 changes: 3 additions & 0 deletions conf/bk_server.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1057,6 +1057,9 @@ zkEnableSecurity=false
# Enable/disable having read operations for a ledger to be sticky to a single bookie.
stickyReadSEnabled=true

# Enable/disable reordering read sequence on reading entries.
reorderReadSequenceEnabled=true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change the default value in the clientconfiguration.java? I think it should be applied to all the clients, right? It will avoid reading a lost bookie as much as possible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. We can change the default value in the next major release.


# The grace period, in milliseconds, that the replication worker waits before fencing and
# replicating a ledger fragment that's still being written to upon bookie failure.
# openLedgerRereplicationGracePeriod=30000
Expand Down
Loading