You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "nsivabalan (via GitHub)" <gi...@apache.org> on 2026/09/30 03:26:52 UTC
[PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
nsivabalan opened a new pull request, #20151:
URL: https://github.com/apache/hudi/pull/20151
### Describe the issue this Pull Request addresses
Part of #19262 (Hudi-aware agent skills and tools), under the Agentic Lakehouse umbrella #19256.
See https://github.com/apache/hudi/issues/19262#issuecomment-5750570015 for the overall plan this
is the first PR of.
Operators managing Hudi tables have no single way to answer "is this table progressing as
expected?" The individual signals exist across the timeline, but composing them means knowing
exactly which APIs to call and what the writer's configuration implies. There is no tool that
takes a table and a set of writer properties and says whether its table services are keeping up.
This is also a prerequisite for the agent-facing work in #19262/#19263: everything the agent
gateway can see today goes through Trino SQL over the catalog, and SQL cannot see timeline state,
file-slice layout, or metadata-table consistency. An agent asked "is this table healthy?" currently
has nothing factual to stand on.
### Summary and Changelog
Adds `HoodieTableHealthChecker`, a read-only tool that reports whether a table's services are
keeping up with ingestion. Intended for a scheduled run — daily is typical — with the exit code
wired into alerting.
This first PR establishes the framework, the four table-service checks (all timeline-only), and a
Claude Code skill that interprets the report. Subsequent PRs add metadata-table checks
(filesystem-vs-metadata sync, metadata compaction lag, record-level-index sizing) on the same
framework, and a separate layout analyzer.
**Framework** (`org.apache.hudi.utilities.health`)
- `HealthCheck` — interface for one diagnosis; `getName()` makes each check selectable from the
command line.
- `HealthStatus` — `SKIPPED < HEALTHY < UNHEALTHY`, ordered so a table-level verdict is a
`reduce(max)` over the individual outcomes.
- `HealthCheckResult` — verdict, one-line summary, findings, and the effective configuration the
check reasoned with.
- `HealthCheckContext` — the table, the writer properties, and whether defaults may be applied;
holds the fail-fast helpers so each check needs three lines of boilerplate.
**Checks**
- `CompactionCadenceCheck` — computes how many delta commits the configured trigger strategy allows
before compaction should be scheduled, and compares against how many accumulated since the last
completed compaction. Applies a 2.0x slack factor, because scheduling and execution are separate
and async compaction routinely trails by a cycle. Skipped on Copy-on-Write. When unhealthy,
distinguishes "nothing scheduled" from "scheduled but never completed".
- `CleanerCadenceCheck` — write commits since the last completed clean against the configured
trigger, with 2.0x slack and a floor of 5 commits (the trigger defaults to one commit, and without
a floor ordinary async lag would alert). Distinguishes "no clean scheduled" from "clean stuck".
- `SavepointArchivalBlockCheck` — archival stops at the earliest savepoint, so a forgotten savepoint
pins the active timeline indefinitely. Reports savepoints sitting behind the configured archival
retention, and separately flags any savepoint older than 7 days as likely forgotten.
- `ArchivalCadenceCheck` — active timeline against configured retention with 1.1x slack, plus an
absolute ceiling of 5000 completed instants past which timeline loading is itself a problem
regardless of configuration. Points at the savepoint check rather than guessing at a cause.
**Driver** — `HoodieTableHealthChecker` with `--base-path`, `--checks`, `--props` / `--hoodie-conf`,
`--apply-all-defaults`, `--output TABLE|JSON`, `--verbose`. Exit codes: `0` healthy, `1` unhealthy,
`2` error.
**Docs** — `HoodieTableHealthChecker.md` runbook: invocation, per-check explanation with governing
properties, JSON schema, a cron example, and caveats.
**Skill** — `hudi-agent-gateway/skills/hudi-table-health/`, following the layout of
`hudi-architect` (#19380): `SKILL.md` drives collecting the base path and writer properties,
running the tool with `--output JSON`, and turning each check's status and findings into a
diagnosis and a next step; `references/checks.md` documents the JSON envelope and per-check
properties and remedies. The skill diagnoses only — it never runs a table service or deletes a
savepoint — and never passes `--apply-all-defaults` without the user choosing it.
The JSON envelope is designed for this: per check, `status`, a one-line `summary`, `findings[]`
written as self-contained actionable sentences, and `effectiveConfigs{}` where `observed.*` keys
are measurements, `effective.*` are derived thresholds, and bare `hoodie.*` keys are the supplied
properties.
No code was copied from elsewhere.
#### Writer properties, and why there is no silent default
Nearly every question here is relative to how the table is written: whether fifty delta commits
since the last compaction is fine 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, so
the tool *could* always produce an answer — but for a table written with non-default settings that
answer is wrong in the worst direction, reporting healthy tables as unhealthy until operators stop
trusting the tool.
So a check whose governing property was not supplied reports `SKIPPED` and names the property,
rather than guessing. `--apply-all-defaults` is an explicit opt-in to evaluating against Hudi
defaults. Every check echoes the configuration it used, so a verdict can be audited against the
values that produced it.
A run where everything skipped exits `0` — a skipped check is silent about health, not an
assertion of it.
### Impact
Purely additive. New class and new package in `hudi-utilities`; nothing existing changes behavior.
- No public API changes. No `@PublicAPIClass` / `@PublicAPIMethod` touched.
- No storage format changes. The tool is strictly read-only — it never schedules or runs a table
service, and never writes to the table.
- **No new `hoodie.*` configs.** Thresholds ship as documented constants rather than configuration,
to avoid adding tuning surface for a diagnostic tool.
- The four checks in this PR are timeline-only, so the tool runs as a plain `java -cp` process; no
Spark session is required.
### Risk Level
none
Read-only tool on a new code path, reachable only by explicit invocation. No existing call site
reaches any of this.
### Documentation Update
`HoodieTableHealthChecker.md` is added alongside the tool. No new configs, so no config
documentation changes are needed. Happy to add a website page if reviewers would like one.
### Contributor's checklist
- [x] Read through [contributor's guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope merged PR #20151:
URL: https://github.com/apache/hudi/pull/20151
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20151:
URL: https://github.com/apache/hudi/pull/20151#issuecomment-5904961126
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5ad40f3270f4ef9c0a3852c5681d06b366691f06",
"status" : "SUCCESS",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17374",
"triggerID" : "5ad40f3270f4ef9c0a3852c5681d06b366691f06",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5ad40f3270f4ef9c0a3852c5681d06b366691f06 Azure: [SUCCESS](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17374)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "hudi-agent (via GitHub)" <gi...@apache.org>.
hudi-agent commented on code in PR #20151:
URL: https://github.com/apache/hudi/pull/20151#discussion_r4140597632
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/health/CompactionCadenceCheck.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * 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.model.HoodieTableType;
+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.table.timeline.TimelineUtils;
+import org.apache.hudi.common.util.CompactionUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.table.action.compact.CompactionTriggerStrategy;
+
+import java.sql.Date;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+
+/**
+ * Checks whether compaction is keeping pace with ingestion on a Merge-on-Read table.
+ *
+ * <p>The question is not "has compaction run recently" -- a quiet table legitimately has no recent
+ * compaction. It is whether delta commits have accumulated well past the point where the configured
+ * trigger should have fired. So the check computes how many delta commits the writer's trigger
+ * strategy says should elapse before compaction is scheduled, then compares that against how many
+ * have actually accumulated since the last completed compaction.
+ *
+ * <p>A slack factor is applied before calling the table unhealthy. Compaction scheduling and
+ * compaction execution are separate: a plan can be scheduled promptly while the execution strategy
+ * defers individual file slices, and async compaction can trail ingestion by a cycle or two without
+ * anything being wrong. Alerting at exactly the trigger threshold would fire on every healthy MOR
+ * table. The factor below is deliberately generous -- this check is meant to catch compaction that
+ * has stopped or fallen badly behind, not to police scheduling jitter.
+ */
+public class CompactionCadenceCheck implements HealthCheck {
+
+ static final String NAME = "compaction";
+
+ /**
+ * How far past the configured trigger point delta commits may accumulate before the table is
+ * called unhealthy. Absorbs the gap between scheduling and execution, and async table services
+ * running on their own cadence.
+ */
+ static final double TRIGGER_SLACK_FACTOR = 2.0;
+
+ @Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
+ public String getDescription() {
+ return "Whether compaction is keeping pace with delta commits on a Merge-on-Read table";
+ }
+
+ @Override
+ public boolean appliesTo(HealthCheckContext context) {
+ return context.getMetaClient().getTableType() == HoodieTableType.MERGE_ON_READ;
+ }
+
+ @Override
+ public HealthCheckResult check(HealthCheckContext context) {
+ if (!appliesTo(context)) {
+ return HealthCheckResult.skipped(NAME, "Table is Copy-on-Write, which has no compaction");
+ }
+
+ Option<String> missing = context.firstMissingConfig(
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key());
+ if (missing.isPresent()) {
+ return context.skipForMissingConfig(NAME, missing.get());
+ }
+
+ CompactionTriggerStrategy strategy = CompactionTriggerStrategy.valueOf(
+ context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.defaultValue()));
+ int maxDeltaCommits = Integer.parseInt(context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.defaultValue()));
+ int maxDeltaSeconds = Integer.parseInt(context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.defaultValue()));
+
+ HoodieActiveTimeline activeTimeline = context.getMetaClient().getActiveTimeline();
+ int expectedBeforeTrigger = expectedDeltaCommitsBeforeTrigger(
+ strategy, maxDeltaCommits, maxDeltaSeconds, activeTimeline);
+ int threshold = (int) Math.ceil(TRIGGER_SLACK_FACTOR * expectedBeforeTrigger);
+
+ Option<Pair<HoodieTimeline, HoodieInstant>> sinceLastCompaction =
+ CompactionUtils.getCompletedDeltaCommitsSinceLatestCompaction(activeTimeline);
+
+ HealthCheckResult.Builder result = HealthCheckResult.builder(NAME)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(), strategy)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key(), maxDeltaCommits)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.key(), maxDeltaSeconds)
+ .config("effective.unhealthy.threshold.delta.commits", threshold);
+
+ if (!sinceLastCompaction.isPresent()) {
+ return result.status(HealthStatus.HEALTHY)
+ .summary("No delta commits have landed yet; nothing for compaction to fall behind on")
+ .build();
+ }
+
+ int pendingDeltaCommits = sinceLastCompaction.get().getLeft().countInstants();
+ int pendingCompactions = CompactionUtils.getPendingCompactionInstantTimes(context.getMetaClient()).size();
+ HoodieInstant lastCompaction = sinceLastCompaction.get().getRight();
+
+ result.config("observed.delta.commits.since.last.compaction", pendingDeltaCommits)
+ .config("observed.pending.compactions", pendingCompactions)
+ .config("observed.last.compaction.instant", lastCompaction.requestedTime());
+
+ if (pendingDeltaCommits <= threshold) {
+ return result.status(HealthStatus.HEALTHY)
+ .summary(String.format(
+ "%d delta commit(s) since compaction at %s, within the threshold of %d",
+ pendingDeltaCommits, lastCompaction.requestedTime(), threshold))
+ .build();
+ }
+
+ result.status(HealthStatus.UNHEALTHY)
+ .summary(String.format(
+ "%d delta commit(s) have accumulated since compaction at %s, past the threshold of %d",
+ pendingDeltaCommits, lastCompaction.requestedTime(), threshold))
+ .finding(String.format(
+ "Trigger strategy %s expects compaction roughly every %d delta commit(s); %d have accumulated.",
+ strategy, expectedBeforeTrigger, pendingDeltaCommits));
+
+ if (pendingCompactions > 0) {
+ result.finding(String.format(
+ "%d compaction(s) are already scheduled but not complete -- compaction is being scheduled "
+ + "and not executed. Check that an async compactor is running and not failing.",
+ pendingCompactions));
+ } else {
+ result.finding("No compaction is scheduled. Check that compaction is enabled for this table "
+ + "and that the scheduling job is running.");
+ }
+
+ return result.build();
+ }
+
+ /**
+ * Number of delta commits the configured trigger strategy allows before compaction should be
+ * scheduled. For the time-based strategies this is expressed in delta commits too, by counting
+ * how many have landed inside the configured time window, so that the caller has a single unit
+ * to compare against.
+ */
+ private int expectedDeltaCommitsBeforeTrigger(CompactionTriggerStrategy strategy,
+ int maxDeltaCommits,
+ int maxDeltaSeconds,
+ HoodieActiveTimeline activeTimeline) {
+ HoodieTimeline completedDeltaCommits = activeTimeline.getDeltaCommitTimeline().filterCompletedInstants();
+ String windowStart = TimelineUtils.formatDate(
+ Date.from(Instant.now().minus(maxDeltaSeconds, ChronoUnit.SECONDS)));
+ int deltaCommitsInTimeWindow = completedDeltaCommits.findInstantsAfter(windowStart).countInstants();
+
+ switch (strategy) {
+ case NUM_COMMITS:
+ return maxDeltaCommits;
+
+ case NUM_COMMITS_AFTER_LAST_REQUEST:
+ // The budget restarts from the last compaction *request*, so delta commits that landed
+ // between the last completed compaction and that request are already spoken for.
+ Option<HoodieInstant> lastCompleted = activeTimeline.getCommitTimeline().filterCompletedInstants().lastInstant();
+ Option<HoodieInstant> lastRequested = activeTimeline.filterPendingCompactionTimeline().lastInstant();
+ if (lastCompleted.isPresent() && lastRequested.isPresent()) {
+ int between = completedDeltaCommits
+ .findInstantsInRange(lastCompleted.get().requestedTime(), lastRequested.get().requestedTime())
+ .countInstants();
+ return between + maxDeltaCommits;
+ }
+ return maxDeltaCommits;
+
+ case TIME_ELAPSED:
+ return deltaCommitsInTimeWindow;
Review Comment:
🤖 For TIME_ELAPSED (and NUM_OR_TIME through the `min` on L196), if no delta commit landed in the last `max.delta.seconds` (1h by default), the expected count is 0. The threshold is then 0 and a single pending delta commit reads UNHEALTHY, so a daily batch MOR table checked a few hours after its run would be flagged every day. Could this mirror the scheduler instead? For example, flag when `latestDeltaCommit.requestedTime - lastCompaction.requestedTime` is more than the slack factor times `max.delta.seconds`.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
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()) {
Review Comment:
🤖 By default Hudi doesn't write a clean instant when the plan is empty (`hoodie.write.empty.clean.interval.hours` defaults to -1). An append-only or mostly-insert table (bulk_insert, or updates rarer than every 5 commits) can go dozens of commits without a completed clean while the cleaner runs fine, and this check would call it UNHEALTHY with "No clean is scheduled". Is there a way to tell "cleaner not running" apart from "nothing to clean"? Even just mentioning empty cleans in the finding would help.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/health/CompactionCadenceCheck.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * 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.model.HoodieTableType;
+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.table.timeline.TimelineUtils;
+import org.apache.hudi.common.util.CompactionUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.table.action.compact.CompactionTriggerStrategy;
+
+import java.sql.Date;
Review Comment:
🤖 nit: this class otherwise uses `java.time.Instant`/`ChronoUnit` for date math — using `java.sql.Date` here (via `TimelineUtils.formatDate`) mixes two date APIs in the same file and could confuse a future reader about which one to use for new date logic.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/health/CompactionCadenceCheck.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * 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.model.HoodieTableType;
+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.table.timeline.TimelineUtils;
+import org.apache.hudi.common.util.CompactionUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.table.action.compact.CompactionTriggerStrategy;
+
+import java.sql.Date;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+
+/**
+ * Checks whether compaction is keeping pace with ingestion on a Merge-on-Read table.
+ *
+ * <p>The question is not "has compaction run recently" -- a quiet table legitimately has no recent
+ * compaction. It is whether delta commits have accumulated well past the point where the configured
+ * trigger should have fired. So the check computes how many delta commits the writer's trigger
+ * strategy says should elapse before compaction is scheduled, then compares that against how many
+ * have actually accumulated since the last completed compaction.
+ *
+ * <p>A slack factor is applied before calling the table unhealthy. Compaction scheduling and
+ * compaction execution are separate: a plan can be scheduled promptly while the execution strategy
+ * defers individual file slices, and async compaction can trail ingestion by a cycle or two without
+ * anything being wrong. Alerting at exactly the trigger threshold would fire on every healthy MOR
+ * table. The factor below is deliberately generous -- this check is meant to catch compaction that
+ * has stopped or fallen badly behind, not to police scheduling jitter.
+ */
+public class CompactionCadenceCheck implements HealthCheck {
+
+ static final String NAME = "compaction";
+
+ /**
+ * How far past the configured trigger point delta commits may accumulate before the table is
+ * called unhealthy. Absorbs the gap between scheduling and execution, and async table services
+ * running on their own cadence.
+ */
+ static final double TRIGGER_SLACK_FACTOR = 2.0;
+
+ @Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
+ public String getDescription() {
+ return "Whether compaction is keeping pace with delta commits on a Merge-on-Read table";
+ }
+
+ @Override
+ public boolean appliesTo(HealthCheckContext context) {
+ return context.getMetaClient().getTableType() == HoodieTableType.MERGE_ON_READ;
+ }
+
+ @Override
+ public HealthCheckResult check(HealthCheckContext context) {
+ if (!appliesTo(context)) {
+ return HealthCheckResult.skipped(NAME, "Table is Copy-on-Write, which has no compaction");
+ }
+
+ Option<String> missing = context.firstMissingConfig(
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key());
+ if (missing.isPresent()) {
+ return context.skipForMissingConfig(NAME, missing.get());
+ }
+
+ CompactionTriggerStrategy strategy = CompactionTriggerStrategy.valueOf(
+ context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.defaultValue()));
+ int maxDeltaCommits = Integer.parseInt(context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.defaultValue()));
+ int maxDeltaSeconds = Integer.parseInt(context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.defaultValue()));
+
+ HoodieActiveTimeline activeTimeline = context.getMetaClient().getActiveTimeline();
+ int expectedBeforeTrigger = expectedDeltaCommitsBeforeTrigger(
+ strategy, maxDeltaCommits, maxDeltaSeconds, activeTimeline);
+ int threshold = (int) Math.ceil(TRIGGER_SLACK_FACTOR * expectedBeforeTrigger);
+
+ Option<Pair<HoodieTimeline, HoodieInstant>> sinceLastCompaction =
+ CompactionUtils.getCompletedDeltaCommitsSinceLatestCompaction(activeTimeline);
+
+ HealthCheckResult.Builder result = HealthCheckResult.builder(NAME)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(), strategy)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key(), maxDeltaCommits)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.key(), maxDeltaSeconds)
+ .config("effective.unhealthy.threshold.delta.commits", threshold);
+
+ if (!sinceLastCompaction.isPresent()) {
+ return result.status(HealthStatus.HEALTHY)
+ .summary("No delta commits have landed yet; nothing for compaction to fall behind on")
+ .build();
+ }
+
+ int pendingDeltaCommits = sinceLastCompaction.get().getLeft().countInstants();
+ int pendingCompactions = CompactionUtils.getPendingCompactionInstantTimes(context.getMetaClient()).size();
+ HoodieInstant lastCompaction = sinceLastCompaction.get().getRight();
+
+ result.config("observed.delta.commits.since.last.compaction", pendingDeltaCommits)
+ .config("observed.pending.compactions", pendingCompactions)
+ .config("observed.last.compaction.instant", lastCompaction.requestedTime());
+
+ if (pendingDeltaCommits <= threshold) {
+ return result.status(HealthStatus.HEALTHY)
+ .summary(String.format(
+ "%d delta commit(s) since compaction at %s, within the threshold of %d",
+ pendingDeltaCommits, lastCompaction.requestedTime(), threshold))
+ .build();
+ }
+
+ result.status(HealthStatus.UNHEALTHY)
+ .summary(String.format(
+ "%d delta commit(s) have accumulated since compaction at %s, past the threshold of %d",
+ pendingDeltaCommits, lastCompaction.requestedTime(), threshold))
+ .finding(String.format(
+ "Trigger strategy %s expects compaction roughly every %d delta commit(s); %d have accumulated.",
+ strategy, expectedBeforeTrigger, pendingDeltaCommits));
+
+ if (pendingCompactions > 0) {
+ result.finding(String.format(
+ "%d compaction(s) are already scheduled but not complete -- compaction is being scheduled "
+ + "and not executed. Check that an async compactor is running and not failing.",
+ pendingCompactions));
+ } else {
+ result.finding("No compaction is scheduled. Check that compaction is enabled for this table "
+ + "and that the scheduling job is running.");
+ }
+
+ return result.build();
+ }
+
+ /**
+ * Number of delta commits the configured trigger strategy allows before compaction should be
+ * scheduled. For the time-based strategies this is expressed in delta commits too, by counting
+ * how many have landed inside the configured time window, so that the caller has a single unit
+ * to compare against.
+ */
+ private int expectedDeltaCommitsBeforeTrigger(CompactionTriggerStrategy strategy,
+ int maxDeltaCommits,
+ int maxDeltaSeconds,
+ HoodieActiveTimeline activeTimeline) {
+ HoodieTimeline completedDeltaCommits = activeTimeline.getDeltaCommitTimeline().filterCompletedInstants();
+ String windowStart = TimelineUtils.formatDate(
+ Date.from(Instant.now().minus(maxDeltaSeconds, ChronoUnit.SECONDS)));
+ int deltaCommitsInTimeWindow = completedDeltaCommits.findInstantsAfter(windowStart).countInstants();
+
+ switch (strategy) {
+ case NUM_COMMITS:
+ return maxDeltaCommits;
+
+ case NUM_COMMITS_AFTER_LAST_REQUEST:
+ // The budget restarts from the last compaction *request*, so delta commits that landed
+ // between the last completed compaction and that request are already spoken for.
+ Option<HoodieInstant> lastCompleted = activeTimeline.getCommitTimeline().filterCompletedInstants().lastInstant();
+ Option<HoodieInstant> lastRequested = activeTimeline.filterPendingCompactionTimeline().lastInstant();
+ if (lastCompleted.isPresent() && lastRequested.isPresent()) {
+ int between = completedDeltaCommits
+ .findInstantsInRange(lastCompleted.get().requestedTime(), lastRequested.get().requestedTime())
+ .countInstants();
+ return between + maxDeltaCommits;
Review Comment:
🤖 With NUM_COMMITS_AFTER_LAST_REQUEST, the budget grows with the backlog. Suppose the compactor stops but the scheduler keeps adding plans (the normal pattern for this strategy, e.g. Flink). Then `between` covers nearly every delta commit since the last completed compaction, and `2 * (between + max)` always stays above the observed count. The check can never report the "scheduled but not executed" case it's meant to catch. Would it work to judge against `max` since the last completed compaction, or to flag on the number of pending plans?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
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;
+ static final int EXIT_ERROR = 2;
+
+ private static final ObjectMapper JSON_MAPPER =
+ new ObjectMapper().enable(SerializationFeature.INDENT_OUTPUT);
+
+ private final Config cfg;
+ private final TypedProperties props;
+ private final HoodieTableMetaClient metaClient;
+
+ public HoodieTableHealthChecker(Configuration hadoopConf, Config cfg) {
+ this.cfg = cfg;
+ this.props = UtilHelpers.buildProperties(hadoopConf, cfg.propsFilePath, cfg.configs);
+ this.metaClient = HoodieTableMetaClient.builder()
+ .setConf(HadoopFSUtils.getStorageConfWithCopy(hadoopConf))
+ .setBasePath(cfg.basePath)
+ .setLoadActiveTimelineOnLoad(true)
+ .build();
+ }
+
+ /** All checks this tool knows about, in report order. */
+ static List<HealthCheck> allChecks() {
+ return Arrays.asList(
+ new CompactionCadenceCheck(),
+ new CleanerCadenceCheck(),
+ new SavepointArchivalBlockCheck(),
+ new ArchivalCadenceCheck());
+ }
+
+ /**
+ * Runs the selected checks and renders the report.
+ *
+ * @return the exit code the process should terminate with.
+ */
+ public int run() {
+ List<HealthCheck> selected = selectChecks(cfg.checks);
+ HealthCheckContext context = new HealthCheckContext(metaClient, props, cfg.applyAllDefaults);
+
+ List<HealthCheckResult> results = new ArrayList<>();
+ List<String> erroredChecks = new ArrayList<>();
+
+ for (HealthCheck check : selected) {
+ try {
+ results.add(check.check(context));
+ } catch (Exception e) {
+ // One check failing to reach a verdict must not hide the others. Record it and continue.
+ log.error("Health check '{}' failed to run for table {}", check.getName(), cfg.basePath, e);
+ erroredChecks.add(check.getName() + ": " + e.getMessage());
+ }
+ }
+
+ if (OutputFormat.JSON == cfg.output) {
+ System.out.println(renderJson(results, erroredChecks));
+ } else {
+ System.out.println(renderTable(results, erroredChecks));
+ }
+
+ if (!erroredChecks.isEmpty()) {
+ return EXIT_ERROR;
+ }
+ return results.stream().anyMatch(r -> r.getStatus().isUnhealthy()) ? EXIT_UNHEALTHY : EXIT_HEALTHY;
+ }
+
+ private List<HealthCheck> selectChecks(String requested) {
+ List<HealthCheck> available = allChecks();
+ if (StringUtils.isNullOrEmpty(requested) || "all".equalsIgnoreCase(requested)) {
+ return available;
+ }
+
+ Map<String, HealthCheck> byName = new LinkedHashMap<>();
+ available.forEach(c -> byName.put(c.getName(), c));
+
+ List<HealthCheck> selected = new ArrayList<>();
+ for (String name : requested.split(",")) {
+ String trimmed = name.trim().toLowerCase();
+ if (!byName.containsKey(trimmed)) {
+ throw new IllegalArgumentException(String.format(
+ "Unknown check '%s'. Available checks: %s", trimmed, String.join(", ", byName.keySet())));
+ }
+ selected.add(byName.get(trimmed));
+ }
+ return selected;
+ }
+
+ private HealthStatus overallStatus(List<HealthCheckResult> results) {
+ return results.stream()
+ .map(HealthCheckResult::getStatus)
+ .reduce(HealthStatus.SKIPPED, HealthStatus::max);
+ }
+
+ private String renderTable(List<HealthCheckResult> results, List<String> erroredChecks) {
+ StringBuilder sb = new StringBuilder();
+ sb.append("\nHudi table health report\n");
+ sb.append(" table : ").append(cfg.basePath).append('\n');
+ sb.append(" type : ").append(metaClient.getTableType()).append('\n');
+ sb.append(" overall : ").append(overallStatus(results)).append("\n\n");
+
+ for (HealthCheckResult result : results) {
+ sb.append(String.format("[%-9s] %s%n", result.getStatus(), result.getCheckName()));
+ sb.append(" ").append(result.getSummary()).append('\n');
+
+ for (String finding : result.getFindings()) {
+ sb.append(" - ").append(finding).append('\n');
+ }
+
+ if (cfg.verbose && !result.getEffectiveConfigs().isEmpty()) {
+ sb.append(" config used:\n");
+ result.getEffectiveConfigs().forEach((k, v) -> sb.append(" ").append(k).append(" = ").append(v).append('\n'));
+ }
+ sb.append('\n');
+ }
+
+ if (!erroredChecks.isEmpty()) {
+ sb.append("Checks that could not run:\n");
+ erroredChecks.forEach(e -> sb.append(" - ").append(e).append('\n'));
+ }
+
+ return sb.toString();
+ }
+
+ private String renderJson(List<HealthCheckResult> results, List<String> erroredChecks) {
+ ObjectNode root = JSON_MAPPER.createObjectNode();
+ root.put("basePath", cfg.basePath);
+ root.put("tableType", metaClient.getTableType().name());
+ root.put("overallStatus", overallStatus(results).name());
+
+ ArrayNode checks = root.putArray("checks");
+ for (HealthCheckResult result : results) {
+ ObjectNode node = checks.addObject();
+ node.put("name", result.getCheckName());
+ node.put("status", result.getStatus().name());
+ node.put("summary", result.getSummary());
+
+ ArrayNode findings = node.putArray("findings");
+ result.getFindings().forEach(findings::add);
+
+ ObjectNode configs = node.putObject("effectiveConfigs");
+ result.getEffectiveConfigs().forEach(configs::put);
+ }
+
+ ArrayNode errors = root.putArray("errors");
+ erroredChecks.forEach(errors::add);
+
+ try {
+ return JSON_MAPPER.writeValueAsString(root);
+ } catch (Exception e) {
+ throw new IllegalStateException("Failed to render health report as JSON", e);
+ }
+ }
+
+ public enum OutputFormat {
+ TABLE, JSON
+ }
+
+ public static class Config implements Serializable {
+
+ private static final long serialVersionUID = 1L;
+
+ @Parameter(names = {"--base-path", "-sp"}, description = "Base path of the Hudi table to check", required = true)
+ public String basePath = null;
+
+ @Parameter(names = {"--checks"}, description = "Comma-separated checks to run, or 'all'. Available: "
+ + "compaction, cleaner, savepoint, archival")
+ public String checks = "all";
+
+ @Parameter(names = {"--apply-all-defaults"}, description = "Evaluate checks against Hudi default values for any "
+ + "writer property not supplied via --props or --hoodie-conf. Off by default: without the writer properties "
+ + "the table was actually written with, a verdict derived from defaults can report a healthy table as "
+ + "unhealthy, so such checks are skipped instead.")
+ public Boolean applyAllDefaults = false;
+
+ @Parameter(names = {"--output"}, description = "Output format: TABLE or JSON")
+ public OutputFormat output = OutputFormat.TABLE;
+
+ @Parameter(names = {"--verbose", "-v"}, description = "Include the effective configuration each check used")
+ public Boolean verbose = false;
+
+ @Parameter(names = {"--props"}, description = "Path to a properties file on local fs or dfs, holding the writer "
+ + "configuration this table is written with")
+ public String propsFilePath = null;
+
+ @Parameter(names = {"--hoodie-conf"}, description = "Any writer configuration that can be set in the properties "
+ + "file (using --props) can also be passed on the command line here. This can be repeated",
+ splitter = IdentitySplitter.class)
+ public List<String> configs = new ArrayList<>();
+
+ @Parameter(names = {"--help", "-h"}, help = true)
+ public Boolean help = false;
+
+ @Override
+ public String toString() {
+ return "HoodieTableHealthCheckerConfig {\n"
+ + " --base-path " + basePath + ", \n"
+ + " --checks " + checks + ", \n"
+ + " --apply-all-defaults " + applyAllDefaults + ", \n"
+ + " --output " + output + ", \n"
+ + " --verbose " + verbose + ", \n"
+ + " --props " + propsFilePath + ", \n"
+ + " --hoodie-conf " + configs
+ + "\n}";
+ }
+ }
+
+ public static void main(String[] args) {
+ Config cfg = new Config();
+ JCommander cmd = new JCommander(cfg, null, args);
Review Comment:
🤖 `new JCommander(cfg, null, args)` parses and validates right here, outside the try. A missing `--base-path`, an unknown flag, or `--output XML` throws `ParameterException` out of `main`, so the JVM exits 1, which this tool uses to mean UNHEALTHY (the cron example would alert "table is unhealthy"). Could you wrap the parse and map `ParameterException` to `EXIT_ERROR`? Also, the `args.length == 0` branch can't be reached, because the required-parameter check throws first.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/health/CompactionCadenceCheck.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * 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.model.HoodieTableType;
+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.table.timeline.TimelineUtils;
+import org.apache.hudi.common.util.CompactionUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.table.action.compact.CompactionTriggerStrategy;
+
+import java.sql.Date;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+
+/**
+ * Checks whether compaction is keeping pace with ingestion on a Merge-on-Read table.
+ *
+ * <p>The question is not "has compaction run recently" -- a quiet table legitimately has no recent
+ * compaction. It is whether delta commits have accumulated well past the point where the configured
+ * trigger should have fired. So the check computes how many delta commits the writer's trigger
+ * strategy says should elapse before compaction is scheduled, then compares that against how many
+ * have actually accumulated since the last completed compaction.
+ *
+ * <p>A slack factor is applied before calling the table unhealthy. Compaction scheduling and
+ * compaction execution are separate: a plan can be scheduled promptly while the execution strategy
+ * defers individual file slices, and async compaction can trail ingestion by a cycle or two without
+ * anything being wrong. Alerting at exactly the trigger threshold would fire on every healthy MOR
+ * table. The factor below is deliberately generous -- this check is meant to catch compaction that
+ * has stopped or fallen badly behind, not to police scheduling jitter.
+ */
+public class CompactionCadenceCheck implements HealthCheck {
+
+ static final String NAME = "compaction";
+
+ /**
+ * How far past the configured trigger point delta commits may accumulate before the table is
+ * called unhealthy. Absorbs the gap between scheduling and execution, and async table services
+ * running on their own cadence.
+ */
+ static final double TRIGGER_SLACK_FACTOR = 2.0;
+
+ @Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
+ public String getDescription() {
+ return "Whether compaction is keeping pace with delta commits on a Merge-on-Read table";
+ }
+
+ @Override
+ public boolean appliesTo(HealthCheckContext context) {
+ return context.getMetaClient().getTableType() == HoodieTableType.MERGE_ON_READ;
+ }
+
+ @Override
+ public HealthCheckResult check(HealthCheckContext context) {
+ if (!appliesTo(context)) {
+ return HealthCheckResult.skipped(NAME, "Table is Copy-on-Write, which has no compaction");
+ }
+
+ Option<String> missing = context.firstMissingConfig(
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key());
+ if (missing.isPresent()) {
+ return context.skipForMissingConfig(NAME, missing.get());
+ }
+
+ CompactionTriggerStrategy strategy = CompactionTriggerStrategy.valueOf(
+ context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.defaultValue()));
+ int maxDeltaCommits = Integer.parseInt(context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.defaultValue()));
+ int maxDeltaSeconds = Integer.parseInt(context.getProps().getString(
+ HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.key(),
+ HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.defaultValue()));
+
+ HoodieActiveTimeline activeTimeline = context.getMetaClient().getActiveTimeline();
+ int expectedBeforeTrigger = expectedDeltaCommitsBeforeTrigger(
+ strategy, maxDeltaCommits, maxDeltaSeconds, activeTimeline);
+ int threshold = (int) Math.ceil(TRIGGER_SLACK_FACTOR * expectedBeforeTrigger);
+
+ Option<Pair<HoodieTimeline, HoodieInstant>> sinceLastCompaction =
+ CompactionUtils.getCompletedDeltaCommitsSinceLatestCompaction(activeTimeline);
+
+ HealthCheckResult.Builder result = HealthCheckResult.builder(NAME)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_TRIGGER_STRATEGY.key(), strategy)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_NUM_DELTA_COMMITS.key(), maxDeltaCommits)
+ .config(HoodieCompactionConfig.INLINE_COMPACT_TIME_DELTA_SECONDS.key(), maxDeltaSeconds)
+ .config("effective.unhealthy.threshold.delta.commits", threshold);
+
+ if (!sinceLastCompaction.isPresent()) {
+ return result.status(HealthStatus.HEALTHY)
+ .summary("No delta commits have landed yet; nothing for compaction to fall behind on")
+ .build();
+ }
+
+ int pendingDeltaCommits = sinceLastCompaction.get().getLeft().countInstants();
+ int pendingCompactions = CompactionUtils.getPendingCompactionInstantTimes(context.getMetaClient()).size();
+ HoodieInstant lastCompaction = sinceLastCompaction.get().getRight();
Review Comment:
🤖 When the table has never compacted, `getCompletedDeltaCommitsSinceLatestCompaction` returns the *first delta commit* as the right side. The summary then reads "N delta commit(s) since compaction at 00001", and `observed.last.compaction.instant` names a delta commit (the JSON sample in the .md shows exactly this). Since the skill quotes summaries verbatim, could you check `lastCompaction.getAction()` and say "never compacted" in that case?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/health/CompactionCadenceCheck.java:
##########
@@ -0,0 +1,206 @@
+/*
+ * 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.model.HoodieTableType;
+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.table.timeline.TimelineUtils;
+import org.apache.hudi.common.util.CompactionUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.common.util.collection.Pair;
+import org.apache.hudi.config.HoodieCompactionConfig;
+import org.apache.hudi.table.action.compact.CompactionTriggerStrategy;
+
+import java.sql.Date;
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+
+/**
+ * Checks whether compaction is keeping pace with ingestion on a Merge-on-Read table.
+ *
+ * <p>The question is not "has compaction run recently" -- a quiet table legitimately has no recent
+ * compaction. It is whether delta commits have accumulated well past the point where the configured
+ * trigger should have fired. So the check computes how many delta commits the writer's trigger
+ * strategy says should elapse before compaction is scheduled, then compares that against how many
+ * have actually accumulated since the last completed compaction.
+ *
+ * <p>A slack factor is applied before calling the table unhealthy. Compaction scheduling and
+ * compaction execution are separate: a plan can be scheduled promptly while the execution strategy
+ * defers individual file slices, and async compaction can trail ingestion by a cycle or two without
+ * anything being wrong. Alerting at exactly the trigger threshold would fire on every healthy MOR
+ * table. The factor below is deliberately generous -- this check is meant to catch compaction that
+ * has stopped or fallen badly behind, not to police scheduling jitter.
+ */
+public class CompactionCadenceCheck implements HealthCheck {
+
+ static final String NAME = "compaction";
+
+ /**
+ * How far past the configured trigger point delta commits may accumulate before the table is
+ * called unhealthy. Absorbs the gap between scheduling and execution, and async table services
+ * running on their own cadence.
+ */
+ static final double TRIGGER_SLACK_FACTOR = 2.0;
+
+ @Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
+ public String getDescription() {
+ return "Whether compaction is keeping pace with delta commits on a Merge-on-Read table";
+ }
+
+ @Override
+ public boolean appliesTo(HealthCheckContext context) {
+ return context.getMetaClient().getTableType() == HoodieTableType.MERGE_ON_READ;
+ }
+
+ @Override
+ public HealthCheckResult check(HealthCheckContext context) {
+ if (!appliesTo(context)) {
+ return HealthCheckResult.skipped(NAME, "Table is Copy-on-Write, which has no compaction");
+ }
+
+ Option<String> missing = context.firstMissingConfig(
Review Comment:
🤖 The gating here doesn't quite match the "skip rather than guess" contract. For TIME_ELAPSED, NUM_OR_TIME and NUM_AND_TIME the verdict depends on `hoodie.compact.inline.max.delta.seconds`, but that key isn't gated, so it quietly falls back to 3600. Meanwhile `trigger.strategy` is required even though most writers never set it. Could the required keys depend on the strategy?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
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:
🤖 `props.containsKey(key)` (and the later `getString(key, default)` in the checks) only look at the primary key, so values supplied under a `withAlternatives` alias are missed. For example, with `hoodie.clean.max.commits` for `CLEAN_TRIGGER_MAX_COMMITS` the check SKIPs, and with `--apply-all-defaults` it evaluates against the default of 1 even though the operator supplied a value. Could this take the `ConfigProperty` and use `ConfigUtils.containsConfigProperty` / `getStringWithAltKeys`?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
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) {
Review Comment:
🤖 nit: `hasConfigsOrDefaultsAllowed` doesn't look like it's called from any of the four `HealthCheck` implementations (they all use `firstMissingConfig` instead) — only the test exercises it. Consider dropping it if it's not needed, or wire it in if it was meant to replace duplicated logic.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
##########
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;
+ static final int EXIT_ERROR = 2;
+
+ private static final ObjectMapper JSON_MAPPER =
+ new ObjectMapper().enable(SerializationFeature.INDENT_OUTPUT);
+
+ private final Config cfg;
+ private final TypedProperties props;
+ private final HoodieTableMetaClient metaClient;
+
+ public HoodieTableHealthChecker(Configuration hadoopConf, Config cfg) {
+ this.cfg = cfg;
+ this.props = UtilHelpers.buildProperties(hadoopConf, cfg.propsFilePath, cfg.configs);
+ this.metaClient = HoodieTableMetaClient.builder()
+ .setConf(HadoopFSUtils.getStorageConfWithCopy(hadoopConf))
+ .setBasePath(cfg.basePath)
+ .setLoadActiveTimelineOnLoad(true)
+ .build();
+ }
+
+ /** All checks this tool knows about, in report order. */
+ static List<HealthCheck> allChecks() {
+ return Arrays.asList(
+ new CompactionCadenceCheck(),
+ new CleanerCadenceCheck(),
+ new SavepointArchivalBlockCheck(),
+ new ArchivalCadenceCheck());
+ }
+
+ /**
+ * Runs the selected checks and renders the report.
+ *
+ * @return the exit code the process should terminate with.
+ */
+ public int run() {
+ List<HealthCheck> selected = selectChecks(cfg.checks);
+ HealthCheckContext context = new HealthCheckContext(metaClient, props, cfg.applyAllDefaults);
+
+ List<HealthCheckResult> results = new ArrayList<>();
+ List<String> erroredChecks = new ArrayList<>();
+
+ for (HealthCheck check : selected) {
+ try {
+ results.add(check.check(context));
Review Comment:
🤖 The `HealthCheck#appliesTo` javadoc says a check that doesn't apply is reported SKIPPED rather than run, but the runner never calls it. Only `CompactionCadenceCheck` behaves correctly, because it calls `appliesTo` itself. Since follow-up PRs will add more checks on this framework, could the runner honor it here? For example: `if (!check.appliesTo(context)) { results.add(HealthCheckResult.skipped(...)); continue; }`.
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag quality.</i></sub>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #20151:
URL: https://github.com/apache/hudi/pull/20151#issuecomment-5903602300
## [Codecov](https://app.codecov.io/gh/apache/hudi/pull/20151?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 79.00%. Comparing base ([`ba26a89`](https://app.codecov.io/gh/apache/hudi/commit/ba26a891e8fc87704767606f11d2cc4eaf3a4950?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5ad40f3`](https://app.codecov.io/gh/apache/hudi/commit/5ad40f3270f4ef9c0a3852c5681d06b366691f06?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #20151 +/- ##
============================================
- Coverage 79.00% 79.00% -0.01%
Complexity 34169 34169
============================================
Files 2546 2546
Lines 142929 142931 +2
Branches 17381 17383 +2
============================================
- Hits 112922 112919 -3
- Misses 22035 22040 +5
Partials 7972 7972
```
| [Components](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hudi-common](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.31% <ø> (ø)` | |
| [hudi-client](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.34% <0.00%> (-0.02%)` | :arrow_down: |
| [hudi-flink](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.75% <ø> (ø)` | |
| [hudi-spark-datasource](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.71% <ø> (ø)` | |
| [hudi-utilities](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.15% <ø> (ø)` | |
| [hudi-cli](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.05% <ø> (ø)` | |
| [hudi-hadoop](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `69.41% <ø> (ø)` | |
| [hudi-sync](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `76.02% <ø> (ø)` | |
| [hudi-io](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.40% <ø> (ø)` | |
| [hudi-timeline-service](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.17% <ø> (ø)` | |
| [hudi-cloud](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `81.00% <ø> (ø)` | |
| [hudi-kafka-connect](https://app.codecov.io/gh/apache/hudi/pull/20151/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.20% <ø> (ø)` | |
| [Flag](https://app.codecov.io/gh/apache/hudi/pull/20151/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hadoop-mr-java-client](https://app.codecov.io/gh/apache/hudi/pull/20151/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.98% <ø> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
[see 1 file with indirect coverage changes](https://app.codecov.io/gh/apache/hudi/pull/20151/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on PR #20151:
URL: https://github.com/apache/hudi/pull/20151#issuecomment-6206033004
Heads-up @codope — I pushed one more commit after your approval (`1b3064508ba8`), so flagging it rather than letting the approval carry silently.
It adds an optional `JavaSparkContext` to `HealthCheckContext` and the driver. Nothing uses it yet; it is plumbing for two things:
- the metadata-table sync check on #20154 should delegate its listing comparison to `HoodieMetadataTableValidator` instead of reimplementing a narrower one — your v6 uncommitted-log-files finding is what surfaced that, and the validator already resolves committed-ness from the log block header, which is version-independent
- the compaction and cleaner checks walk file slices per file group and file versions per partition on the driver, which will not hold on a table with hundreds of thousands of file groups
Both existing constructors are kept, so a caller that supplies no context behaves exactly as before and the timeline-only checks stay runnable without a cluster. A check that needs a context and is not given one will report SKIPPED naming what it needed, matching the contract already used for a missing writer property.
Happy to split this into its own PR if you would rather #20151 stay as reviewed.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #20151:
URL: https://github.com/apache/hudi/pull/20151#discussion_r4279810579
##########
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;
+ static final int EXIT_ERROR = 2;
+
+ private static final ObjectMapper JSON_MAPPER =
+ new ObjectMapper().enable(SerializationFeature.INDENT_OUTPUT);
+
+ private final Config cfg;
+ private final TypedProperties props;
+ private final HoodieTableMetaClient metaClient;
+
+ public HoodieTableHealthChecker(Configuration hadoopConf, Config cfg) {
+ this.cfg = cfg;
+ this.props = UtilHelpers.buildProperties(hadoopConf, cfg.propsFilePath, cfg.configs);
+ this.metaClient = HoodieTableMetaClient.builder()
+ .setConf(HadoopFSUtils.getStorageConfWithCopy(hadoopConf))
+ .setBasePath(cfg.basePath)
+ .setLoadActiveTimelineOnLoad(true)
+ .build();
+ }
+
+ /** All checks this tool knows about, in report order. */
+ static List<HealthCheck> allChecks() {
+ return Arrays.asList(
+ new CompactionCadenceCheck(),
+ new CleanerCadenceCheck(),
+ new SavepointArchivalBlockCheck(),
+ new ArchivalCadenceCheck());
+ }
+
+ /**
+ * Runs the selected checks and renders the report.
+ *
+ * @return the exit code the process should terminate with.
+ */
+ public int run() {
+ List<HealthCheck> selected = selectChecks(cfg.checks);
+ HealthCheckContext context = new HealthCheckContext(metaClient, props, cfg.applyAllDefaults);
+
+ List<HealthCheckResult> results = new ArrayList<>();
+ List<String> erroredChecks = new ArrayList<>();
+
+ for (HealthCheck check : selected) {
+ try {
+ results.add(check.check(context));
+ } catch (Exception e) {
+ // One check failing to reach a verdict must not hide the others. Record it and continue.
+ log.error("Health check '{}' failed to run for table {}", check.getName(), cfg.basePath, e);
+ erroredChecks.add(check.getName() + ": " + e.getMessage());
+ }
+ }
+
+ if (OutputFormat.JSON == cfg.output) {
+ System.out.println(renderJson(results, erroredChecks));
+ } else {
+ System.out.println(renderTable(results, erroredChecks));
+ }
+
+ if (!erroredChecks.isEmpty()) {
+ return EXIT_ERROR;
+ }
+ return results.stream().anyMatch(r -> r.getStatus().isUnhealthy()) ? EXIT_UNHEALTHY : EXIT_HEALTHY;
Review Comment:
Agreed — UNHEALTHY now wins. The table's verdict is the more actionable of the two, and an errored check shouldn't mask it.
Both are still reported in full in the output; only the exit code is ranked. And since the codes moved anyway (see the other thread), "unhealthy + errors" has its own slot:
| Code | Meaning |
|---|---|
| `0` | Every check that ran reported healthy (or was skipped) |
| `2` | Bad arguments, or the table could not be read |
| `3` | At least one check reported unhealthy |
| `4` | No check reported unhealthy, but at least one could not run |
##########
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:
Good catch — that's a real trap, and the documented command hit it exactly.
`1` is now unused and reserved. UNHEALTHY moves to `3`, and a check that could not run gets `4`:
| Code | Meaning |
|---|---|
| `0` | Every check that ran reported healthy (or was skipped) |
| `2` | Bad arguments, or the table could not be read |
| `3` | At least one check reported unhealthy |
| `4` | No check reported unhealthy, but at least one could not run |
The class javadoc, the `--help` output and the README table all say why `1` is skipped, so nobody reintroduces it. The `java -cp` command that produced the `NoClassDefFoundError` is gone too — see the README thread.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #20151:
URL: https://github.com/apache/hudi/pull/20151#discussion_r4279813980
##########
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:
Both done.
The `java -cp` command is gone. You're right that the bundle doesn't carry slf4j, Hadoop or Jackson — the claim that it could run as a plain Java process was wrong. The text now says the checks are timeline-only so no Spark session is used for the work itself, but the bundle still needs the classes `spark-submit` puts on the classpath, so run it that way.
The cron example further down had the same `java -cp` invocation and keyed off the old exit codes, so it would have failed twice over. It now uses `spark-submit` and the new codes, including an `elif` for a non-zero code that isn't a verdict — which is exactly the case your `NoClassDefFoundError` would have produced.
Renamed `HoodieTableHealthChecker.md` to `README.md`. `README.md` is clearly the convention here (42 of them in the repo); `TimelineInspector.md` is the only other `ClassName.md` and it's mine too, so I'll rename that one in its own patch rather than leave a second instance of a convention we're moving away from.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20151:
URL: https://github.com/apache/hudi/pull/20151#issuecomment-5903586414
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5ad40f3270f4ef9c0a3852c5681d06b366691f06",
"status" : "PENDING",
"url" : "https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17374",
"triggerID" : "5ad40f3270f4ef9c0a3852c5681d06b366691f06",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5ad40f3270f4ef9c0a3852c5681d06b366691f06 Azure: [PENDING](https://dev.azure.com/apachehudi/a1a51da7-8592-47d4-88dc-fd67bed336bb/_build/results?buildId=17374)
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #20151:
URL: https://github.com/apache/hudi/pull/20151#discussion_r4163867874
##########
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);
+
+ HealthCheckResult.Builder result = HealthCheckResult.builder(NAME)
+ .config(HoodieArchivalConfig.MAX_COMMITS_TO_KEEP.key(), maxCommitsToKeep)
+ .config("effective.unhealthy.threshold.write.instants", retentionThreshold)
+ .config("effective.absolute.ceiling.completed.instants", MAX_REASONABLE_ACTIVE_INSTANTS)
+ .config("observed.write.instants", writeInstants)
+ .config("observed.completed.instants", completedInstants);
+
+ boolean overAbsoluteCeiling = completedInstants >= MAX_REASONABLE_ACTIVE_INSTANTS;
+ boolean overConfiguredRetention = writeInstants >= retentionThreshold;
Review Comment:
Looks like `archival` ignores the cleaner-driven retention the archiver actually enforces. `ArchivalUtils.getMinAndMaxInstantsToKeep` raises min and max whenever the cleaner retains more than keep.min.commits. The archiver also holds back for:
- the earliest pending write instant (TimelineArchiverV2)
- MOR compaction retention
- the MDT's latest compaction
So a supported setup like clean.commits.retained=100 with keep.max.commits=30 can be flagged UNHEALTHY.
Should we check, and model, the cleaner keys the same way `getMinAndMaxInstantsToKeep` does, and name the actual blocker (oldest pending instant, MDT compaction) in the finding? Also, `getWriteTimeline()` includes requested and inflight compaction/clustering, while the archiver counts completed commits only.
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/health/SavepointArchivalBlockCheck.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.HoodieInstant;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.TimelineUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieArchivalConfig;
+
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Checks for savepoints that are holding back archival.
+ *
+ * <p>Archival stops at the earliest savepoint and will not advance past it. A savepoint taken for a
+ * one-off restore and then forgotten therefore pins the active timeline indefinitely. The timeline
+ * grows without bound, every timeline load gets slower, and because archival is blocked the
+ * cleaner's retention interacts with it in ways that keep data files alive too. This is a quiet
+ * failure: the table keeps accepting writes and nothing errors.
+ *
+ * <p>Two signals are reported. A savepoint sitting further back than the configured archival
+ * retention is actively blocking archival now. A savepoint older than {@link #STALE_SAVEPOINT_DAYS}
+ * is flagged separately as likely forgotten, since savepoints are normally short-lived.
+ *
+ * <p>The remedy this check points at is always to release the savepoint once the operation it was
+ * taken for is done. It deliberately does not suggest loosening archival to step over savepoints.
+ */
+public class SavepointArchivalBlockCheck implements HealthCheck {
+
+ static final String NAME = "savepoint";
+
+ /**
+ * Age past which a savepoint is called out as probably forgotten. Savepoints are normally taken
+ * around a specific operation and released soon after; one lingering for weeks is usually an
+ * oversight rather than an intent.
+ */
+ static final int STALE_SAVEPOINT_DAYS = 7;
+
+ @Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
+ public String getDescription() {
+ return "Whether a savepoint is blocking archival or has been left behind";
+ }
+
+ @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()));
+ HoodieTimeline writeTimeline = context.getMetaClient().getActiveTimeline().getWriteTimeline();
+ List<HoodieInstant> savepoints = context.getMetaClient().getActiveTimeline()
+ .getSavePointTimeline().filterCompletedInstants().getInstants();
+
+ HealthCheckResult.Builder result = HealthCheckResult.builder(NAME)
+ .config(HoodieArchivalConfig.MAX_COMMITS_TO_KEEP.key(), maxCommitsToKeep)
+ .config("observed.savepoint.count", savepoints.size());
+
+ if (savepoints.isEmpty()) {
+ return result.status(HealthStatus.HEALTHY).summary("No savepoints on the table").build();
+ }
+
+ List<String> savepointTimes = savepoints.stream()
+ .map(HoodieInstant::requestedTime).collect(Collectors.toList());
+ result.config("observed.savepoints", String.join(",", savepointTimes));
+
+ List<String> blocking = savepointsBeyondRetention(writeTimeline, savepointTimes, maxCommitsToKeep);
+ List<String> stale = savepointTimes.stream()
+ .filter(SavepointArchivalBlockCheck::isOlderThanStaleThreshold)
+ .collect(Collectors.toList());
+
+ if (!stale.isEmpty()) {
+ result.finding(String.format(
+ "Savepoint(s) %s are more than %d days old. If the operation they were taken for is "
+ + "finished, delete them so archival can advance.",
+ String.join(", ", stale), STALE_SAVEPOINT_DAYS));
+ }
+
+ if (blocking.isEmpty()) {
+ return result.status(HealthStatus.HEALTHY)
+ .summary(String.format("%d savepoint(s) present, none older than the archival retention of %d commits",
+ savepoints.size(), maxCommitsToKeep))
+ .build();
+ }
+
+ return result.status(HealthStatus.UNHEALTHY)
+ .summary(String.format(
+ "%d savepoint(s) older than the archival retention are blocking archival: %s",
+ blocking.size(), String.join(", ", blocking)))
+ .finding(String.format(
+ "Archival stops at the earliest savepoint. The active timeline cannot shrink past %s "
+ + "until that savepoint is released.", blocking.get(0)))
+ .finding("Delete the savepoint once the operation it was taken for is complete, using the "
Review Comment:
looks like savepoint can ignore hoodie.archive.beyond.savepoint. When the key is true, the archiver skips savepointed commits and does not stop at them (TimelineArchiverV2), yet the check still reports "blocking archival". `remedyIsAlwaysToReleaseTheSavepoint` asserts the key is never consulted. The skill's guardrail against recommending it is fine, but the check has to honour it when it is already on.
Also, on a table with no savepoints, the check gates on keep.max.commits first and reports SKIPPED. It should report HEALTHY without needing any configuration.
##########
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;
+ static final int EXIT_ERROR = 2;
+
+ private static final ObjectMapper JSON_MAPPER =
+ new ObjectMapper().enable(SerializationFeature.INDENT_OUTPUT);
+
+ private final Config cfg;
+ private final TypedProperties props;
+ private final HoodieTableMetaClient metaClient;
+
+ public HoodieTableHealthChecker(Configuration hadoopConf, Config cfg) {
+ this.cfg = cfg;
+ this.props = UtilHelpers.buildProperties(hadoopConf, cfg.propsFilePath, cfg.configs);
+ this.metaClient = HoodieTableMetaClient.builder()
+ .setConf(HadoopFSUtils.getStorageConfWithCopy(hadoopConf))
+ .setBasePath(cfg.basePath)
+ .setLoadActiveTimelineOnLoad(true)
+ .build();
+ }
+
+ /** All checks this tool knows about, in report order. */
+ static List<HealthCheck> allChecks() {
+ return Arrays.asList(
+ new CompactionCadenceCheck(),
+ new CleanerCadenceCheck(),
+ new SavepointArchivalBlockCheck(),
+ new ArchivalCadenceCheck());
+ }
+
+ /**
+ * Runs the selected checks and renders the report.
+ *
+ * @return the exit code the process should terminate with.
+ */
+ public int run() {
+ List<HealthCheck> selected = selectChecks(cfg.checks);
+ HealthCheckContext context = new HealthCheckContext(metaClient, props, cfg.applyAllDefaults);
+
+ List<HealthCheckResult> results = new ArrayList<>();
+ List<String> erroredChecks = new ArrayList<>();
+
+ for (HealthCheck check : selected) {
+ try {
+ results.add(check.check(context));
+ } catch (Exception e) {
+ // One check failing to reach a verdict must not hide the others. Record it and continue.
+ log.error("Health check '{}' failed to run for table {}", check.getName(), cfg.basePath, e);
+ erroredChecks.add(check.getName() + ": " + e.getMessage());
+ }
+ }
+
+ if (OutputFormat.JSON == cfg.output) {
+ System.out.println(renderJson(results, erroredChecks));
+ } else {
+ System.out.println(renderTable(results, erroredChecks));
+ }
+
+ if (!erroredChecks.isEmpty()) {
+ return EXIT_ERROR;
+ }
+ return results.stream().anyMatch(r -> r.getStatus().isUnhealthy()) ? EXIT_UNHEALTHY : EXIT_HEALTHY;
Review Comment:
so can an errored check outrank UNHEALTHY? i think we should keep the error, but let UNHEALTHY win the exit code, or give "unhealthy + errors" its own code.
##########
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()) {
Review Comment:
i think bot is flagging correctly.. i wrote something similar false positive for archival here https://github.com/apache/hudi/pull/20151/changes#r4163867874
i think cleaner can give a false UNHEALTHY on new tables and on insert-only or bulk_insert tables.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "hudi-bot (via GitHub)" <gi...@apache.org>.
hudi-bot commented on PR #20151:
URL: https://github.com/apache/hudi/pull/20151#issuecomment-5903528417
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "5ad40f3270f4ef9c0a3852c5681d06b366691f06",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "5ad40f3270f4ef9c0a3852c5681d06b366691f06",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 5ad40f3270f4ef9c0a3852c5681d06b366691f06 UNKNOWN
<details>
<summary>Bot commands</summary>
@hudi-bot supports the following commands:
- `@hudi-bot run azure` re-run the last Azure build
</details>
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #20151:
URL: https://github.com/apache/hudi/pull/20151#discussion_r4279812704
##########
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:
Fixed, and this was the sharpest of the findings — the check was blaming the service for what is a configuration conflict.
Under `KEEP_LATEST_COMMITS` the cleaner only acts once it can see more than `hoodie.clean.commits.retained` completed commits. If `hoodie.keep.min.commits` is at or below that, archival trims the timeline first and the cleaner never reaches its trigger: file versions pile up and no clean is ever scheduled, exactly as you saw with 60 versions and zero cleans.
The check now detects that pairing and names it before falling back to the generic finding:
> `hoodie.keep.min.commits=20` is not greater than `hoodie.clean.commits.retained=30`, so archival trims the active timeline before the cleaner has enough commits to act on. Raise `hoodie.keep.min.commits` above 30. Until then file versions accumulate and no clean is scheduled, whatever `hoodie.clean.automatic` is set to.
Two tests cover it: the starved pairing produces that finding and *not* the `hoodie.clean.automatic` one, and a healthy pairing still gets the generic finding so the diagnosis doesn't fire spuriously.
##########
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:
Yes — same root cause as the thread above, and the `KEEP_LATEST_BY_HOURS` reproduction is the clearer demonstration of it, since there the cleaner's retention has no commit-count expression at all.
The threshold now reuses the calculation rather than the raw config. `CleanerUtils.getEarliestCommitToRetain` is called with the table's actual cleaner policy, so `KEEP_LATEST_BY_HOURS` places its window by time exactly as the cleaner does, and the effective bound follows from where that lands. A direct call to `getMinAndMaxInstantsToKeep` isn't possible here — it takes a `HoodieTable` — so the arithmetic is reproduced with a pointer to the original.
On the finding pointing at savepoints: that was the closing finding firing regardless of cause. It now names the earliest pending write instant when there is one, and otherwise lists compaction retention and MDT compaction alongside savepoints rather than leading with savepoints.
Worth flagging a bug this surfaced: the earliest-commit-to-retain lookup parsed the latest commit's instant time unconditionally, but only `KEEP_LATEST_BY_HOURS` needs it as wall-clock. An instant time that wouldn't parse silently disabled the whole derivation and fell back to the configured bound — which is exactly the failure you'd reported, just reached a different way. Now parsed only for the policy that needs it.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "codope (via GitHub)" <gi...@apache.org>.
codope commented on code in PR #20151:
URL: https://github.com/apache/hudi/pull/20151#discussion_r4172174604
##########
hudi-utilities/src/main/java/org/apache/hudi/utilities/health/SavepointArchivalBlockCheck.java:
##########
@@ -0,0 +1,150 @@
+/*
+ * 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.HoodieInstant;
+import org.apache.hudi.common.table.timeline.HoodieTimeline;
+import org.apache.hudi.common.table.timeline.TimelineUtils;
+import org.apache.hudi.common.util.Option;
+import org.apache.hudi.config.HoodieArchivalConfig;
+
+import java.time.Instant;
+import java.time.temporal.ChronoUnit;
+import java.util.Collections;
+import java.util.List;
+import java.util.stream.Collectors;
+
+/**
+ * Checks for savepoints that are holding back archival.
+ *
+ * <p>Archival stops at the earliest savepoint and will not advance past it. A savepoint taken for a
+ * one-off restore and then forgotten therefore pins the active timeline indefinitely. The timeline
+ * grows without bound, every timeline load gets slower, and because archival is blocked the
+ * cleaner's retention interacts with it in ways that keep data files alive too. This is a quiet
+ * failure: the table keeps accepting writes and nothing errors.
+ *
+ * <p>Two signals are reported. A savepoint sitting further back than the configured archival
+ * retention is actively blocking archival now. A savepoint older than {@link #STALE_SAVEPOINT_DAYS}
+ * is flagged separately as likely forgotten, since savepoints are normally short-lived.
+ *
+ * <p>The remedy this check points at is always to release the savepoint once the operation it was
+ * taken for is done. It deliberately does not suggest loosening archival to step over savepoints.
+ */
+public class SavepointArchivalBlockCheck implements HealthCheck {
+
+ static final String NAME = "savepoint";
+
+ /**
+ * Age past which a savepoint is called out as probably forgotten. Savepoints are normally taken
+ * around a specific operation and released soon after; one lingering for weeks is usually an
+ * oversight rather than an intent.
+ */
+ static final int STALE_SAVEPOINT_DAYS = 7;
+
+ @Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
+ public String getDescription() {
+ return "Whether a savepoint is blocking archival or has been left behind";
+ }
+
+ @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()));
+ HoodieTimeline writeTimeline = context.getMetaClient().getActiveTimeline().getWriteTimeline();
+ List<HoodieInstant> savepoints = context.getMetaClient().getActiveTimeline()
+ .getSavePointTimeline().filterCompletedInstants().getInstants();
+
+ HealthCheckResult.Builder result = HealthCheckResult.builder(NAME)
+ .config(HoodieArchivalConfig.MAX_COMMITS_TO_KEEP.key(), maxCommitsToKeep)
+ .config("observed.savepoint.count", savepoints.size());
+
+ if (savepoints.isEmpty()) {
+ return result.status(HealthStatus.HEALTHY).summary("No savepoints on the table").build();
+ }
+
+ List<String> savepointTimes = savepoints.stream()
+ .map(HoodieInstant::requestedTime).collect(Collectors.toList());
+ result.config("observed.savepoints", String.join(",", savepointTimes));
+
+ List<String> blocking = savepointsBeyondRetention(writeTimeline, savepointTimes, maxCommitsToKeep);
+ List<String> stale = savepointTimes.stream()
+ .filter(SavepointArchivalBlockCheck::isOlderThanStaleThreshold)
+ .collect(Collectors.toList());
+
+ if (!stale.isEmpty()) {
+ result.finding(String.format(
+ "Savepoint(s) %s are more than %d days old. If the operation they were taken for is "
+ + "finished, delete them so archival can advance.",
+ String.join(", ", stale), STALE_SAVEPOINT_DAYS));
+ }
+
+ if (blocking.isEmpty()) {
+ return result.status(HealthStatus.HEALTHY)
+ .summary(String.format("%d savepoint(s) present, none older than the archival retention of %d commits",
+ savepoints.size(), maxCommitsToKeep))
+ .build();
+ }
+
+ return result.status(HealthStatus.UNHEALTHY)
+ .summary(String.format(
+ "%d savepoint(s) older than the archival retention are blocking archival: %s",
+ blocking.size(), String.join(", ", blocking)))
+ .finding(String.format(
+ "Archival stops at the earliest savepoint. The active timeline cannot shrink past %s "
+ + "until that savepoint is released.", blocking.get(0)))
+ .finding("Delete the savepoint once the operation it was taken for is complete, using the "
Review Comment:
this is incorrect.. i ran a test and looks good.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on code in PR #20151:
URL: https://github.com/apache/hudi/pull/20151#discussion_r4279809301
##########
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);
+
+ HealthCheckResult.Builder result = HealthCheckResult.builder(NAME)
+ .config(HoodieArchivalConfig.MAX_COMMITS_TO_KEEP.key(), maxCommitsToKeep)
+ .config("effective.unhealthy.threshold.write.instants", retentionThreshold)
+ .config("effective.absolute.ceiling.completed.instants", MAX_REASONABLE_ACTIVE_INSTANTS)
+ .config("observed.write.instants", writeInstants)
+ .config("observed.completed.instants", completedInstants);
+
+ boolean overAbsoluteCeiling = completedInstants >= MAX_REASONABLE_ACTIVE_INSTANTS;
+ boolean overConfiguredRetention = writeInstants >= retentionThreshold;
Review Comment:
You're right, and both reproductions were real. The check compared against the raw `hoodie.keep.max.commits` and never looked at the cleaner at all.
It now reproduces `ArchivalUtils.getMinAndMaxInstantsToKeep`: derive the cleaner's earliest-commit-to-retain via `CleanerUtils.getEarliestCommitToRetain`, compute `minInstantsToKeepBasedOnCleaning`, and raise the bound to `minBasedOnCleaning + max - min` when the cleaner is holding more history than archival would. It can't call `ArchivalUtils` directly — that takes a `HoodieTable` and this tool reads only the timeline — so the calculation is reproduced, with a comment saying so and a fallback to the configured bound when the cleaner's retention can't be determined. `effective.max.commits.to.keep` is echoed in the output so a verdict can be checked against it.
Your `clean.commits.retained=100` / `keep.max.commits=30` case is now a regression test and comes out HEALTHY.
Also fixed the counting: you were right that `getWriteTimeline()` includes requested and inflight compaction and clustering, which archival hasn't had the chance to consider. It now counts completed commits, matching what the archiver reasons over.
On the other blockers — the earliest pending write instant is visible from the timeline, so when one is present the finding names it with its action and state, since that's the thing an operator can act on. MOR compaction retention and the MDT's latest compaction aren't visible here, so the closing finding now says archival also holds back for those rather than implying a savepoint is the only cause.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(utilities): add HoodieTableHealthChecker for table service health [hudi]
Posted by "nsivabalan (via GitHub)" <gi...@apache.org>.
nsivabalan commented on PR #20151:
URL: https://github.com/apache/hudi/pull/20151#issuecomment-6186595861
One more false positive, found after the review round: **the cleaner check flagged every pure-append table.**
A table that only receives inserts never produces a clean instant, and that is correct — cleaning retires superseded file slices, and an append-only workload never supersedes anything. Every file group has one slice, nothing is replaced, so the cleaner runs and finds no work. The check read that empty cleaner timeline as a stalled service, so such a table came out UNHEALTHY every day with nothing the operator could do about it.
Fixed in `f786277318f1`. Append-only is read from the commit metadata rather than from `hoodie.datasource.write.operation`, because what the table actually received is the question: a writer configured for upsert that has only ever inserted still has nothing to clean, and a property file can be stale or absent. One `upsert`, `delete` or `insert_overwrite` anywhere on the timeline and the normal cadence judgement applies again.
It is conservative on failure — an unreadable commit, or an operation type the metadata does not carry, means the exemption does not apply and the check judges as before. Suppressing a real finding is the worse error, so the exemption has to be earned by every completed write on the timeline.
Three tests: append-only is HEALTHY with the reason in the summary, an upsert table with no clean is still UNHEALTHY, and a timeline that is 59 inserts plus a single upsert is still judged.
--
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: commits-unsubscribe@hudi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org