Prevent bookie shutdown due to rest api when bookie prohibits readOnl…

…yMode (apache#3972)
sijie · Jul 12, 2024 · 3d1e888 · 3d1e888
1 parent cc32363
commit 3d1e888
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 4 deletions.
diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieStateManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/BookieStateManager.java
@@ -235,6 +235,11 @@ public boolean isForceReadOnly(){
         return forceReadOnly.get();
     }
 
+    @Override
+    public boolean isReadOnlyModeEnabled() {
+        return conf.isReadOnlyModeEnabled();
+    }
+
     @Override
     public boolean isAvailableForHighPriorityWrites() {
         return availableForHighPriorityWrites;

diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/StateManager.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/StateManager.java
@@ -53,6 +53,11 @@ public interface StateManager extends AutoCloseable {
      */
     boolean isForceReadOnly();
 
+    /**
+     * Check is readOnlyModeEnabled.
+     */
+    boolean isReadOnlyModeEnabled();
+
     /**
      * Check is Running.
      */

diff --git a/...r/src/main/java/org/apache/bookkeeper/server/http/service/BookieStateReadOnlyService.java b/...r/src/main/java/org/apache/bookkeeper/server/http/service/BookieStateReadOnlyService.java
@@ -60,6 +60,11 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
                 }
                 stateManager.transitionToWritableMode().get();
             } else if (!stateManager.isReadOnly() && inState.isReadOnly()) {
+                if (!stateManager.isReadOnlyModeEnabled()) {
+                    response.setCode(HttpServer.StatusCode.BAD_REQUEST);
+                    response.setBody("Bookie is disabled ReadOnly mode, cannot transit to readOnly mode");
+                    return response;
+                }
                 stateManager.transitionToReadOnlyMode().get();
             }
         } else if (!HttpServer.Method.GET.equals(request.getMethod())) {

diff --git a/bookkeeper-server/src/test/java/org/apache/bookkeeper/server/http/TestHttpService.java b/bookkeeper-server/src/test/java/org/apache/bookkeeper/server/http/TestHttpService.java
@@ -1023,11 +1023,43 @@ public void testBookieReadOnlyState() throws Exception {
         assertFalse(readOnlyState.isReadOnly());
 
         //forceReadonly to writable
-        baseConf.setForceReadOnlyBookie(true);
-        baseConf.setReadOnlyModeEnabled(true);
-        restartBookies();
+        MetadataBookieDriver metadataDriver = BookieResources.createMetadataDriver(
+                baseConf, NullStatsLogger.INSTANCE);
+        restartBookies(c -> {
+            c.setForceReadOnlyBookie(true);
+            c.setReadOnlyModeEnabled(true);
+            return c;
+        });
+        // the old bkHttpServiceProvider has an old bookie instance who has been shutdown
+        // so we need create a new bkHttpServiceProvider2 to contains a new bookie which has created by restart.
+        BKHttpServiceProvider bkHttpServiceProvider2 = new BKHttpServiceProvider.Builder()
+                .setBookieServer(serverByIndex(numberOfBookies - 1))
+                .setServerConfiguration(baseConf)
+                .setLedgerManagerFactory(metadataDriver.getLedgerManagerFactory())
+                .build();
+        HttpEndpointService bookieReadOnlyService2 = bkHttpServiceProvider2
+                .provideHttpEndpointService(HttpServer.ApiType.BOOKIE_STATE_READONLY);
+
         request = new HttpServiceRequest(JsonUtil.toJson(new ReadOnlyState(false)), HttpServer.Method.PUT,  null);
-        response = bookieReadOnlyService.handle(request);
+        response = bookieReadOnlyService2.handle(request);
+        assertEquals(400, response.getStatusCode());
+
+        // disable readOnly mode
+        restartBookies(c -> {
+            c.setForceReadOnlyBookie(false);
+            c.setReadOnlyModeEnabled(false);
+            return c;
+        });
+        bkHttpServiceProvider2 = new BKHttpServiceProvider.Builder()
+                .setBookieServer(serverByIndex(numberOfBookies - 1))
+                .setServerConfiguration(baseConf)
+                .setLedgerManagerFactory(metadataDriver.getLedgerManagerFactory())
+                .build();
+        bookieReadOnlyService2 = bkHttpServiceProvider2
+                .provideHttpEndpointService(HttpServer.ApiType.BOOKIE_STATE_READONLY);
+
+        request = new HttpServiceRequest(JsonUtil.toJson(new ReadOnlyState(true)), HttpServer.Method.PUT,  null);
+        response = bookieReadOnlyService2.handle(request);
         assertEquals(400, response.getStatusCode());
     }