Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public class AuthenticationFilter implements ContainerRequestFilter, ContainerRe
private static final AntPathMatcher MATCHER = new AntPathMatcher();
private static final Set<String> FIXED_WHITE_API_SET = ImmutableSet.of(
"versions",
"readiness",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ readiness is added to the auth and path whitelists but not to LoadDetectFilter.WHITE_API_LIST ("", apis, metrics, versions). So /readiness goes through the worker-load and free-memory checks, before the result cache is reached, and gets a 503 once max_worker_threads - 1 other requests are in flight. restserver.max_worker_threads defaults to 2 * 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 /versions to /readiness can lose every endpoint during a spike while storage is fine.

Could readiness go into WHITE_API_LIST next to versions (LoadReleaseFilter reads the same list, so the counter stays balanced), with a case like testFilter_WhiteListPathIgnored? If shedding load through readiness is intended, please say so in the ReadinessAPI Javadoc.

Copy link
Copy Markdown
Contributor Author

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. readiness is in LoadDetectFilter.WHITE_API_LIST next to versions; LoadReleaseFilter reads the same list, so the workLoad counter stays balanced. testFilter_ReadinessIgnoredLikeVersions in LoadDetectFilterTest: with a 2-thread limit and one request in flight the filter lets /readiness through without touching the counter and without a log entry, in the same shape as testFilter_WhiteListPathIgnored. Readiness is not meant to shed load; the ReadinessAPI Javadoc says it answers from the storage state.

"openapi.json"
);
/** Remove auth/login API from whitelist */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ public class LoadDetectFilter implements ContainerRequestFilter {
"",
"apis",
"metrics",
"versions"
"versions",
"readiness"
);

// Call gc every 30+ seconds if memory is low and request frequently
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public class PathFilter implements ContainerRequestFilter {
"apis",
"metrics",
"versions",
"readiness",
"health",
"gremlin",
"graphs/auth",
Expand Down
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,
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,30 @@ public class ServerOptions extends OptionHolder {
300
);

public static final ConfigOption<Integer> READINESS_TIMEOUT =
new ConfigOption<>(
"readiness.timeout",
"The whole time budget in ms of one GET /readiness probe: " +
"one cheap call to every known Store in parallel, the " +
"first answer wins; the PD call that refreshes the Store " +
"list runs in the background and is only waited for " +
"before the first list is known. No Store answering " +
"within the budget makes the server report not ready.",
rangeInt(100, 60000),
1000
);

public static final ConfigOption<Integer> READINESS_CACHE_TTL =
new ConfigOption<>(
"readiness.cache_ttl",
"How many ms a GET /readiness result is reused before " +
"the storage is probed again, so that several probes " +
"(Kubernetes, load balancers) cost one round of Store " +
"calls per interval; 0 probes on every request.",
rangeInt(0, 60000),
2000
);

public static final ConfigOption<Boolean> SERVER_USE_K8S =
new ConfigOption<>(
"server.use_k8s",
Expand Down
Loading
Loading