diff --git a/docs/usage/rest_api/interpreter.md b/docs/usage/rest_api/interpreter.md index 9d81dd60e06..1a52ce40170 100644 --- a/docs/usage/rest_api/interpreter.md +++ b/docs/usage/rest_api/interpreter.md @@ -520,6 +520,108 @@ The role of registered interpreters, settings and interpreters group are describ +
+### Health check an interpreter + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
DescriptionThis ```POST``` method probes the interpreter processes of the given interpreter setting + and reports whether they answer. Useful right after a restart, which only closes the + processes and leaves the next use to start them again. The check never starts an + interpreter: a setting with nothing running is reported with the reason + ```NOT_RUNNING```. A setting can own one process per user or note, so the response holds + an entry per interpreter group. The server bounds how long it waits, and a group whose + probe did not answer in time is reported with the reason ```PROBE_TIMEOUT``` rather than + holding the request.
URL```http://[zeppelin-server]:[zeppelin-port]/api/interpreter/setting/[interpreter ID]/healthcheck```
Success code200
Fail code 403 if the note of the request may not be acted on, 404 if there is no such interpreter setting
Sample JSON input (Optional)The note to authorize the caller against, needed only when the caller is not allowed to + reach the interpreter endpoints on their own. Running a paragraph of that note is enough. + +```json +{ + "noteId": "2AVQJVC8N" +} +``` +
Sample JSON response```healthy``` follows whether the interpreter answered. ```alive``` and ```running``` + are left out when no probe was made, so that an interpreter found dead stays + distinguishable from one that was never asked. + +```json +{ + "status": "OK", + "message": "", + "body": { + "settingId": "2CH2VMY7B", + "settingName": "spark", + "groups": [ + { + "groupId": "2CH2VMY7B-shared_process", + "healthy": true, + "reason": "OK", + "alive": true, + "running": true, + "probeTookMs": 3 + }, + { + "groupId": "2CH2VMY7B-user2", + "healthy": false, + "reason": "NOT_REACHABLE", + "alive": true, + "running": false, + "probeTookMs": 1004 + }, + { + "groupId": "2CH2VMY7B-user3", + "healthy": false, + "reason": "PROBE_TIMEOUT", + "probeTookMs": 3000 + } + ] + } +} +``` +
Sample JSON response of an interpreter that is not running + +```json +{ + "status": "OK", + "message": "", + "body": { + "settingId": "2CH2VMY7B", + "settingName": "spark", + "reason": "NOT_RUNNING", + "groups": [] + } +} +``` +
+
### Add a new repository for dependency resolving diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterHealthCheck.java b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterHealthCheck.java new file mode 100644 index 00000000000..75c6f16a971 --- /dev/null +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterHealthCheck.java @@ -0,0 +1,185 @@ +/* + * 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.zeppelin.interpreter; + +import java.util.Collections; +import java.util.List; + +/** + * Result of a user triggered health check of a single interpreter setting. + * + *

A setting owns zero or more {@link ManagedInterpreterGroup}s, one per interpreter process, so + * the result reports an entry per group instead of one verdict for the setting. + * + *

