From 5ed6effb5c58d8ff9deb65e5c44dad34971108c9 Mon Sep 17 00:00:00 2001 From: rajan Date: Mon, 7 Nov 2022 18:52:11 -0800 Subject: [PATCH 1/3] Add Http-service to check bookie sanity state --- .../apache/bookkeeper/http/HttpRouter.java | 2 + .../apache/bookkeeper/http/HttpServer.java | 1 + .../server/http/BKHttpServiceProvider.java | 3 + .../http/service/BookieSanityService.java | 90 +++++++++++++++++++ .../server/http/TestHttpService.java | 20 +++++ site3/website/docs/admin/http.md | 19 ++++ 6 files changed, 135 insertions(+) create mode 100644 bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java diff --git a/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpRouter.java b/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpRouter.java index ed460b73c9b..0fc4fec308d 100644 --- a/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpRouter.java +++ b/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpRouter.java @@ -50,6 +50,7 @@ public abstract class HttpRouter { public static final String SUSPEND_GC_COMPACTION = "/api/v1/bookie/gc/suspend_compaction"; public static final String RESUME_GC_COMPACTION = "/api/v1/bookie/gc/resume_compaction"; public static final String BOOKIE_STATE = "/api/v1/bookie/state"; + public static final String BOOKIE_SANITY = "/api/v1/bookie/sanity"; public static final String BOOKIE_STATE_READONLY = "/api/v1/bookie/state/readonly"; public static final String BOOKIE_IS_READY = "/api/v1/bookie/is_ready"; public static final String BOOKIE_INFO = "/api/v1/bookie/info"; @@ -85,6 +86,7 @@ public HttpRouter(AbstractHttpHandlerFactory handlerFactory) { this.endpointHandlers.put(GC, handlerFactory.newHandler(HttpServer.ApiType.GC)); this.endpointHandlers.put(GC_DETAILS, handlerFactory.newHandler(HttpServer.ApiType.GC_DETAILS)); this.endpointHandlers.put(BOOKIE_STATE, handlerFactory.newHandler(HttpServer.ApiType.BOOKIE_STATE)); + this.endpointHandlers.put(BOOKIE_SANITY, handlerFactory.newHandler(HttpServer.ApiType.BOOKIE_SANITY)); this.endpointHandlers.put(BOOKIE_STATE_READONLY, handlerFactory.newHandler(HttpServer.ApiType.BOOKIE_STATE_READONLY)); this.endpointHandlers.put(BOOKIE_IS_READY, handlerFactory.newHandler(HttpServer.ApiType.BOOKIE_IS_READY)); diff --git a/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpServer.java b/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpServer.java index 7bfd62e975c..f413ae2b65d 100644 --- a/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpServer.java +++ b/bookkeeper-http/http-server/src/main/java/org/apache/bookkeeper/http/HttpServer.java @@ -82,6 +82,7 @@ enum ApiType { GC, GC_DETAILS, BOOKIE_STATE, + BOOKIE_SANITY, BOOKIE_STATE_READONLY, BOOKIE_IS_READY, BOOKIE_INFO, diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java index 8ebf29e831c..eba32d44ecc 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/BKHttpServiceProvider.java @@ -40,6 +40,7 @@ import org.apache.bookkeeper.server.http.service.AutoRecoveryStatusService; import org.apache.bookkeeper.server.http.service.BookieInfoService; import org.apache.bookkeeper.server.http.service.BookieIsReadyService; +import org.apache.bookkeeper.server.http.service.BookieSanityService; import org.apache.bookkeeper.server.http.service.BookieStateReadOnlyService; import org.apache.bookkeeper.server.http.service.BookieStateService; import org.apache.bookkeeper.server.http.service.ConfigurationService; @@ -219,6 +220,8 @@ public HttpEndpointService provideHttpEndpointService(ApiType type) { return new GCDetailsService(configuration, bookieServer); case BOOKIE_STATE: return new BookieStateService(bookieServer.getBookie()); + case BOOKIE_SANITY: + return new BookieSanityService(configuration); case BOOKIE_STATE_READONLY: return new BookieStateReadOnlyService(bookieServer.getBookie()); case BOOKIE_IS_READY: diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java new file mode 100644 index 00000000000..7b2b53d34c5 --- /dev/null +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java @@ -0,0 +1,90 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.bookkeeper.server.http.service; + +import static com.google.common.base.Preconditions.checkNotNull; + +import org.apache.bookkeeper.bookie.StateManager; +import org.apache.bookkeeper.common.util.JsonUtil; +import org.apache.bookkeeper.conf.ServerConfiguration; +import org.apache.bookkeeper.http.HttpServer; +import org.apache.bookkeeper.http.service.HttpEndpointService; +import org.apache.bookkeeper.http.service.HttpServiceRequest; +import org.apache.bookkeeper.http.service.HttpServiceResponse; +import org.apache.bookkeeper.tools.cli.commands.bookie.SanityTestCommand; + +import lombok.Data; +import lombok.NoArgsConstructor; + +/** + * HttpEndpointService that exposes the bookie sanity state. + * + *

Get the current bookie sanity response: + * + * 

+ * 
+ * {
+ *  "passed" : true,
+ *  "readOnly" : false
+ *}
+ * 
+ *
+ */ +public class BookieSanityService implements HttpEndpointService { + + private final ServerConfiguration config; + + public BookieSanityService(ServerConfiguration config) { + this.config = checkNotNull(config); + } + + /** + * POJO definition for the bookie sanity response. + */ + @Data + @NoArgsConstructor + public static class BookieSanity { + private boolean passed; + private boolean readOnly; + } + + @Override + public HttpServiceResponse handle(HttpServiceRequest request) throws Exception { + HttpServiceResponse response = new HttpServiceResponse(); + + if (HttpServer.Method.GET != request.getMethod()) { + response.setCode(HttpServer.StatusCode.NOT_FOUND); + response.setBody("Only support GET method to retrieve bookie sanity state."); + return response; + } + + BookieSanity bs = new BookieSanity(); + if (config.isForceReadOnlyBookie()) { + bs.readOnly = true; + } else { + SanityTestCommand sanity = new SanityTestCommand(); + bs.passed = sanity.apply(config, new SanityTestCommand.SanityFlags()); + } + + String jsonResponse = JsonUtil.toJson(bs); + response.setBody(jsonResponse); + response.setCode(HttpServer.StatusCode.OK); + return response; + } +} 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 index cdb9c4fdd4f..96e7ce1d86a 100644 --- 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 @@ -52,6 +52,8 @@ import org.apache.bookkeeper.net.BookieSocketAddress; import org.apache.bookkeeper.replication.AuditorElector; import org.apache.bookkeeper.server.http.service.BookieInfoService; +import org.apache.bookkeeper.server.http.service.BookieSanityService; +import org.apache.bookkeeper.server.http.service.BookieSanityService.BookieSanity; import org.apache.bookkeeper.server.http.service.BookieStateReadOnlyService.ReadOnlyState; import org.apache.bookkeeper.server.http.service.BookieStateService.BookieState; import org.apache.bookkeeper.stats.NullStatsLogger; @@ -853,6 +855,24 @@ public void testGetBookieState() throws Exception { assertEquals(false, bs.isShuttingDown()); } + @Test + public void testGetBookieSanity() throws Exception { + HttpEndpointService bookieStateServer = bkHttpServiceProvider + .provideHttpEndpointService(HttpServer.ApiType.BOOKIE_SANITY); + + HttpServiceRequest request1 = new HttpServiceRequest(null, HttpServer.Method.GET, null); + ServerConfiguration conf = servers.get(0).getConfiguration(); + BookieSanityService service = new BookieSanityService(conf); + HttpServiceResponse response1 = service.handle(request1); + assertEquals(HttpServer.StatusCode.OK.getValue(), response1.getStatusCode()); + BookieSanity bs = JsonUtil.fromJson(response1.getBody(), BookieSanity.class); + assertEquals(true, bs.isPassed()); + assertEquals(false, bs.isReadOnly()); + + HttpServiceResponse response2 = bookieStateServer.handle(request1); + assertEquals(HttpServer.StatusCode.OK.getValue(), response2.getStatusCode()); + } + @Test public void testGetBookieIsReady() throws Exception { HttpEndpointService bookieStateServer = bkHttpServiceProvider diff --git a/site3/website/docs/admin/http.md b/site3/website/docs/admin/http.md index 6c71c097407..14358f8f372 100644 --- a/site3/website/docs/admin/http.md +++ b/site3/website/docs/admin/http.md @@ -413,6 +413,25 @@ Currently all the HTTP endpoints could be divided into these 5 components: } ``` +### Endpoint: /api/v1/bookie/sanity +1. Method: GET + * Description: Exposes the bookie sanity state + * Response: + + | Code | Description | + |:-------|:------------| + |200 | Successful operation | + |403 | Permission denied | + |404 | Not found | + * Body: + ```json + { + "passed" : true, + "readOnly" : false + } + ``` + + ### Endpoint: /api/v1/bookie/is_ready 1. Method: GET * Description: Return true if the bookie is ready From 269d998485b39c42458f0aabb68fb1fc0e888cfc Mon Sep 17 00:00:00 2001 From: rajan Date: Thu, 10 Nov 2022 10:45:12 -0800 Subject: [PATCH 2/3] restrict concurrent requests --- .../server/http/service/BookieSanityService.java | 14 ++++++++++++-- .../bookkeeper/server/http/TestHttpService.java | 11 +++++++---- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java index 7b2b53d34c5..80f4b17533a 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java @@ -20,6 +20,8 @@ import static com.google.common.base.Preconditions.checkNotNull; +import java.util.concurrent.Semaphore; + import org.apache.bookkeeper.bookie.StateManager; import org.apache.bookkeeper.common.util.JsonUtil; import org.apache.bookkeeper.conf.ServerConfiguration; @@ -49,6 +51,7 @@ public class BookieSanityService implements HttpEndpointService { private final ServerConfiguration config; + private Semaphore lock = new Semaphore(1); public BookieSanityService(ServerConfiguration config) { this.config = checkNotNull(config); @@ -78,8 +81,15 @@ public HttpServiceResponse handle(HttpServiceRequest request) throws Exception { if (config.isForceReadOnlyBookie()) { bs.readOnly = true; } else { - SanityTestCommand sanity = new SanityTestCommand(); - bs.passed = sanity.apply(config, new SanityTestCommand.SanityFlags()); + try { + // allow max concurrent request as sanity-test check relatively + // longer time to complete + lock.acquire(); + SanityTestCommand sanity = new SanityTestCommand(); + bs.passed = sanity.apply(config, new SanityTestCommand.SanityFlags()); + } finally { + lock.release(); + } } String jsonResponse = JsonUtil.toJson(bs); 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 index 96e7ce1d86a..076cfbf5399 100644 --- 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 @@ -865,10 +865,13 @@ public void testGetBookieSanity() throws Exception { BookieSanityService service = new BookieSanityService(conf); HttpServiceResponse response1 = service.handle(request1); assertEquals(HttpServer.StatusCode.OK.getValue(), response1.getStatusCode()); - BookieSanity bs = JsonUtil.fromJson(response1.getBody(), BookieSanity.class); - assertEquals(true, bs.isPassed()); - assertEquals(false, bs.isReadOnly()); - + // run multiple iteration to validate any server side throttling doesn't + // fail sequential requests. + for (int i = 0; i < 3; i++) { + BookieSanity bs = JsonUtil.fromJson(response1.getBody(), BookieSanity.class); + assertEquals(true, bs.isPassed()); + assertEquals(false, bs.isReadOnly()); + } HttpServiceResponse response2 = bookieStateServer.handle(request1); assertEquals(HttpServer.StatusCode.OK.getValue(), response2.getStatusCode()); } From 944dfd58af955ab5f374c66a3a3f8191ee5cae0f Mon Sep 17 00:00:00 2001 From: rajan Date: Fri, 18 Nov 2022 13:35:40 -0800 Subject: [PATCH 3/3] Add request timeout --- .../http/service/BookieSanityService.java | 67 +++++++++++-------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java index 80f4b17533a..9d786425a39 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/server/http/service/BookieSanityService.java @@ -21,8 +21,8 @@ import static com.google.common.base.Preconditions.checkNotNull; import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; -import org.apache.bookkeeper.bookie.StateManager; import org.apache.bookkeeper.common.util.JsonUtil; import org.apache.bookkeeper.conf.ServerConfiguration; import org.apache.bookkeeper.http.HttpServer; @@ -30,6 +30,8 @@ import org.apache.bookkeeper.http.service.HttpServiceRequest; import org.apache.bookkeeper.http.service.HttpServiceResponse; import org.apache.bookkeeper.tools.cli.commands.bookie.SanityTestCommand; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import lombok.Data; import lombok.NoArgsConstructor; @@ -37,7 +39,8 @@ /** * HttpEndpointService that exposes the bookie sanity state. * - *

Get the current bookie sanity response: + *

+ * Get the current bookie sanity response: * * 

  * 
@@ -50,8 +53,11 @@
  */
 public class BookieSanityService implements HttpEndpointService {
 
+    static final Logger LOG = LoggerFactory.getLogger(BookieSanityService.class);
     private final ServerConfiguration config;
     private Semaphore lock = new Semaphore(1);
+    private static final int TIMEOUT_MS = 5000;
+    private static final int MAX_CONCURRENT_REQUESTS = 1;
 
     public BookieSanityService(ServerConfiguration config) {
         this.config = checkNotNull(config);
@@ -68,33 +74,40 @@ public static class BookieSanity {
     }
 
     @Override
-	public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
-		HttpServiceResponse response = new HttpServiceResponse();
+    public HttpServiceResponse handle(HttpServiceRequest request) throws Exception {
+        HttpServiceResponse response = new HttpServiceResponse();
 
-		if (HttpServer.Method.GET != request.getMethod()) {
-			response.setCode(HttpServer.StatusCode.NOT_FOUND);
-			response.setBody("Only support GET method to retrieve bookie sanity state.");
-			return response;
-		}
+        if (HttpServer.Method.GET != request.getMethod()) {
+            response.setCode(HttpServer.StatusCode.NOT_FOUND);
+            response.setBody("Only support GET method to retrieve bookie sanity state.");
+            return response;
+        }
 
-		BookieSanity bs = new BookieSanity();
-		if (config.isForceReadOnlyBookie()) {
-			bs.readOnly = true;
-		} else {
-		    try {
+        BookieSanity bs = new BookieSanity();
+        if (config.isForceReadOnlyBookie()) {
+            bs.readOnly = true;
+        } else {
+            try {
                 // allow max concurrent request as sanity-test check relatively
                 // longer time to complete
-		        lock.acquire();
-	            SanityTestCommand sanity = new SanityTestCommand();
-	            bs.passed = sanity.apply(config, new SanityTestCommand.SanityFlags());
-		    } finally {
-		        lock.release();
-		    }
-		}
-
-		String jsonResponse = JsonUtil.toJson(bs);
-		response.setBody(jsonResponse);
-		response.setCode(HttpServer.StatusCode.OK);
-		return response;
-	}
+                try {
+                    lock.tryAcquire(MAX_CONCURRENT_REQUESTS, TIMEOUT_MS, TimeUnit.MILLISECONDS);
+                } catch (InterruptedException e) {
+                    LOG.error("Timing out due to max {} of sanity request are running concurrently",
+                            MAX_CONCURRENT_REQUESTS);
+                    response.setCode(HttpServer.StatusCode.INTERNAL_ERROR);
+                    response.setBody("Timing out due to max number of sanity request are running concurrently");
+                    return response;
+                }
+                SanityTestCommand sanity = new SanityTestCommand();
+                bs.passed = sanity.apply(config, new SanityTestCommand.SanityFlags());
+            } finally {
+                lock.release();
+            }
+        }
+        String jsonResponse = JsonUtil.toJson(bs);
+        response.setBody(jsonResponse);
+        response.setCode(HttpServer.StatusCode.OK);
+        return response;
+    }
 }