codope commented on code in PR #20151: URL: https://github.com/apache/hudi/pull/20151#discussion_r4172177828
########## hudi-utilities/src/main/java/org/apache/hudi/utilities/health/CleanerCadenceCheck.java: ########## @@ -0,0 +1,151 @@ +/* + * 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.hudi.utilities.health; + +import org.apache.hudi.avro.model.HoodieCleanMetadata; +import org.apache.hudi.common.table.timeline.HoodieActiveTimeline; +import org.apache.hudi.common.table.timeline.HoodieInstant; +import org.apache.hudi.common.table.timeline.HoodieTimeline; +import org.apache.hudi.common.util.CleanerUtils; +import org.apache.hudi.common.util.Option; +import org.apache.hudi.config.HoodieCleanConfig; + +/** + * Checks whether the cleaner is keeping pace with ingestion. + * + * <p>Cleaning is what releases storage: every commit leaves behind older file versions that stay on + * disk until a clean removes them. When the cleaner stops, the table keeps working and nothing + * errors -- storage simply grows, and with it the cost of every listing. Like the other cadence + * checks this one asks not "has a clean run recently" but whether write commits have accumulated + * well past the point where the configured trigger should have scheduled one. + * + * <p>Two allowances are made before calling the table unhealthy. The configured trigger is + * multiplied by a slack factor, since async cleaning trails ingestion by design. And a floor is + * applied on top, because the trigger defaults to one commit: without a floor, a cleaner that is + * two commits behind -- routine when cleaning is asynchronous or briefly loses a lock to a + * concurrent writer -- would be reported as a failure. + */ +public class CleanerCadenceCheck implements HealthCheck { + + static final String NAME = "cleaner"; + + /** Multiplier on the configured trigger, absorbing async cleaning running on its own cadence. */ + static final double TRIGGER_SLACK_FACTOR = 2.0; + + /** + * Minimum number of commits since the last clean before the table can be called unhealthy, + * regardless of how low the trigger is set. Keeps a trigger of one commit from turning ordinary + * async lag into an alert. + */ + static final int MIN_UNHEALTHY_COMMITS = 5; + + @Override + public String getName() { + return NAME; + } + + @Override + public String getDescription() { + return "Whether the cleaner is keeping pace with write commits"; + } + + @Override + public HealthCheckResult check(HealthCheckContext context) throws Exception { + Option<String> missing = context.firstMissingConfig(HoodieCleanConfig.CLEAN_TRIGGER_MAX_COMMITS.key()); + if (missing.isPresent()) { + return context.skipForMissingConfig(NAME, missing.get()); + } + + int triggerCommits = Integer.parseInt(context.getProps().getString( + HoodieCleanConfig.CLEAN_TRIGGER_MAX_COMMITS.key(), + HoodieCleanConfig.CLEAN_TRIGGER_MAX_COMMITS.defaultValue())); + int threshold = Math.max((int) Math.ceil(TRIGGER_SLACK_FACTOR * triggerCommits), MIN_UNHEALTHY_COMMITS); + + HoodieActiveTimeline activeTimeline = context.getMetaClient().getActiveTimeline(); + HoodieTimeline completedWrites = activeTimeline.getCommitsTimeline().filterCompletedInstants(); + Option<HoodieInstant> lastClean = activeTimeline.getCleanerTimeline().filterCompletedInstants().lastInstant(); + + HealthCheckResult.Builder result = HealthCheckResult.builder(NAME) + .config(HoodieCleanConfig.CLEAN_TRIGGER_MAX_COMMITS.key(), triggerCommits) + .config("effective.unhealthy.threshold.commits", threshold) + .config("observed.completed.write.commits", completedWrites.countInstants()); + echoIfSupplied(context, result, HoodieCleanConfig.CLEANER_POLICY.key()); + echoIfSupplied(context, result, HoodieCleanConfig.CLEANER_COMMITS_RETAINED.key()); + + int commitsSinceClean; + if (lastClean.isPresent()) { + commitsSinceClean = completedWrites.findInstantsAfter(lastClean.get().requestedTime()).countInstants(); + result.config("observed.last.clean.instant", lastClean.get().requestedTime()); + describeLastClean(context, lastClean.get(), result); + } else { + commitsSinceClean = completedWrites.countInstants(); + result.config("observed.last.clean.instant", "none"); + } + result.config("observed.commits.since.last.clean", commitsSinceClean); + + if (commitsSinceClean <= threshold) { + return result.status(HealthStatus.HEALTHY) + .summary(lastClean.isPresent() + ? String.format("%d commit(s) since clean at %s, within the threshold of %d", + commitsSinceClean, lastClean.get().requestedTime(), threshold) + : String.format("%d commit(s) and no clean yet, within the threshold of %d", + commitsSinceClean, threshold)) + .build(); + } + + result.status(HealthStatus.UNHEALTHY) + .summary(lastClean.isPresent() + ? String.format("%d commit(s) have accumulated since clean at %s, past the threshold of %d", + commitsSinceClean, lastClean.get().requestedTime(), threshold) + : String.format("%d commit(s) have accumulated and the table has never been cleaned, past the threshold of %d", + commitsSinceClean, threshold)) + .finding(String.format( + "The cleaner is configured to run every %d commit(s); %d have landed without one completing.", + triggerCommits, commitsSinceClean)); + + if (activeTimeline.getCleanerTimeline().filterInflightsAndRequested().countInstants() > 0) { + result.finding("A clean is scheduled or in flight but has not completed. Look for a failing or " + + "stuck clean in the writer logs; a clean that repeatedly fails to acquire the table lock " + + "under concurrent writers is a common cause."); + } else { + result.finding("No clean is scheduled. Check that cleaning is enabled for this table (" Review Comment: flag keep.min.commits <= clean.commits.retained directly. With that setup, archival trims the timeline before the cleaner ever has enough commits, so the cleaner never runs. I got 60 file versions and zero cleans, and this finding blamed hoodie.clean.automatic instead. ########## hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieTableHealthChecker.java: ########## @@ -0,0 +1,327 @@ +/* + * 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.hudi.utilities; + +import org.apache.hudi.common.config.TypedProperties; +import org.apache.hudi.common.table.HoodieTableMetaClient; +import org.apache.hudi.common.util.StringUtils; +import org.apache.hudi.hadoop.fs.HadoopFSUtils; +import org.apache.hudi.utilities.health.ArchivalCadenceCheck; +import org.apache.hudi.utilities.health.CleanerCadenceCheck; +import org.apache.hudi.utilities.health.CompactionCadenceCheck; +import org.apache.hudi.utilities.health.HealthCheck; +import org.apache.hudi.utilities.health.HealthCheckContext; +import org.apache.hudi.utilities.health.HealthCheckResult; +import org.apache.hudi.utilities.health.HealthStatus; +import org.apache.hudi.utilities.health.SavepointArchivalBlockCheck; + +import com.beust.jcommander.JCommander; +import com.beust.jcommander.Parameter; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.ObjectNode; +import lombok.extern.slf4j.Slf4j; +import org.apache.hadoop.conf.Configuration; + +import java.io.Serializable; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; + +/** + * Reports whether a Hudi table's table services are keeping up with ingestion. + * + * <p>Intended to be run on a schedule -- daily is typical -- against tables an operator is + * responsible for. Each check answers one question about whether a service is progressing as + * expected, and the process exit code makes the overall verdict usable from a cron job or CI + * pipeline without parsing the output. + * + * <p>This tool is strictly read-only. It inspects timeline state and reports; it never schedules or + * runs a table service, and never writes to the table. + * + * <h3>Writer properties are required</h3> + * + * <p>Nearly every question this tool answers is relative to how the table is written. Whether + * fifty delta commits since the last compaction is fine or alarming depends on the configured + * trigger; whether a thousand instants in the active timeline is fine depends on the configured + * retention. Hudi has a default for each of these, so the tool could always produce an answer -- + * but for a table written with non-default settings that answer would be wrong, and wrong in the + * direction that erodes trust, reporting healthy tables as unhealthy. + * + * <p>So by default a check whose governing property was not supplied reports {@code SKIPPED} and + * names the property, rather than guessing. Supply properties with {@code --props} or + * {@code --hoodie-conf}, or pass {@code --apply-all-defaults} to explicitly accept Hudi defaults + * for anything unset. + * + * <h3>Exit codes</h3> + * <ul> + * <li>{@code 0} -- every check that ran reported healthy</li> + * <li>{@code 1} -- at least one check reported unhealthy</li> + * <li>{@code 2} -- bad arguments, or the table could not be read</li> + * </ul> + * + * <h3>Example</h3> + * <pre> + * spark-submit --master local \ + * --class org.apache.hudi.utilities.HoodieTableHealthChecker \ + * $HUDI_DIR/packaging/hudi-utilities-bundle/target/hudi-utilities-bundle_2.12-1.3.0-SNAPSHOT.jar \ + * --base-path s3://bucket/path/to/table \ + * --props s3://bucket/path/to/writer.properties \ + * --output JSON + * </pre> + */ +@Slf4j +public class HoodieTableHealthChecker implements Serializable { + + private static final long serialVersionUID = 1L; + + static final int EXIT_HEALTHY = 0; + static final int EXIT_UNHEALTHY = 1; Review Comment: Exit 1 is also what the JVM returns when the class fails to load. The documented java -cp <bundle> command hits exactly that (NoClassDefFoundError: org/slf4j/LoggerFactory), so a broken setup reads as UNHEALTHY. Could UNHEALTHY use a different code, for example 3? ########## hudi-utilities/src/main/java/org/apache/hudi/utilities/health/ArchivalCadenceCheck.java: ########## @@ -0,0 +1,127 @@ +/* + * 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.hudi.utilities.health; + +import org.apache.hudi.common.table.timeline.HoodieActiveTimeline; +import org.apache.hudi.common.util.Option; +import org.apache.hudi.config.HoodieArchivalConfig; + +/** + * Checks whether archival is keeping the active timeline bounded. + * + * <p>Archival moves old instants out of the active timeline into the archived timeline. When it + * stops, the active timeline grows without limit, and because it is read and parsed on essentially + * every table operation, everything touching the table gets progressively slower. Unlike a failed + * write this produces no error -- the table simply degrades. + * + * <p>Two independent signals are reported. The first compares the active timeline against the + * configured retention, with slack: archival runs periodically rather than on every commit, so a + * timeline slightly over the configured maximum is normal. The second is an absolute ceiling, + * because past a few thousand instants timeline loading is a problem regardless of what the + * configuration permits -- a table configured to retain a very large number of commits is + * misconfigured rather than healthy. + * + * <p>This check deliberately does not attribute the cause. A blocked archival is most often a + * savepoint, which {@link SavepointArchivalBlockCheck} diagnoses specifically; the two are meant to + * be read together. + */ +public class ArchivalCadenceCheck implements HealthCheck { + + static final String NAME = "archival"; + + /** + * How far past the configured retention the active timeline may grow before being called + * unhealthy. Archival is periodic, so the timeline routinely sits somewhat above the maximum + * between runs. + */ + static final double RETENTION_SLACK_FACTOR = 1.1; + + /** + * Active timeline size past which the table is called unhealthy regardless of configuration. + * Beyond roughly this many instants, loading and parsing the timeline is itself a performance + * problem on every operation against the table. + */ + static final int MAX_REASONABLE_ACTIVE_INSTANTS = 5000; + + @Override + public String getName() { + return NAME; + } + + @Override + public String getDescription() { + return "Whether archival is keeping the active timeline bounded"; + } + + @Override + public HealthCheckResult check(HealthCheckContext context) { + Option<String> missing = context.firstMissingConfig(HoodieArchivalConfig.MAX_COMMITS_TO_KEEP.key()); + if (missing.isPresent()) { + return context.skipForMissingConfig(NAME, missing.get()); + } + + int maxCommitsToKeep = Integer.parseInt(context.getProps().getString( + HoodieArchivalConfig.MAX_COMMITS_TO_KEEP.key(), + HoodieArchivalConfig.MAX_COMMITS_TO_KEEP.defaultValue())); + + HoodieActiveTimeline activeTimeline = context.getMetaClient().getActiveTimeline(); + int completedInstants = activeTimeline.filterCompletedInstants().countInstants(); + int writeInstants = activeTimeline.getWriteTimeline().countInstants(); + int retentionThreshold = (int) Math.ceil(RETENTION_SLACK_FACTOR * maxCommitsToKeep); Review Comment: The archiver can keep more than keep.max.commits by design, because ArchivalUtils.getMinAndMaxInstantsToKeep raises the bounds to cover the cleaner's retention. With KEEP_LATEST_BY_HOURS (1h) and keep.max.commits=30, 50 commits came out UNHEALTHY and the finding pointed at savepoints. Could the threshold reuse getMinAndMaxInstantsToKeep? ########## hudi-utilities/src/main/java/org/apache/hudi/utilities/HoodieTableHealthChecker.md: ########## @@ -0,0 +1,218 @@ +<!-- +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. +--> + +# HoodieTableHealthChecker + +Reports whether a Hudi table's table services are keeping up with ingestion. Meant to be run on a +schedule — daily is typical — against the tables you are responsible for, with the exit code wired +into whatever alerts you. + +The tool is strictly read-only. It inspects timeline state and reports; it never schedules or runs +a table service, and never writes to the table. + +## Quick start + +```bash +spark-submit \ + --master local \ + --class org.apache.hudi.utilities.HoodieTableHealthChecker \ + $HUDI_DIR/packaging/hudi-utilities-bundle/target/hudi-utilities-bundle_2.12-1.3.0-SNAPSHOT.jar \ + --base-path s3://bucket/path/to/table \ + --props s3://bucket/path/to/writer.properties +``` + +The checks currently shipped are timeline-only, so no Spark session is actually required. Running +it as a plain Java process works too, and is cheaper: + +```bash +java -cp $HUDI_DIR/packaging/hudi-utilities-bundle/target/hudi-utilities-bundle_2.12-1.3.0-SNAPSHOT.jar \ Review Comment: This command can't start, because the bundle doesn't include slf4j, Hadoop or Jackson. spark-submit --class org.apache.hudi.utilities.HoodieTableHealthChecker <bundle> ... works. Also, can we rename this file to follow the convention for markdown files ########## hudi-utilities/src/main/java/org/apache/hudi/utilities/health/HealthCheckContext.java: ########## @@ -0,0 +1,110 @@ +/* + * 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.hudi.utilities.health; + +import org.apache.hudi.common.config.TypedProperties; +import org.apache.hudi.common.table.HoodieTableMetaClient; +import org.apache.hudi.common.util.Option; + +/** + * Everything a {@link HealthCheck} is given: the table, the writer properties it is judged against, + * and whether unset properties may fall back to Hudi defaults. + * + * <p>The distinction that {@code applyAllDefaults} draws is the point of this class. A health verdict + * is derived by comparing observed table state against what the writer configuration says should + * happen -- how many delta commits before compaction triggers, how many commits the cleaner retains, + * how many instants archival keeps. Hudi supplies a default for every one of those, so a checker can + * always produce an answer. But when the table is actually written with different settings, that + * answer is wrong, and wrong in the worst direction: a table that is perfectly healthy under its own + * configuration gets reported as UNHEALTHY, and an operator learns to distrust the tool. + * + * <p>So by default a check whose governing property was not supplied reports + * {@link HealthStatus#SKIPPED} rather than guessing. Operators who know their table runs on stock + * settings, or who just want a rough look, opt into defaults explicitly with + * {@code --apply-all-defaults}. + */ +public class HealthCheckContext { + + private final HoodieTableMetaClient metaClient; + private final TypedProperties props; + private final boolean applyAllDefaults; + + public HealthCheckContext(HoodieTableMetaClient metaClient, TypedProperties props, boolean applyAllDefaults) { + this.metaClient = metaClient; + this.props = props; + this.applyAllDefaults = applyAllDefaults; + } + + public HoodieTableMetaClient getMetaClient() { + return metaClient; + } + + public TypedProperties getProps() { + return props; + } + + public boolean isApplyAllDefaults() { + return applyAllDefaults; + } + + /** + * Whether every one of {@code keys} was supplied by the operator, or defaults were explicitly + * allowed. Checks call this before reading configuration so that a missing key becomes a + * {@link HealthStatus#SKIPPED} verdict instead of a verdict derived from a value the table may + * never have been written with. + */ + public boolean hasConfigsOrDefaultsAllowed(String... keys) { + if (applyAllDefaults) { + return true; + } + for (String key : keys) { + if (!props.containsKey(key)) { + return false; + } + } + return true; + } + + /** + * The first of {@code keys} that the operator did not supply, for naming in a skip reason. + * Empty when all are present or defaults are allowed. + */ + public Option<String> firstMissingConfig(String... keys) { + if (applyAllDefaults) { + return Option.empty(); + } + for (String key : keys) { + if (!props.containsKey(key)) { Review Comment: Real writer props files usually leave out keys that are at their default, so most checks report SKIPPED. Since an all-skipped run exits 0, a cron job stays green while checking nothing. Could a supplied --props file count as the full config, so a missing key means the default? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
