Skip to content

Commit bde9ff2

Browse files
poorbarcodepoorbarcode
authored
[fix]BK stays at read_only state even if the disk is empty (#4640)
* [fix]BK stay at read_only state even if the disk is empty * correct the javadoc * improve test
1 parent 0621ae6 commit bde9ff2

File tree

8 files changed

+202
-30
lines changed

8 files changed

+202
-30
lines changed

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieStateManager.java‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ private boolean isRegistrationManagerDisabled() {
202202
@Override
203203
public void initState(){
204204
if (forceReadOnly.get()) {
205-
this.bookieStatus.setToReadOnlyMode();
205+
this.bookieStatus.setToReadOnlyMode(false);
206206
} else if (conf.isPersistBookieStatusEnabled()) {
207207
this.bookieStatus.readFromDirectories(statusDirs);
208208
}
@@ -293,22 +293,22 @@ public Void call() throws IOException {
293293
}
294294

295295
@Override
296-
public Future<Void> transitionToWritableMode() {
296+
public Future<Void> transitionToWritableMode(boolean isManuallyModify) {
297297
return stateService.submit(new Callable<Void>() {
298298
@Override
299299
public Void call() throws Exception{
300-
doTransitionToWritableMode();
300+
doTransitionToWritableMode(isManuallyModify);
301301
return null;
302302
}
303303
});
304304
}
305305

306306
@Override
307-
public Future<Void> transitionToReadOnlyMode() {
307+
public Future<Void> transitionToReadOnlyMode(boolean isManuallyModify) {
308308
return stateService.submit(new Callable<Void>() {
309309
@Override
310310
public Void call() throws Exception{
311-
doTransitionToReadOnlyMode();
311+
doTransitionToReadOnlyMode(isManuallyModify);
312312
return null;
313313
}
314314
});
@@ -335,10 +335,15 @@ private void doRegisterBookie(boolean isReadOnly) throws IOException {
335335
}
336336

337337
@VisibleForTesting
338-
public void doTransitionToWritableMode() {
338+
public void doTransitionToWritableMode(boolean isManuallyModify) {
339339
if (shuttingdown || forceReadOnly.get()) {
340340
return;
341341
}
342+
if (!isManuallyModify && bookieStatus.isInReadOnlyMode() && bookieStatus.isManuallyModifiedToReadOnly()) {
343+
LOG.info("Skip to transition Bookie to Writable mode automatically because it is manually set to read-only"
344+
+ " mode, which can only be changed manually.");
345+
return;
346+
}
342347

343348
if (!bookieStatus.setToWritableMode()) {
344349
// do nothing if already in writable mode
@@ -371,11 +376,11 @@ public void doTransitionToWritableMode() {
371376
}
372377

373378
@VisibleForTesting
374-
public void doTransitionToReadOnlyMode() {
379+
public void doTransitionToReadOnlyMode(boolean isManuallyModify) {
375380
if (shuttingdown) {
376381
return;
377382
}
378-
if (!bookieStatus.setToReadOnlyMode()) {
383+
if (!bookieStatus.setToReadOnlyMode(isManuallyModify)) {
379384
return;
380385
}
381386
if (!conf.isReadOnlyModeEnabled()) {

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieStatus.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.io.InputStreamReader;
3232
import java.io.OutputStreamWriter;
3333
import java.util.List;
34+
import lombok.Getter;
3435
import org.slf4j.Logger;
3536
import org.slf4j.LoggerFactory;
3637

@@ -53,6 +54,8 @@ enum BookieMode {
5354
private int layoutVersion;
5455
private long lastUpdateTime;
5556
private volatile BookieMode bookieMode;
57+
@Getter
58+
private boolean isManuallyModifiedToReadOnly;
5659

5760
BookieStatus() {
5861
this.bookieMode = BookieMode.READ_WRITE;
@@ -72,6 +75,7 @@ synchronized boolean setToWritableMode() {
7275
if (!bookieMode.equals(BookieMode.READ_WRITE)) {
7376
bookieMode = BookieMode.READ_WRITE;
7477
this.lastUpdateTime = System.currentTimeMillis();
78+
this.isManuallyModifiedToReadOnly = false;
7579
return true;
7680
}
7781
return false;
@@ -81,10 +85,11 @@ boolean isInReadOnlyMode() {
8185
return bookieMode.equals(BookieMode.READ_ONLY);
8286
}
8387

84-
synchronized boolean setToReadOnlyMode() {
88+
synchronized boolean setToReadOnlyMode(boolean isManuallyModify) {
8589
if (!bookieMode.equals(BookieMode.READ_ONLY)) {
8690
bookieMode = BookieMode.READ_ONLY;
8791
this.lastUpdateTime = System.currentTimeMillis();
92+
this.isManuallyModifiedToReadOnly = isManuallyModify;
8893
return true;
8994
}
9095
return false;
@@ -147,6 +152,7 @@ void readFromDirectories(List<File> directories) {
147152
this.lastUpdateTime = status.lastUpdateTime;
148153
this.layoutVersion = status.layoutVersion;
149154
this.bookieMode = status.bookieMode;
155+
this.isManuallyModifiedToReadOnly = status.isManuallyModifiedToReadOnly;
150156
success = true;
151157
}
152158
}
@@ -216,6 +222,15 @@ public BookieStatus parse(BufferedReader reader)
216222
if (status.layoutVersion == 1 && parts.length == 3) {
217223
status.bookieMode = BookieMode.valueOf(parts[1]);
218224
status.lastUpdateTime = Long.parseLong(parts[2].trim());
225+
status.isManuallyModifiedToReadOnly = true;
226+
return status;
227+
}
228+
// Since we should guarantee the compatibility of downgrade. We do not change the layoutVersion, otherwise,
229+
// the string can not be parsed by the lower version.
230+
if (status.layoutVersion == 1 && parts.length == 4) {
231+
status.bookieMode = BookieMode.valueOf(parts[1]);
232+
status.lastUpdateTime = Long.parseLong(parts[2].trim());
233+
status.isManuallyModifiedToReadOnly = Boolean.parseBoolean(parts[3].trim());
219234
return status;
220235
}
221236
}
@@ -231,6 +246,8 @@ public String toString() {
231246
builder.append(getBookieMode());
232247
builder.append(",");
233248
builder.append(System.currentTimeMillis());
249+
builder.append(",");
250+
builder.append(isManuallyModifiedToReadOnly);
234251
builder.append("\n");
235252
return builder.toString();
236253
}

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/LedgerDirsMonitor.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
package org.apache.bookkeeper.bookie;
2323

24+
import com.google.common.annotations.VisibleForTesting;
2425
import com.google.common.util.concurrent.ThreadFactoryBuilder;
2526
import java.io.File;
2627
import java.io.IOException;
@@ -54,6 +55,7 @@ class LedgerDirsMonitor {
5455
private final long minUsableSizeForHighPriorityWrites;
5556
private ScheduledExecutorService executor;
5657
private ScheduledFuture<?> checkTask;
58+
private boolean isFirstLoopOfCheckTask = true;
5759

5860
public LedgerDirsMonitor(final ServerConfiguration conf,
5961
final DiskChecker diskChecker,
@@ -67,6 +69,10 @@ public LedgerDirsMonitor(final ServerConfiguration conf,
6769
}
6870

6971
private void check(final LedgerDirsManager ldm) {
72+
final boolean isFirstLoopOfCheckTaskLocalValue = this.isFirstLoopOfCheckTask;
73+
if (isFirstLoopOfCheckTaskLocalValue) {
74+
this.isFirstLoopOfCheckTask = false;
75+
}
7076
final ConcurrentMap<File, Float> diskUsages = ldm.getDiskUsages();
7177
boolean someDiskFulled = false;
7278
boolean highPriorityWritesAllowed = true;
@@ -175,6 +181,14 @@ private void check(final LedgerDirsManager ldm) {
175181
}
176182
}
177183

184+
if (isFirstLoopOfCheckTaskLocalValue && ldm.getFullFilledLedgerDirs().isEmpty()) {
185+
// notify no disk full.
186+
for (LedgerDirsListener listener : ldm.getListeners()) {
187+
listener.allDisksWritable();
188+
}
189+
return;
190+
}
191+
178192
if (conf.isReadOnlyModeOnAnyDiskFullEnabled()) {
179193
if (someDiskFulled && !ldm.getFullFilledLedgerDirs().isEmpty()) {
180194
// notify any disk full.
@@ -192,7 +206,8 @@ private void check(final LedgerDirsManager ldm) {
192206
}
193207
}
194208

195-
private void check() {
209+
@VisibleForTesting
210+
void check() {
196211
dirsManagers.forEach(this::check);
197212
}
198213

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/StateManager.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,12 +100,20 @@ public interface StateManager extends AutoCloseable {
100100
/**
101101
* Change the state of bookie to Writable mode.
102102
*/
103-
Future<Void> transitionToWritableMode();
103+
Future<Void> transitionToWritableMode(boolean isManuallyModify);
104+
105+
default Future<Void> transitionToWritableMode() {
106+
return transitionToWritableMode(false);
107+
}
104108

105109
/**
106110
* Change the state of bookie to ReadOnly mode.
107111
*/
108-
Future<Void> transitionToReadOnlyMode();
112+
Future<Void> transitionToReadOnlyMode(boolean isManuallyModify);
113+
114+
default Future<Void> transitionToReadOnlyMode() {
115+
return transitionToReadOnlyMode(false);
116+
}
109117

110118
/**
111119
* ShutdownHandler used to shutdown bookie.

‎bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieStateReadOnlyService.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,14 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
5858
response.setBody("Bookie is in forceReadOnly mode, cannot transit to writable mode");
5959
return response;
6060
}
61-
stateManager.transitionToWritableMode().get();
61+
stateManager.transitionToWritableMode(true).get();
6262
} else if (!stateManager.isReadOnly() && inState.isReadOnly()) {
6363
if (!stateManager.isReadOnlyModeEnabled()) {
6464
response.setCode(HttpServer.StatusCode.BAD_REQUEST);
6565
response.setBody("Bookie is disabled ReadOnly mode, cannot transit to readOnly mode");
6666
return response;
6767
}
68-
stateManager.transitionToReadOnlyMode().get();
68+
stateManager.transitionToReadOnlyMode(true).get();
6969
}
7070
} else if (!HttpServer.Method.GET.equals(request.getMethod())) {
7171
response.setCode(HttpServer.StatusCode.NOT_FOUND);

0 commit comments

Comments
 (0)