You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2022/12/06 02:59:55 UTC
[pinot] branch master updated: Allow liveness check during server shutting down (#9915)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new a1010a4e26 Allow liveness check during server shutting down (#9915)
a1010a4e26 is described below
commit a1010a4e263d086789e317e81f658a0205012f3c
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Mon Dec 5 18:59:50 2022 -0800
Allow liveness check during server shutting down (#9915)
---
.../helix => api}/AdminApiApplication.java | 22 ++++++----
.../server/api/resources/HealthCheckResource.java | 2 +-
.../api/resources/PinotServerAppConfigs.java | 2 +-
.../pinot/server/api/resources/ShutDownFilter.java | 49 ++++++++++++++++++++++
.../pinot/server/api/resources/TablesResource.java | 2 +-
.../server/starter/helix/BaseServerStarter.java | 4 +-
.../apache/pinot/server/api/AccessControlTest.java | 1 -
.../apache/pinot/server/api/BaseResourceTest.java | 5 +--
.../pinot/server/api/HealthCheckResourceTest.java | 40 +++++++++++-------
9 files changed, 95 insertions(+), 32 deletions(-)
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/AdminApiApplication.java b/pinot-server/src/main/java/org/apache/pinot/server/api/AdminApiApplication.java
similarity index 93%
rename from pinot-server/src/main/java/org/apache/pinot/server/starter/helix/AdminApiApplication.java
rename to pinot-server/src/main/java/org/apache/pinot/server/api/AdminApiApplication.java
index c0dc370cf9..823b349901 100644
--- a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/AdminApiApplication.java
+++ b/pinot-server/src/main/java/org/apache/pinot/server/api/AdminApiApplication.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.server.starter.helix;
+package org.apache.pinot.server.api;
import io.swagger.jaxrs.config.BeanConfig;
import java.io.IOException;
@@ -25,6 +25,7 @@ import java.net.URL;
import java.net.URLClassLoader;
import java.net.UnknownHostException;
import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.container.ContainerResponseContext;
import javax.ws.rs.container.ContainerResponseFilter;
@@ -52,21 +53,20 @@ public class AdminApiApplication extends ResourceConfig {
public static final String RESOURCE_PACKAGE = "org.apache.pinot.server.api.resources";
public static final String SERVER_INSTANCE_ID = "serverInstanceId";
+ private final AtomicBoolean _shutDownInProgress = new AtomicBoolean();
private final ServerInstance _serverInstance;
- private final AccessControlFactory _accessControlFactory;
- private boolean _started = false;
private HttpServer _httpServer;
public AdminApiApplication(ServerInstance instance, AccessControlFactory accessControlFactory,
PinotConfiguration serverConf) {
_serverInstance = instance;
- _accessControlFactory = accessControlFactory;
packages(RESOURCE_PACKAGE);
property(PINOT_CONFIGURATION, serverConf);
register(new AbstractBinder() {
@Override
protected void configure() {
+ bind(_shutDownInProgress).to(AtomicBoolean.class);
bind(_serverInstance).to(ServerInstance.class);
bind(_serverInstance.getServerMetrics()).to(ServerMetrics.class);
bind(accessControlFactory).to(AccessControlFactory.class);
@@ -110,7 +110,6 @@ public class AdminApiApplication extends ResourceConfig {
LOGGER.info("Starting swagger for the Pinot server.");
PinotReflectionUtils.runWithLock(() -> setupSwagger(pinotConfiguration));
}
- _started = true;
return true;
}
@@ -147,10 +146,17 @@ public class AdminApiApplication extends ResourceConfig {
_httpServer.getServerConfiguration().addHttpHandler(swaggerDist, "/swaggerui-dist/");
}
+ /**
+ * Starts shutting down the HTTP server, which rejects all requests except for the liveness check.
+ */
+ public void startShuttingDown() {
+ _shutDownInProgress.set(true);
+ }
+
+ /**
+ * Stops the HTTP server.
+ */
public void stop() {
- if (!_started) {
- return;
- }
_httpServer.shutdownNow();
}
}
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/HealthCheckResource.java b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/HealthCheckResource.java
index f5bc5ec3ff..11c71afdfd 100644
--- a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/HealthCheckResource.java
+++ b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/HealthCheckResource.java
@@ -36,7 +36,7 @@ import org.apache.pinot.common.metrics.ServerMeter;
import org.apache.pinot.common.metrics.ServerMetrics;
import org.apache.pinot.common.utils.ServiceStatus;
import org.apache.pinot.common.utils.ServiceStatus.Status;
-import org.apache.pinot.server.starter.helix.AdminApiApplication;
+import org.apache.pinot.server.api.AdminApiApplication;
/**
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/PinotServerAppConfigs.java b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/PinotServerAppConfigs.java
index 7ff922feb6..677ff46c92 100644
--- a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/PinotServerAppConfigs.java
+++ b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/PinotServerAppConfigs.java
@@ -31,7 +31,7 @@ import javax.ws.rs.core.Context;
import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import org.apache.pinot.common.utils.PinotAppConfigs;
-import org.apache.pinot.server.starter.helix.AdminApiApplication;
+import org.apache.pinot.server.api.AdminApiApplication;
import org.apache.pinot.spi.env.PinotConfiguration;
import static org.apache.pinot.spi.utils.CommonConstants.SWAGGER_AUTHORIZATION_KEY;
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/ShutDownFilter.java b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/ShutDownFilter.java
new file mode 100644
index 0000000000..719c2b4c01
--- /dev/null
+++ b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/ShutDownFilter.java
@@ -0,0 +1,49 @@
+/**
+ * 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.pinot.server.api.resources;
+
+import java.io.IOException;
+import java.util.concurrent.atomic.AtomicBoolean;
+import javax.inject.Inject;
+import javax.ws.rs.WebApplicationException;
+import javax.ws.rs.container.ContainerRequestContext;
+import javax.ws.rs.container.ContainerRequestFilter;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.Provider;
+
+
+/**
+ * This filter is used to reject all requests when server is shutting down, and only allow the liveness check to go
+ * through.
+ */
+@Provider
+public class ShutDownFilter implements ContainerRequestFilter {
+
+ @Inject
+ private AtomicBoolean _shutDownInProgress;
+
+ @Override
+ public void filter(ContainerRequestContext requestContext)
+ throws IOException {
+ if (_shutDownInProgress.get() && !(requestContext.getMethod().equals("GET") && requestContext.getUriInfo().getPath()
+ .equals("health/liveness"))) {
+ throw new WebApplicationException("Server is shutting down", Response.Status.SERVICE_UNAVAILABLE);
+ }
+ }
+}
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
index 79df252dcd..27df969782 100644
--- a/pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
+++ b/pinot-server/src/main/java/org/apache/pinot/server/api/resources/TablesResource.java
@@ -85,8 +85,8 @@ import org.apache.pinot.server.access.AccessControl;
import org.apache.pinot.server.access.AccessControlFactory;
import org.apache.pinot.server.access.HttpRequesterIdentity;
import org.apache.pinot.server.access.RequesterIdentity;
+import org.apache.pinot.server.api.AdminApiApplication;
import org.apache.pinot.server.starter.ServerInstance;
-import org.apache.pinot.server.starter.helix.AdminApiApplication;
import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.FieldSpec.DataType;
diff --git a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
index 9589a02a79..68773ab89c 100644
--- a/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
+++ b/pinot-server/src/main/java/org/apache/pinot/server/starter/helix/BaseServerStarter.java
@@ -70,6 +70,7 @@ import org.apache.pinot.core.util.ListenerConfigUtil;
import org.apache.pinot.segment.local.realtime.impl.invertedindex.RealtimeLuceneIndexRefreshState;
import org.apache.pinot.segment.spi.memory.PinotDataBuffer;
import org.apache.pinot.server.access.AccessControlFactory;
+import org.apache.pinot.server.api.AdminApiApplication;
import org.apache.pinot.server.conf.ServerConf;
import org.apache.pinot.server.realtime.ControllerLeaderLocator;
import org.apache.pinot.server.realtime.ServerSegmentCompletionProtocolHandler;
@@ -614,7 +615,7 @@ public abstract class BaseServerStarter implements ServiceStartable {
} catch (IOException e) {
LOGGER.warn("Caught exception closing PinotFS classes", e);
}
- _adminApiApplication.stop();
+ _adminApiApplication.startShuttingDown();
_helixAdmin.setConfig(_instanceConfigScope,
Collections.singletonMap(Helix.IS_SHUTDOWN_IN_PROGRESS, Boolean.toString(true)));
@@ -634,6 +635,7 @@ public abstract class BaseServerStarter implements ServiceStartable {
_realtimeLuceneIndexRefreshState.stop();
LOGGER.info("Deregistering service status handler");
ServiceStatus.removeServiceStatusCallback(_instanceId);
+ _adminApiApplication.stop();
LOGGER.info("Finish shutting down Pinot server for {}", _instanceId);
}
diff --git a/pinot-server/src/test/java/org/apache/pinot/server/api/AccessControlTest.java b/pinot-server/src/test/java/org/apache/pinot/server/api/AccessControlTest.java
index 23e1678705..146b758f15 100644
--- a/pinot-server/src/test/java/org/apache/pinot/server/api/AccessControlTest.java
+++ b/pinot-server/src/test/java/org/apache/pinot/server/api/AccessControlTest.java
@@ -41,7 +41,6 @@ import org.apache.pinot.server.access.GrpcRequesterIdentity;
import org.apache.pinot.server.access.HttpRequesterIdentity;
import org.apache.pinot.server.access.RequesterIdentity;
import org.apache.pinot.server.starter.ServerInstance;
-import org.apache.pinot.server.starter.helix.AdminApiApplication;
import org.apache.pinot.server.starter.helix.DefaultHelixStarterServerConfig;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.CommonConstants;
diff --git a/pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java b/pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java
index 7eba8b7f0d..77288b8286 100644
--- a/pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java
+++ b/pinot-server/src/test/java/org/apache/pinot/server/api/BaseResourceTest.java
@@ -49,7 +49,6 @@ import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
import org.apache.pinot.segment.spi.creator.SegmentIndexCreationDriver;
import org.apache.pinot.server.access.AllowAllAccessFactory;
import org.apache.pinot.server.starter.ServerInstance;
-import org.apache.pinot.server.starter.helix.AdminApiApplication;
import org.apache.pinot.server.starter.helix.DefaultHelixStarterServerConfig;
import org.apache.pinot.spi.env.PinotConfiguration;
import org.apache.pinot.spi.utils.CommonConstants;
@@ -84,8 +83,8 @@ public abstract class BaseResourceTest {
private final Map<String, TableDataManager> _tableDataManagerMap = new HashMap<>();
protected final List<ImmutableSegment> _realtimeIndexSegments = new ArrayList<>();
protected final List<ImmutableSegment> _offlineIndexSegments = new ArrayList<>();
- private File _avroFile;
- private AdminApiApplication _adminApiApplication;
+ protected File _avroFile;
+ protected AdminApiApplication _adminApiApplication;
protected WebTarget _webTarget;
protected String _instanceId;
diff --git a/pinot-server/src/test/java/org/apache/pinot/server/api/HealthCheckResourceTest.java b/pinot-server/src/test/java/org/apache/pinot/server/api/HealthCheckResourceTest.java
index 0484adc7ba..e4e80ff7b5 100644
--- a/pinot-server/src/test/java/org/apache/pinot/server/api/HealthCheckResourceTest.java
+++ b/pinot-server/src/test/java/org/apache/pinot/server/api/HealthCheckResourceTest.java
@@ -20,17 +20,17 @@ package org.apache.pinot.server.api;
import javax.ws.rs.core.Response;
import org.apache.pinot.common.utils.ServiceStatus;
-import org.testng.Assert;
import org.testng.annotations.Test;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import static org.testng.Assert.assertEquals;
public class HealthCheckResourceTest extends BaseResourceTest {
+
@Test
- public void checkHealthProbes()
- throws Exception {
+ public void checkHealthProbes() {
String healthPath = "/health";
String livenessPath = "/health/liveness";
String readinessPath = "/health/readiness";
@@ -40,22 +40,30 @@ public class HealthCheckResourceTest extends BaseResourceTest {
when(mockSuccessCallback.getServiceStatus()).thenReturn(ServiceStatus.Status.GOOD);
when(mockFailureCallback.getServiceStatus()).thenReturn(ServiceStatus.Status.BAD);
- Assert.assertEquals(_webTarget.path(livenessPath).request().get(Response.class).getStatus(), 200);
- Assert.assertEquals(_webTarget.path(healthPath).request().get(Response.class).getStatus(), 503);
- Assert.assertEquals(_webTarget.path(readinessPath).request().get(Response.class).getStatus(), 503);
+ assertEquals(_webTarget.path(livenessPath).request().get(Response.class).getStatus(), 200);
+ assertEquals(_webTarget.path(healthPath).request().get(Response.class).getStatus(), 503);
+ assertEquals(_webTarget.path(readinessPath).request().get(Response.class).getStatus(), 503);
ServiceStatus.setServiceStatusCallback(_instanceId, mockSuccessCallback);
- Assert.assertEquals(_webTarget.path(livenessPath).request().get(Response.class).getStatus(), 200);
- Assert.assertEquals(_webTarget.path(healthPath).request().get(Response.class).getStatus(), 200);
- Assert.assertEquals(_webTarget.path(readinessPath).request().get(Response.class).getStatus(), 200);
+ assertEquals(_webTarget.path(livenessPath).request().get(Response.class).getStatus(), 200);
+ assertEquals(_webTarget.path(healthPath).request().get(Response.class).getStatus(), 200);
+ assertEquals(_webTarget.path(readinessPath).request().get(Response.class).getStatus(), 200);
ServiceStatus.setServiceStatusCallback(_instanceId, mockFailureCallback);
- Assert.assertEquals(_webTarget.path(livenessPath).request().get(Response.class).getStatus(), 200);
- Assert.assertEquals(_webTarget.path(healthPath).request().get(Response.class).getStatus(), 503);
- Assert.assertEquals(_webTarget.path(readinessPath).request().get(Response.class).getStatus(), 503);
- Assert.assertEquals(_webTarget.path(healthPath).queryParam("checkType", "readiness")
- .request().get(Response.class).getStatus(), 503);
- Assert.assertEquals(_webTarget.path(healthPath).queryParam("checkType", "liveness")
- .request().get(Response.class).getStatus(), 200);
+ assertEquals(_webTarget.path(livenessPath).request().get(Response.class).getStatus(), 200);
+ assertEquals(_webTarget.path(healthPath).request().get(Response.class).getStatus(), 503);
+ assertEquals(_webTarget.path(readinessPath).request().get(Response.class).getStatus(), 503);
+ assertEquals(
+ _webTarget.path(healthPath).queryParam("checkType", "readiness").request().get(Response.class).getStatus(),
+ 503);
+ assertEquals(
+ _webTarget.path(healthPath).queryParam("checkType", "liveness").request().get(Response.class).getStatus(), 200);
+
+ // Start shutting down the HTTP server, only liveness check should go through
+ ServiceStatus.setServiceStatusCallback(_instanceId, mockSuccessCallback);
+ _adminApiApplication.startShuttingDown();
+ assertEquals(_webTarget.path(livenessPath).request().get(Response.class).getStatus(), 200);
+ assertEquals(_webTarget.path(healthPath).request().get(Response.class).getStatus(), 503);
+ assertEquals(_webTarget.path(readinessPath).request().get(Response.class).getStatus(), 503);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org