-
Notifications
You must be signed in to change notification settings - Fork 639
feat(server): add a storage-aware GET /readiness endpoint #3221
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
d0f4a25
c9e70b1
213fd9c
4a4aa1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| /* | ||
| * 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.hugegraph.api.profile; | ||
|
|
||
| import java.util.Map; | ||
|
|
||
| import org.apache.hugegraph.api.API; | ||
| import org.apache.hugegraph.config.HugeConfig; | ||
| import org.apache.hugegraph.config.ServerOptions; | ||
| import org.apache.hugegraph.core.GraphManager; | ||
| import org.apache.hugegraph.util.JsonUtil; | ||
|
|
||
| import com.codahale.metrics.annotation.Timed; | ||
|
|
||
| import io.swagger.v3.oas.annotations.tags.Tag; | ||
| import jakarta.annotation.security.PermitAll; | ||
| import jakarta.inject.Singleton; | ||
| import jakarta.ws.rs.GET; | ||
| import jakarta.ws.rs.Path; | ||
| import jakarta.ws.rs.Produces; | ||
| import jakarta.ws.rs.core.Context; | ||
| import jakarta.ws.rs.core.Response; | ||
|
|
||
| /** | ||
| * Storage-aware readiness for Kubernetes and load balancers: 200 while at | ||
| * least one known Store answers this server, 503 while none does (or, before | ||
| * any Store list is known, while PD does not answer). Unauthenticated, like | ||
| * /versions, so that an httpGet probe needs no credential; the body carries | ||
| * no addresses and no raw exception text. | ||
| */ | ||
| @Path("readiness") | ||
| @Singleton | ||
| @Tag(name = "ReadinessAPI") | ||
| public class ReadinessAPI extends API { | ||
|
|
||
| @GET | ||
| @Timed | ||
| @Produces(APPLICATION_JSON_WITH_CHARSET) | ||
| @PermitAll | ||
| public Response get(@Context GraphManager manager, @Context HugeConfig conf) { | ||
| Map<String, Object> body = StorageReadiness.check( | ||
| manager, conf.get(ServerOptions.READINESS_TIMEOUT), | ||
| conf.get(ServerOptions.READINESS_CACHE_TTL)); | ||
| Response.Status status = StorageReadiness.isReady(body) ? | ||
| Response.Status.OK : | ||
| Response.Status.SERVICE_UNAVAILABLE; | ||
| return Response.status(status) | ||
| .type(APPLICATION_JSON_WITH_CHARSET) | ||
| .entity(JsonUtil.toJson(body)) | ||
| .build(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| /* | ||
| * 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.hugegraph.api.profile; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.LinkedHashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import org.apache.hugegraph.HugeGraph; | ||
| import org.apache.hugegraph.auth.HugeGraphAuthProxy; | ||
| import org.apache.hugegraph.core.GraphManager; | ||
| import org.apache.hugegraph.util.Log; | ||
| import org.slf4j.Logger; | ||
|
|
||
| /** | ||
| * Whether this server can serve graph traffic, for a readiness probe. | ||
| * Graphs on an embedded backend are ready as soon as the REST layer answers. | ||
| * Graphs on hstore are probed through the backend's "storage_readiness" | ||
| * metadata: at least one known Store answers a cheap direct call; PD is only | ||
| * needed until the first Store list is known. The storage is shared by every | ||
| * hstore graph of the process, so one graph is probed and the result is | ||
| * reused for a short TTL to keep repeated probes cheap. The body never | ||
| * carries raw exception text, since the endpoint is unauthenticated. | ||
| */ | ||
| public final class StorageReadiness { | ||
|
|
||
| public static final String STORAGE_READINESS_META = "storage_readiness"; | ||
| public static final String BACKEND_HSTORE = "hstore"; | ||
|
|
||
| private static final Logger LOG = Log.logger(StorageReadiness.class); | ||
|
|
||
| private static volatile Map<String, Object> lastResult; | ||
| private static volatile long lastCheckedAt; | ||
|
|
||
| private StorageReadiness() { | ||
| } | ||
|
|
||
| /** One storage probe with a time budget in ms. */ | ||
| public interface Probe { | ||
|
|
||
| Map<String, Object> probe(long timeoutMs) throws Exception; | ||
| } | ||
|
|
||
| public static Map<String, Object> check(GraphManager manager, | ||
| long timeoutMs, long cacheTtlMs) { | ||
| // The graphs are auth proxies and the probe request carries no user, | ||
| // so look the graph up and probe it as the internal admin, the way | ||
| // other internal paths do; the result carries no data or addresses | ||
| List<Map<String, Object>> holder = new ArrayList<>(1); | ||
| HugeGraphAuthProxy.runAsAdmin(() -> { | ||
| HugeGraph graph = firstHstoreGraph(manager); | ||
| if (graph == null) { | ||
| Map<String, Object> body = new LinkedHashMap<>(); | ||
| body.put("ready", true); | ||
| body.put("storage", "embedded"); | ||
| body.put("reason", "no graph on a remote storage"); | ||
| holder.add(body); | ||
| return; | ||
| } | ||
| holder.add(check(t -> graph.metadata(null, STORAGE_READINESS_META, t), | ||
| timeoutMs, cacheTtlMs)); | ||
| }); | ||
| return holder.get(0); | ||
| } | ||
|
|
||
| public static synchronized Map<String, Object> check(Probe probe, long timeoutMs, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| long cacheTtlMs) { | ||
| long now = System.currentTimeMillis(); | ||
| Map<String, Object> cached = lastResult; | ||
| if (cached != null && now - lastCheckedAt < cacheTtlMs) { | ||
| Map<String, Object> body = new LinkedHashMap<>(cached); | ||
| body.put("cached", true); | ||
| return body; | ||
| } | ||
| Map<String, Object> body = new LinkedHashMap<>(); | ||
| body.put("ready", false); | ||
| body.put("storage", BACKEND_HSTORE); | ||
| try { | ||
| Map<String, Object> result = probe.probe(timeoutMs); | ||
| body.putAll(result); | ||
| } catch (Throwable e) { | ||
| LOG.warn("Storage readiness probe failed", e); | ||
| body.put("ready", false); | ||
| body.put("reason", "probe failed: " + e.getClass().getSimpleName()); | ||
| } | ||
| body.put("cached", false); | ||
| lastResult = body; | ||
| lastCheckedAt = System.currentTimeMillis(); | ||
| return new LinkedHashMap<>(body); | ||
| } | ||
|
|
||
| public static boolean isReady(Map<String, Object> body) { | ||
| return Boolean.TRUE.equals(body.get("ready")); | ||
| } | ||
|
|
||
| public static synchronized void resetCache() { | ||
| lastResult = null; | ||
| lastCheckedAt = 0L; | ||
| } | ||
|
|
||
| private static HugeGraph firstHstoreGraph(GraphManager manager) { | ||
| for (String name : manager.graphs()) { | ||
| try { | ||
| HugeGraph graph = manager.graph(name); | ||
| if (graph != null && BACKEND_HSTORE.equals(graph.backend())) { | ||
| return graph; | ||
| } | ||
| } catch (Throwable e) { | ||
| LOG.debug("Skip graph {} while looking for a remote storage", name, e); | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
readinessis added to the auth and path whitelists but not toLoadDetectFilter.WHITE_API_LIST("",apis,metrics,versions). So/readinessgoes through the worker-load and free-memory checks, before the result cache is reached, and gets a 503 oncemax_worker_threads - 1other requests are in flight.restserver.max_worker_threadsdefaults to2 * CPUS, so on a 2-CPU pod the probe fails while 3 other requests run.Under sustained load Kubernetes then drops busy Servers from the Service and shifts their traffic to the rest, so a deployment that moves its readiness probe from
/versionsto/readinesscan lose every endpoint during a spike while storage is fine.Could
readinessgo intoWHITE_API_LISTnext toversions(LoadReleaseFilterreads the same list, so the counter stays balanced), with a case liketestFilter_WhiteListPathIgnored? If shedding load through readiness is intended, please say so in theReadinessAPIJavadoc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in 4a4aa1c, thanks, that would have been a nasty interaction: a readiness probe shed under load would pull exactly the busiest Servers out of the Service while the storage is healthy.
readinessis inLoadDetectFilter.WHITE_API_LISTnext toversions;LoadReleaseFilterreads the same list, so theworkLoadcounter stays balanced.testFilter_ReadinessIgnoredLikeVersionsinLoadDetectFilterTest: with a 2-thread limit and one request in flight the filter lets/readinessthrough without touching the counter and without a log entry, in the same shape astestFilter_WhiteListPathIgnored. Readiness is not meant to shed load; theReadinessAPIJavadoc says it answers from the storage state.