{@code healthy} follows {@code InterpreterClient#isRunning()}, documented as "the interpreter + * can communicate with server", while {@code isAlive()} only states that a process exists. Both are + * reported so that a process which is up but no longer answering stays distinguishable from one + * that is gone. + * + *

{@code alive} and {@code running} are {@code null} whenever no probe was performed, so that + * "probed and found dead" does not read the same as "never probed". + */ +public class InterpreterHealthCheck { + + /** + * Why a group is, or is not, healthy. Only {@link #OK} means healthy; the remaining reasons are + * not equally bad, and a client is expected to tell them apart rather than to treat every one of + * them as a failure. + */ + public enum Reason { + /** The interpreter answered the probe. */ + OK, + /** A process handle exists, but the interpreter did not answer. */ + NOT_REACHABLE, + /** The probe did not finish within the budget of this health check. */ + PROBE_TIMEOUT, + /** A process is currently being launched for this group, which is not a failure. */ + LAUNCHING, + /** No process is running, which is the normal state right after a restart. */ + NOT_RUNNING + } + + private final String settingId; + private final String settingName; + private final Reason reason; + private final List groups; + + private InterpreterHealthCheck(InterpreterSetting setting, Reason reason, + List groups) { + this.settingId = setting.getId(); + this.settingName = setting.getName(); + this.reason = reason; + this.groups = groups; + } + + /** + * A setting that holds no interpreter group at all. Restarting a setting only closes its + * processes and the next use lazily creates them again, so this is the expected state right after + * a restart rather than something being wrong. + */ + public static InterpreterHealthCheck notRunning(InterpreterSetting setting) { + return new InterpreterHealthCheck(setting, Reason.NOT_RUNNING, Collections.emptyList()); + } + + /** + * A setting with groups to report. The reason lives on each group, so no setting wide reason is + * set here. + */ + public static InterpreterHealthCheck of(InterpreterSetting setting, List groups) { + return new InterpreterHealthCheck(setting, null, groups); + } + + public String getSettingId() { + return settingId; + } + + public String getSettingName() { + return settingName; + } + + public Reason getReason() { + return reason; + } + + public List getGroups() { + return groups; + } + + /** + * Health of one interpreter group, which is to say of one interpreter process. + * + *

Instances are created through the factory methods so that {@code healthy} is always derived + * from the same probe result the reason was derived from, and the two can never disagree. + */ + public static class GroupHealth { + + private final String groupId; + private final boolean healthy; + private final Reason reason; + private final Boolean alive; + private final Boolean running; + private final long probeTookMs; + + private GroupHealth(String groupId, Reason reason, Boolean alive, Boolean running, + long probeTookMs) { + this.groupId = groupId; + this.reason = reason; + this.alive = alive; + this.running = running; + this.probeTookMs = probeTookMs; + this.healthy = Boolean.TRUE.equals(running); + } + + /** The probe came back, and whether the interpreter answered decides the reason. */ + public static GroupHealth probed(String groupId, boolean alive, boolean running, + long probeTookMs) { + return new GroupHealth(groupId, running ? Reason.OK : Reason.NOT_REACHABLE, + alive, running, probeTookMs); + } + + /** + * The probe did not come back within the budget. Nothing is known about the process, hence no + * {@code alive} or {@code running}. + */ + public static GroupHealth timedOut(String groupId, long probeTookMs) { + return new GroupHealth(groupId, Reason.PROBE_TIMEOUT, null, null, probeTookMs); + } + + /** + * The probe itself failed, which says the interpreter could not be reached but leaves the state + * of the process unknown, hence no {@code alive} or {@code running}. + */ + public static GroupHealth probeFailed(String groupId, long probeTookMs) { + return new GroupHealth(groupId, Reason.NOT_REACHABLE, null, null, probeTookMs); + } + + /** A process is being launched for this group, so it is deliberately left unprobed. */ + public static GroupHealth launching(String groupId) { + return new GroupHealth(groupId, Reason.LAUNCHING, null, null, 0); + } + + /** The group carries no process handle, so there is nothing to probe. */ + public static GroupHealth notRunning(String groupId) { + return new GroupHealth(groupId, Reason.NOT_RUNNING, null, null, 0); + } + + public String getGroupId() { + return groupId; + } + + public boolean isHealthy() { + return healthy; + } + + public Reason getReason() { + return reason; + } + + public Boolean getAlive() { + return alive; + } + + public Boolean getRunning() { + return running; + } + + public long getProbeTookMs() { + return probeTookMs; + } + } +} diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterHealthChecker.java b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterHealthChecker.java new file mode 100644 index 00000000000..9dce1d50c10 --- /dev/null +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterHealthChecker.java @@ -0,0 +1,201 @@ +/* + * 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.zeppelin.interpreter; + +import com.google.common.annotations.VisibleForTesting; + +import org.apache.zeppelin.interpreter.InterpreterHealthCheck.GroupHealth; +import org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcess; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Future; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.RejectedExecutionException; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +/** + * Probes the interpreter processes of one interpreter setting and reports whether they answer, + * within a deadline that holds for the whole request. + * + *

Unlike the interpreter status snapshot, which is built from server memory alone, this does + * contact the interpreter. That is only acceptable because a probe happens when a user explicitly + * asks for one, against a single setting, and under a deadline: {@code isRunning()} costs a socket + * connect for docker and a kube-apiserver round trip for k8s, so the same call would be an accident + * if a listing made it on a timer. + * + *

The deadline can only be imposed from the outside. {@code isRunning()} takes no timeout + * argument, the socket connect behind the docker implementation hardcodes one second, and the + * kubernetes client is created with the defaults of its library. So the probe runs on another + * thread and this class stops waiting for it, which bounds the response but not the probe: + * {@code Future#cancel(boolean)} interrupts, and neither a blocking socket connect nor an HTTP + * client call ends on an interrupt. A probe thread can therefore stay busy after its result was + * given up on, which is why the pool is small and its threads are daemons. + */ +public class InterpreterHealthChecker { + + private static final Logger LOGGER = LoggerFactory.getLogger(InterpreterHealthChecker.class); + + /** + * Budget for one health check request, shared by all groups of the setting rather than granted to + * each of them, so that an isolated setting cannot turn into a per group multiple of it. + */ + @VisibleForTesting + static final long PROBE_TIMEOUT_IN_MILLIS = 3_000; + + /** + * A probe that was given up on keeps its thread, so this bounds how many can pile up while an + * interpreter host is unreachable. Further probes wait for a thread instead of getting one, and + * are reported as timed out by the deadline if they never start, which is the honest answer. + */ + private static final int MAX_CONCURRENT_PROBES = 4; + + private final long probeTimeoutInMillis; + private final ExecutorService probeExecutor; + + public InterpreterHealthChecker() { + this(PROBE_TIMEOUT_IN_MILLIS); + } + + @VisibleForTesting + InterpreterHealthChecker(long probeTimeoutInMillis) { + this.probeTimeoutInMillis = probeTimeoutInMillis; + this.probeExecutor = newProbeExecutor(); + } + + private static ExecutorService newProbeExecutor() { + ThreadPoolExecutor executor = new ThreadPoolExecutor( + MAX_CONCURRENT_PROBES, MAX_CONCURRENT_PROBES, 60L, TimeUnit.SECONDS, + new LinkedBlockingQueue<>(), + runnable -> { + Thread thread = new Thread(runnable, "InterpreterHealthChecker-probe"); + thread.setDaemon(true); + return thread; + }); + executor.allowCoreThreadTimeOut(true); + return executor; + } + + /** + * Probes every interpreter group of the given setting. + * + *

A setting owns one group per interpreter process, so an isolated setting is probed once per + * user or note rather than once in total. The probes are submitted together and then collected + * against one deadline, so a group that does not answer costs the request its budget once instead + * of delaying the groups that would have answered right away. + */ + public InterpreterHealthCheck check(InterpreterSetting interpreterSetting) { + List interpreterGroups = interpreterSetting.getAllInterpreterGroups(); + if (interpreterGroups.isEmpty()) { + return InterpreterHealthCheck.notRunning(interpreterSetting); + } + + long startTimeInMillis = System.currentTimeMillis(); + // Keeps the report in the order of the groups, whether an entry needed a probe or not. + Map healthByGroupId = new LinkedHashMap<>(); + Map> probes = new LinkedHashMap<>(); + + for (ManagedInterpreterGroup interpreterGroup : interpreterGroups) { + String groupId = interpreterGroup.getId(); + if (interpreterGroup.isLaunchingInterpreterProcess()) { + // The handle is published before the process is ready and a launch can take minutes for + // Spark on YARN, so probing here would report a starting interpreter as broken. + healthByGroupId.put(groupId, GroupHealth.launching(groupId)); + continue; + } + // Read the handle once: it is set to null as soon as the last session of the group closes, so + // checking it and then reading it again would risk a NullPointerException in between. + RemoteInterpreterProcess process = interpreterGroup.getInterpreterProcess(); + if (process == null) { + healthByGroupId.put(groupId, GroupHealth.notRunning(groupId)); + continue; + } + try { + healthByGroupId.put(groupId, null); + probes.put(groupId, probeExecutor.submit(() -> probe(groupId, process))); + } catch (RejectedExecutionException e) { + // Only reachable once stop() ran, since probes queue rather than being rejected. + LOGGER.warn("Not probing interpreter group {}, no longer accepting probes", groupId); + healthByGroupId.put(groupId, GroupHealth.timedOut(groupId, 0)); + } + } + + long deadline = startTimeInMillis + probeTimeoutInMillis; + for (Map.Entry> probe : probes.entrySet()) { + healthByGroupId.put(probe.getKey(), + awaitProbe(probe.getKey(), probe.getValue(), deadline, startTimeInMillis)); + } + return InterpreterHealthCheck.of(interpreterSetting, + new ArrayList<>(healthByGroupId.values())); + } + + /** + * Stops accepting probes. Probes already in flight are interrupted, which they may well ignore, + * but their threads are daemons and do not hold up a shutdown. + */ + public void stop() { + probeExecutor.shutdownNow(); + } + + private GroupHealth probe(String groupId, RemoteInterpreterProcess process) { + long startTimeInMillis = System.currentTimeMillis(); + try { + boolean running = process.isRunning(); + // An interpreter that answers necessarily has a process, so isAlive() is only worth its own + // remote call when the interpreter did not answer - which is where telling a process that is + // gone from one that is up but mute actually helps. + boolean alive = running || process.isAlive(); + return GroupHealth.probed(groupId, alive, running, + System.currentTimeMillis() - startTimeInMillis); + } catch (Exception e) { + // The probe failing is itself an answer about reachability, so it is reported rather than + // propagated: one unreachable group must not hide the results of the others. + LOGGER.warn("Fail to probe interpreter group: {}", groupId, e); + return GroupHealth.probeFailed(groupId, System.currentTimeMillis() - startTimeInMillis); + } + } + + private GroupHealth awaitProbe(String groupId, Future probe, long deadline, + long startTimeInMillis) { + long remainingInMillis = Math.max(deadline - System.currentTimeMillis(), 0); + try { + return probe.get(remainingInMillis, TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + LOGGER.info("Probe of interpreter group {} did not finish within {}ms", + groupId, probeTimeoutInMillis); + probe.cancel(true); + return GroupHealth.timedOut(groupId, System.currentTimeMillis() - startTimeInMillis); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + probe.cancel(true); + return GroupHealth.timedOut(groupId, System.currentTimeMillis() - startTimeInMillis); + } catch (ExecutionException e) { + // probe() already reports a failing probe, so reaching here means something else broke. + LOGGER.warn("Fail to probe interpreter group: {}", groupId, e.getCause()); + return GroupHealth.probeFailed(groupId, System.currentTimeMillis() - startTimeInMillis); + } + } +} diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java index 8b4c2fe55be..948c0817d55 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/interpreter/InterpreterSettingManager.java @@ -148,6 +148,7 @@ public class InterpreterSettingManager implements NoteEventListener { private List includesInterpreters; private List excludesInterpreters; private final IdleInterpreterReclaimer idleInterpreterReclaimer; + private final InterpreterHealthChecker interpreterHealthChecker = new InterpreterHealthChecker(); @Inject public InterpreterSettingManager(ZeppelinConfiguration zConf, @@ -218,6 +219,15 @@ public IdleInterpreterReclaimer getIdleInterpreterReclaimer() { return idleInterpreterReclaimer; } + /** + * Probes the interpreter processes of the given setting and reports whether they answer. + * + * @see InterpreterHealthChecker + */ + public InterpreterHealthCheck healthCheck(InterpreterSetting interpreterSetting) { + return interpreterHealthChecker.check(interpreterSetting); + } + public RemoteInterpreterEventServer getInterpreterEventServer() { return interpreterEventServer; } @@ -1132,6 +1142,7 @@ public void close(String settingId) { public void close() { idleInterpreterReclaimer.stop(); + interpreterHealthChecker.stop(); List closeThreads = interpreterSettings.values().stream() .map(intpSetting-> new Thread(intpSetting::close, intpSetting.getId() + "-close")) .peek(t -> t.setUncaughtExceptionHandler((th, e) -> diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java index 3b9d754e919..4261def0d6d 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/InterpreterRestApi.java @@ -30,6 +30,7 @@ import org.apache.zeppelin.notebook.AuthorizationService; import org.apache.zeppelin.common.Message; import org.apache.zeppelin.common.Message.OP; +import org.apache.zeppelin.rest.message.HealthCheckInterpreterRequest; import org.apache.zeppelin.rest.message.InterpreterInstallationRequest; import org.apache.zeppelin.rest.message.NewInterpreterSettingRequest; import org.apache.zeppelin.rest.message.RestartInterpreterRequest; @@ -206,12 +207,7 @@ public Response restartSetting(String message, @PathParam("settingId") String se if (null == noteId) { interpreterSettingManager.close(settingId); } else { - Set entities = new HashSet<>(); - entities.add(authenticationService.getPrincipal()); - entities.addAll(authenticationService.getAssociatedRoles()); - if (authorizationService.hasRunPermission(entities, noteId) || - authorizationService.hasWritePermission(entities, noteId) || - authorizationService.isOwner(entities, noteId)) { + if (hasPermissionOnNote(noteId)) { interpreterSettingManager.restart(settingId, authenticationService.getPrincipal(), noteId); } else { return new JsonResponse<>(Status.FORBIDDEN, "No privilege to restart interpreter") @@ -229,6 +225,53 @@ public Response restartSetting(String message, @PathParam("settingId") String se return new JsonResponse<>(Status.OK, "", setting).build(); } + /** + * Health check of an interpreter setting: probes its interpreter processes and reports whether + * they answer. + * + *

This is the one interpreter read path that does contact the interpreter, which is acceptable + * because a user asks for it explicitly, it names a single setting, and the server bounds the + * wait. It never starts an interpreter: a setting with nothing running is reported as such, + * since that is the normal state right after a restart. + * + * @param message HealthCheckInterpreterRequest, optional + */ + @POST + @Path("setting/{settingId}/healthcheck") + @ZeppelinApi + public Response healthCheckSetting(String message, @PathParam("settingId") String settingId) { + InterpreterSetting setting = interpreterSettingManager.get(settingId); + if (setting == null) { + return new JsonResponse<>(Status.NOT_FOUND, "", settingId).build(); + } + + HealthCheckInterpreterRequest request = + GSON.fromJson(message, HealthCheckInterpreterRequest.class); + String noteId = request == null ? null : request.getNoteId(); + if (null != noteId && !hasPermissionOnNote(noteId)) { + return new JsonResponse<>(Status.FORBIDDEN, "No privilege to health check interpreter") + .build(); + } + + LOGGER.info("Health check interpreterSetting {}, user={}", settingId, + authenticationService.getPrincipal()); + return new JsonResponse<>(Status.OK, "", interpreterSettingManager.healthCheck(setting)) + .build(); + } + + /** + * @return whether the current user may act on the interpreters of the given note, which running a + * paragraph of it already implies + */ + private boolean hasPermissionOnNote(String noteId) { + Set entities = new HashSet<>(); + entities.add(authenticationService.getPrincipal()); + entities.addAll(authenticationService.getAssociatedRoles()); + return authorizationService.hasRunPermission(entities, noteId) + || authorizationService.hasWritePermission(entities, noteId) + || authorizationService.isOwner(entities, noteId); + } + /** * List all available interpreters by group. */ diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/HealthCheckInterpreterRequest.java b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/HealthCheckInterpreterRequest.java new file mode 100644 index 00000000000..89ebdebe64b --- /dev/null +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/rest/message/HealthCheckInterpreterRequest.java @@ -0,0 +1,38 @@ +/* + * 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.zeppelin.rest.message; + +/** + * HealthCheckInterpreter rest api request message. + * + *

The note is what a caller who is not an administrator is authorized against, the same way a + * restart from a note page is. A request without one is only served to whoever the deployment lets + * reach the interpreter endpoints at all. + */ +public class HealthCheckInterpreterRequest { + + private final String noteId; + + public HealthCheckInterpreterRequest(String noteId) { + this.noteId = noteId; + } + + public String getNoteId() { + return noteId; + } +} diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterHealthCheckerTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterHealthCheckerTest.java new file mode 100644 index 00000000000..8d4dc03c467 --- /dev/null +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/interpreter/InterpreterHealthCheckerTest.java @@ -0,0 +1,215 @@ +/* + * 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.zeppelin.interpreter; + +import org.apache.zeppelin.interpreter.InterpreterHealthCheck.GroupHealth; +import org.apache.zeppelin.interpreter.InterpreterHealthCheck.Reason; +import org.apache.zeppelin.interpreter.remote.RemoteInterpreterProcess; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * Tests the on demand health check, above all that it stays within its deadline and that it never + * starts an interpreter to answer. + * + *

The deadline is what makes probing acceptable here at all: {@code isRunning()} has no timeout + * of its own, so without it a single unreachable interpreter would hold the request for as long as + * the launcher underneath takes to give up. + */ +class InterpreterHealthCheckerTest { + + /** Short enough to keep the tests quick, long enough that a local probe finishes well inside. */ + private static final long PROBE_TIMEOUT_IN_MILLIS = 500; + + private InterpreterHealthChecker healthChecker; + + @AfterEach + void tearDown() { + if (healthChecker != null) { + healthChecker.stop(); + } + } + + /** + * A setting whose interpreters are not running is the normal state right after a restart, so it + * is reported as such - and answering must not be what starts them. + */ + @Test + void reportsNotRunningWithoutStartingAnInterpreter() throws Exception { + ManagedInterpreterGroup interpreterGroup = interpreterGroup("group-1", null); + InterpreterSetting interpreterSetting = + interpreterSetting(Collections.singletonList(interpreterGroup)); + healthChecker = new InterpreterHealthChecker(PROBE_TIMEOUT_IN_MILLIS); + + InterpreterHealthCheck healthCheck = healthChecker.check(interpreterSetting); + + assertEquals(1, healthCheck.getGroups().size()); + GroupHealth groupHealth = healthCheck.getGroups().get(0); + assertEquals(Reason.NOT_RUNNING, groupHealth.getReason()); + assertFalse(groupHealth.isHealthy(), "a group without a process cannot be healthy"); + assertNull(groupHealth.getAlive(), "nothing was probed, so nothing is known about the process"); + assertNull(groupHealth.getRunning()); + verify(interpreterGroup, never()).getOrCreateInterpreterProcess(anyString(), any()); + } + + /** A setting without any group at all reports on the setting rather than on groups. */ + @Test + void reportsNotRunningWhenThereIsNoGroup() { + InterpreterSetting interpreterSetting = interpreterSetting(Collections.emptyList()); + healthChecker = new InterpreterHealthChecker(PROBE_TIMEOUT_IN_MILLIS); + + InterpreterHealthCheck healthCheck = healthChecker.check(interpreterSetting); + + assertEquals(Reason.NOT_RUNNING, healthCheck.getReason()); + assertTrue(healthCheck.getGroups().isEmpty()); + } + + /** A reachable interpreter is healthy, and saying so takes a single remote call. */ + @Test + void reportsHealthyOnASingleRemoteCall() { + RemoteInterpreterProcess process = mock(RemoteInterpreterProcess.class); + when(process.isRunning()).thenReturn(true); + InterpreterSetting interpreterSetting = interpreterSetting( + Collections.singletonList(interpreterGroup("group-1", process))); + healthChecker = new InterpreterHealthChecker(PROBE_TIMEOUT_IN_MILLIS); + + GroupHealth groupHealth = healthChecker.check(interpreterSetting).getGroups().get(0); + + assertEquals(Reason.OK, groupHealth.getReason()); + assertTrue(groupHealth.isHealthy()); + assertEquals(Boolean.TRUE, groupHealth.getAlive()); + // Asking whether the process exists would be another remote call for no added answer. + verify(process, never()).isAlive(); + } + + /** + * The point of the whole exercise: a probe that does not come back must not hold the request. The + * interpreter here never answers, which is what an unreachable host looks like. + */ + @Test + void stopsWaitingForAProbeThatDoesNotComeBack() { + RemoteInterpreterProcess process = mock(RemoteInterpreterProcess.class); + when(process.isRunning()).thenAnswer(invocation -> { + Thread.sleep(60_000); + return true; + }); + InterpreterSetting interpreterSetting = interpreterSetting( + Collections.singletonList(interpreterGroup("group-1", process))); + healthChecker = new InterpreterHealthChecker(PROBE_TIMEOUT_IN_MILLIS); + + long startTimeInMillis = System.currentTimeMillis(); + GroupHealth groupHealth = healthChecker.check(interpreterSetting).getGroups().get(0); + long tookInMillis = System.currentTimeMillis() - startTimeInMillis; + + assertEquals(Reason.PROBE_TIMEOUT, groupHealth.getReason()); + assertNull(groupHealth.getAlive(), "the probe never answered, so nothing is known"); + assertTrue(tookInMillis < PROBE_TIMEOUT_IN_MILLIS * 10, + "the request should end on the deadline rather than with the probe, but took " + + tookInMillis + "ms"); + } + + /** + * The deadline belongs to the request, so the groups are probed together: one interpreter that + * does not answer must not cost the others their result. + */ + @Test + void oneUnreachableGroupDoesNotHideTheOthers() { + RemoteInterpreterProcess reachable = mock(RemoteInterpreterProcess.class); + when(reachable.isRunning()).thenReturn(true); + RemoteInterpreterProcess unreachable = mock(RemoteInterpreterProcess.class); + when(unreachable.isRunning()).thenAnswer(invocation -> { + Thread.sleep(60_000); + return true; + }); + InterpreterSetting interpreterSetting = interpreterSetting(Arrays.asList( + interpreterGroup("group-unreachable", unreachable), + interpreterGroup("group-reachable", reachable))); + healthChecker = new InterpreterHealthChecker(PROBE_TIMEOUT_IN_MILLIS); + + List groupHealths = healthChecker.check(interpreterSetting).getGroups(); + + assertEquals(2, groupHealths.size()); + // Reported in the order of the groups, whether an entry had to wait for the deadline or not. + assertEquals("group-unreachable", groupHealths.get(0).getGroupId()); + assertEquals(Reason.PROBE_TIMEOUT, groupHealths.get(0).getReason()); + assertEquals("group-reachable", groupHealths.get(1).getGroupId()); + assertEquals(Reason.OK, groupHealths.get(1).getReason()); + } + + /** A group whose process is still being launched is not broken, and is not probed either. */ + @Test + void reportsLaunchingWithoutProbing() { + RemoteInterpreterProcess process = mock(RemoteInterpreterProcess.class); + ManagedInterpreterGroup interpreterGroup = interpreterGroup("group-1", process); + when(interpreterGroup.isLaunchingInterpreterProcess()).thenReturn(true); + InterpreterSetting interpreterSetting = + interpreterSetting(Collections.singletonList(interpreterGroup)); + healthChecker = new InterpreterHealthChecker(PROBE_TIMEOUT_IN_MILLIS); + + GroupHealth groupHealth = healthChecker.check(interpreterSetting).getGroups().get(0); + + assertEquals(Reason.LAUNCHING, groupHealth.getReason()); + verify(process, never()).isRunning(); + } + + /** A probe that throws says the interpreter could not be reached, not that the request failed. */ + @Test + void reportsAFailingProbeAsUnreachable() { + RemoteInterpreterProcess process = mock(RemoteInterpreterProcess.class); + when(process.isRunning()).thenThrow(new RuntimeException("boom")); + InterpreterSetting interpreterSetting = interpreterSetting( + Collections.singletonList(interpreterGroup("group-1", process))); + healthChecker = new InterpreterHealthChecker(PROBE_TIMEOUT_IN_MILLIS); + + GroupHealth groupHealth = healthChecker.check(interpreterSetting).getGroups().get(0); + + assertEquals(Reason.NOT_REACHABLE, groupHealth.getReason()); + assertNull(groupHealth.getRunning(), "the probe failed, so the process state is unknown"); + } + + private static InterpreterSetting interpreterSetting(List groups) { + InterpreterSetting interpreterSetting = mock(InterpreterSetting.class); + when(interpreterSetting.getId()).thenReturn("setting-1"); + when(interpreterSetting.getName()).thenReturn("test"); + when(interpreterSetting.getAllInterpreterGroups()).thenReturn(groups); + return interpreterSetting; + } + + private static ManagedInterpreterGroup interpreterGroup(String groupId, + RemoteInterpreterProcess process) { + ManagedInterpreterGroup interpreterGroup = mock(ManagedInterpreterGroup.class); + when(interpreterGroup.getId()).thenReturn(groupId); + when(interpreterGroup.getInterpreterProcess()).thenReturn(process); + return interpreterGroup; + } +} diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java index 19435b32112..b39c3738ba8 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/InterpreterRestApiTest.java @@ -496,6 +496,38 @@ void testAddDeleteRepository() throws IOException { delete.close(); } + /** + * A setting that has never been used has no interpreter process, which is also its state right + * after a restart, so the health check reports that instead of starting one. + */ + @Test + void testHealthCheckOfAnInterpreterThatIsNotRunning() throws IOException { + InterpreterSetting mdIntpSetting = notebook.getInterpreterSettingManager() + .getInterpreterSettingByName("md"); + + CloseableHttpResponse post = + httpPost("/interpreter/setting/" + mdIntpSetting.getId() + "/healthcheck", ""); + JsonObject body = getBodyFieldFromResponse( + EntityUtils.toString(post.getEntity(), StandardCharsets.UTF_8)); + + assertThat("health check of an unused interpreter:", post, isAllowed()); + assertEquals(mdIntpSetting.getId(), body.get("settingId").getAsString()); + assertEquals("NOT_RUNNING", body.get("reason").getAsString()); + assertEquals(0, body.getAsJsonArray("groups").size()); + assertEquals(0, mdIntpSetting.getAllInterpreterGroups().size(), + "the health check must not have started an interpreter"); + post.close(); + } + + @Test + void testHealthCheckOfNonExistInterpreterSetting() throws IOException { + CloseableHttpResponse post = + httpPost("/interpreter/setting/no_such_setting/healthcheck", ""); + + assertThat("health check of an unknown setting:", post, isNotFound()); + post.close(); + } + private JsonObject getBodyFieldFromResponse(String rawResponse) { JsonObject response = gson.fromJson(rawResponse, JsonElement.class).getAsJsonObject(); return response.getAsJsonObject("body");