Skip to content

Commit abac270

Browse files
authored
[#11196] feat(maintenance): Collect manifest statistics in single-writer table cycles (#13717)
### What changes were proposed in this pull request? Rebase the collector directly onto current main and remove the dependency on #13532. Collect every spec from one pinned table snapshot with one `allManifests` traversal. Publish both complete maps and `custom-manifest-snapshot-id` in one existing PUT/update call. Default to last-write-wins publication without a coordination directory. Consumers requiring fresh statistics must compare the published snapshot ID with the table's current snapshot and skip/recollect on mismatch. An optional shared coordination directory retains per-table locking through publication and explicit recovery after ambiguous writes. Collection failures are always rerunnable. The collector forces `gravitino.client.retryTimes=0`; other clients retain the default of one retry. ### Why are the changes needed? Follow the single-writer direction agreed in #13532. Avoid scanning the same manifest list once per spec and introducing a public merge API solely for this collector. Related to #11196 and design #12942. ### Does this PR introduce _any_ user-facing change? Adds `builtin-iceberg-update-manifest-stats` with optional `coordination_directory`. Omitted or blank uses last-write-wins; opting in requires a shared filesystem supporting durable files and cross-host Java/POSIX locks. Collection is table-wide with no `spec_id` option. Defined specs without manifests publish zeroes; each update replaces the complete maps, removing stale entries. Snapshot ID is -1 when no snapshot exists. Automatic policy triggering remains separate. ### How was this patch tested? - 222 jobs tests, 19 optimizer API tests, and 30 focused Java client tests passed. - Coverage includes partition evolution, empty tables, one snapshot/manifest pass, independent processes contending for the lock, uncoordinated publication and reruns, safe reruns after collection failures, ambiguous PUT recovery, auth alias propagation, and HTTP retry counts of zero, one, and two. - Formatting, jobs Javadocs, `:docs:build`, `rat`, and `git diff --check` passed. - Cross-host filesystem failure behavior was not exercised; deployments must verify shared lock/durability semantics.
1 parent a00fc7d commit abac270

17 files changed

Lines changed: 1407 additions & 4 deletions

File tree

‎clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClientConfiguration.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,12 @@ public class GravitinoClientConfiguration {
6666
public static final String CLIENT_SOCKET_TIMEOUT_MS =
6767
GRAVITINO_CLIENT_CONFIG_PREFIX + "socketTimeoutMs";
6868

69+
/** Maximum automatic HTTP retries after the initial request; zero disables retries. */
70+
public static final String CLIENT_RETRY_TIMES = GRAVITINO_CLIENT_CONFIG_PREFIX + "retryTimes";
71+
6972
private static final Set<String> SUPPORT_CLIENT_CONFIG_KEYS =
7073
ImmutableSet.of(
74+
CLIENT_RETRY_TIMES,
7175
CLIENT_CONNECTION_TIMEOUT_MS,
7276
CLIENT_SOCKET_TIMEOUT_MS,
7377
CLIENT_MAX_CONNECTIONS,
@@ -96,6 +100,17 @@ public static GravitinoClientConfiguration buildFromProperties(Map<String, Strin
96100
return config;
97101
}
98102

103+
/**
104+
* Returns the maximum automatic HTTP retries after the initial request.
105+
*
106+
* @return nonnegative retry count, defaulting to one as in Apache HttpClient
107+
*/
108+
public int getClientRetryTimes() {
109+
int retryTimes = MapUtils.propertyAsInt(properties, CLIENT_RETRY_TIMES, 1);
110+
checkValue(value -> value >= 0, CLIENT_RETRY_TIMES, retryTimes, POSITIVE_NUMBER_ERROR_MSG);
111+
return retryTimes;
112+
}
113+
99114
/**
100115
* Extract connection timeout from the properties map
101116
*

‎clients/client-java/src/main/java/org/apache/gravitino/client/HTTPClient.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
import org.apache.hc.client5.http.classic.methods.HttpUriRequest;
4949
import org.apache.hc.client5.http.classic.methods.HttpUriRequestBase;
5050
import org.apache.hc.client5.http.config.ConnectionConfig;
51+
import org.apache.hc.client5.http.impl.DefaultHttpRequestRetryStrategy;
5152
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
5253
import org.apache.hc.client5.http.impl.classic.CloseableHttpResponse;
5354
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
@@ -68,6 +69,7 @@
6869
import org.apache.hc.core5.io.CloseMode;
6970
import org.apache.hc.core5.net.URIBuilder;
7071
import org.apache.hc.core5.reactor.ssl.SSLBufferMode;
72+
import org.apache.hc.core5.util.TimeValue;
7173

7274
/**
7375
* An HttpClient for usage with the REST catalog.
@@ -148,6 +150,9 @@ private HTTPClient(
148150
GravitinoClientConfiguration.buildFromProperties(properties);
149151

150152
HttpClientBuilder clientBuilder = HttpClients.custom();
153+
clientBuilder.setRetryStrategy(
154+
new DefaultHttpRequestRetryStrategy(
155+
clientConfiguration.getClientRetryTimes(), TimeValue.ofSeconds(1)));
151156
clientBuilder.setConnectionManager(
152157
configureConnectionManager(clientConfiguration, tlsConfigurer));
153158

‎clients/client-java/src/test/java/org/apache/gravitino/client/TestGravitinoClientConfiguration.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,29 @@
2424
import org.junit.jupiter.api.Test;
2525

2626
public class TestGravitinoClientConfiguration {
27+
@Test
28+
void testAutomaticRetryConfiguration() {
29+
Assertions.assertEquals(
30+
1,
31+
GravitinoClientConfiguration.buildFromProperties(ImmutableMap.of()).getClientRetryTimes());
32+
for (int count : new int[] {0, 1, 3}) {
33+
Assertions.assertEquals(
34+
count,
35+
GravitinoClientConfiguration.buildFromProperties(
36+
ImmutableMap.of(
37+
GravitinoClientConfiguration.CLIENT_RETRY_TIMES, Integer.toString(count)))
38+
.getClientRetryTimes());
39+
}
40+
for (String invalid : new String[] {"-1", "invalid", "1.5", "2147483648"}) {
41+
Assertions.assertThrows(
42+
IllegalArgumentException.class,
43+
() ->
44+
GravitinoClientConfiguration.buildFromProperties(
45+
ImmutableMap.of(GravitinoClientConfiguration.CLIENT_RETRY_TIMES, invalid))
46+
.getClientRetryTimes());
47+
}
48+
}
49+
2750
@Test
2851
void testValidConfig() {
2952
Map<String, String> properties =

‎clients/client-java/src/test/java/org/apache/gravitino/client/TestHTTPClient.java‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
import org.junit.jupiter.api.BeforeAll;
5555
import org.junit.jupiter.api.Test;
5656
import org.mockserver.integration.ClientAndServer;
57+
import org.mockserver.model.HttpError;
5758
import org.mockserver.model.HttpRequest;
5859
import org.mockserver.model.HttpResponse;
5960

@@ -100,6 +101,34 @@ public void testPutSuccess() throws Exception {
100101
testHttpMethodOnSuccess(Method.PUT, true, true);
101102
}
102103

104+
/** Verifies ambiguous PUT connection failures can be surfaced without an automatic resend. */
105+
@Test
106+
public void testPutRetryTimes() throws Exception {
107+
for (int retryTimes : new int[] {0, 1, 2}) {
108+
String path = "put-dropped-connection-" + retryTimes;
109+
HttpRequest expected = request("/" + path).withMethod("PUT");
110+
mockServer.when(expected).error(HttpError.error().withDropConnection(true));
111+
try (RESTClient client =
112+
HTTPClient.builder(
113+
ImmutableMap.of(
114+
GravitinoClientConfiguration.CLIENT_RETRY_TIMES,
115+
Integer.toString(retryTimes)))
116+
.uri("http://127.0.0.1:" + mockServer.getPort())
117+
.build()) {
118+
Assertions.assertThrows(
119+
RESTException.class,
120+
() ->
121+
client.put(
122+
path,
123+
new Item(1L, "stats"),
124+
Item.class,
125+
ImmutableMap.of(),
126+
mock(ErrorHandler.class)));
127+
}
128+
Assertions.assertEquals(retryTimes + 1, mockServer.retrieveRecordedRequests(expected).length);
129+
}
130+
}
131+
103132
@Test
104133
public void testPutFailure() throws Exception {
105134
testHttpMethodOnFailure(Method.PUT, true, true);

‎docs/how-to-use-gravitino-client.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ GravitinoAdminClient gravitinoAdminClient = GravitinoAdminClient.builder("http:/
4242
| Configuration item | Description | Default value | Required |
4343
|----------------------------------------|------------------------------------------------------|---------------------|----------|
4444
| `gravitino.client.connectionTimeoutMs` | An optional http connection timeout in milliseconds. | `180000`(3 minutes) | No |
45+
| `gravitino.client.retryTimes` | Maximum automatic HTTP retries after the initial request. Must be a nonnegative integer; `0` disables retries. | `1` | No |
4546
| `gravitino.client.socketTimeoutMs` | An optional http socket timeout in milliseconds. | `180000`(3 minutes) | No |
4647

4748
**Note:** Invalid configuration properties will result in exceptions.

‎docs/table-maintenance-service/optimizer-cli-reference.md‎

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -237,7 +237,7 @@ EvaluationResult{scopeType=TABLE, identifier=rest_catalog.db.t1, partitionPath=<
237237

238238
## Built-in Job Templates
239239

240-
Five job templates ship with the service, and they are complementary rather than alternatives. A full maintenance pass collects statistics, compacts data files, expires the snapshot history that compaction just created, consolidates manifests, and removes old orphan files.
240+
The following Iceberg job templates ship with the service. A full maintenance pass collects statistics, compacts data files, expires the snapshot history that compaction just created, consolidates manifests, and removes old orphan files.
241241

242242
| Job template | What it does |
243243
|---------------------------------------|-------------------------------------------|
@@ -246,6 +246,7 @@ Five job templates ship with the service, and they are complementary rather than
246246
| `builtin-iceberg-expire-snapshots` | Removes old snapshot metadata |
247247
| `builtin-iceberg-remove-orphan-files` | Removes unreferenced files from storage |
248248
| `builtin-iceberg-rewrite-manifests` | Consolidates small manifest files |
249+
| `builtin-iceberg-update-manifest-stats` | Collects complete per-spec manifest maps |
249250

250251
Each can be submitted directly over REST, and the first two are also what the policy-driven workflow submits on your behalf. See [Quick Start](./optimizer.md#walkthrough) for the policy-driven path.
251252

@@ -282,6 +283,111 @@ table, so it must be asked for explicitly.
282283

283284
Its `jobConf` is documented in [Configuration](./optimizer-configuration.md#job-submission-configuration).
284285

286+
## Update Manifest Statistics
287+
288+
`builtin-iceberg-update-manifest-stats` is a separate Spark job that collects two table-level
289+
statistics, including for partitioned tables:
290+
291+
| Statistic | Object entry for each decimal spec ID |
292+
| --- | --- |
293+
| `custom-manifest-number-by-spec` | Long manifest count |
294+
| `custom-avg-manifest-size-by-spec` | Double average manifest size in bytes |
295+
296+
The job reads manifest metadata without scanning file statistics. It works after partition
297+
evolution, including for specs whose partition fields are absent from older files.
298+
`builtin-iceberg-update-stats` and the `submit-update-stats-job` CLI retain their existing behavior;
299+
they do not collect manifest statistics. Submit the new template directly through the jobs API.
300+
301+
Each table cycle pins one current snapshot, calls `allManifests` once, and groups data and
302+
delete manifests by `partitionSpecId`. Every defined spec is included. A spec with no manifests,
303+
including a table without a snapshot, produces count `0` and average `0.0`.
304+
The job also publishes `custom-manifest-snapshot-id` in the same update (`-1` without a snapshot).
305+
Snapshot IDs identify measurements; they are not monotonically increasing version numbers.
306+
307+
The template accepts `catalog_name`, `table_identifier`, `updater_options`, and optional
308+
`coordination_directory` and `spark_conf`, alongside the standard Spark template settings. There is no `spec_id` filter:
309+
collection always replaces the complete maps for the table.
310+
`updater_options` is a JSON string containing `gravitino_uri` and `metalake`; it can also select
311+
a `statistics_updater` implementing the existing table statistics update operation.
312+
For example:
313+
314+
```bash
315+
curl -X POST http://localhost:8090/api/metalakes/test/jobs/runs \
316+
-H 'Content-Type: application/json' \
317+
-d '{
318+
"jobTemplateName": "builtin-iceberg-update-manifest-stats",
319+
"jobConf": {
320+
"catalog_name": "rest_catalog",
321+
"table_identifier": "db.t1",
322+
"updater_options": "{\"gravitino_uri\":\"http://localhost:8090\",\"metalake\":\"test\"}",
323+
"spark_master": "local[2]",
324+
"spark_executor_instances": "1",
325+
"spark_executor_cores": "1",
326+
"spark_executor_memory": "1g",
327+
"spark_driver_memory": "1g",
328+
"catalog_type": "rest",
329+
"catalog_uri": "http://localhost:9001/iceberg",
330+
"warehouse_location": ""
331+
}
332+
}'
333+
```
334+
335+
For example, one collection can produce complete maps `{"0": 620, "1": 120}` and
336+
`{"0": 10485760.0, "1": 4194304.0}`. The collector owns both statistic names and publishes
337+
them together with the snapshot ID in one existing PUT/update call. Old map entries are
338+
replaced, not merged. Other statistic names are unchanged. No PATCH or merge API is needed.
339+
340+
By default, `coordination_directory` is omitted or blank and publications use last-write-wins
341+
semantics. Each update contains the complete maps and snapshot ID; a concurrent older collection
342+
may overwrite newer measurements. Failed jobs can simply be rerun, with no lock files or recovery.
343+
Consumers requiring fresh statistics must compare `custom-manifest-snapshot-id` with the table's
344+
current snapshot ID (`-1` if none) and skip/recollect on a mismatch. This is a consumer requirement,
345+
not a freshness check enforced by this collector. Snapshot IDs must be compared for equality,
346+
not numerical ordering.
347+
348+
### Optional shared coordination and recovery
349+
350+
For stricter serialization, provision `coordination_directory` (`--coordination-dir` for direct Spark invocation) as an
351+
existing absolute directory on a shared filesystem that supports durable files and cross-host
352+
Java/POSIX file locks. Every runner for a destination table must use the same shared directory,
353+
metalake, and canonical catalog/schema/table names. A host-local directory, object-store mount,
354+
or filesystem without reliable cross-host locking does not satisfy this requirement.
355+
All writers of these statistic names must opt into this mode and use the same directory;
356+
mixing coordinated and uncoordinated writers does not provide serialization.
357+
358+
The job locks a persistent file keyed by metalake and table before refreshing the Iceberg table
359+
or choosing a snapshot, and holds it through the PUT. Concurrent runs for that table fail;
360+
other tables can proceed. Snapshot IDs are recorded for provenance, not compared numerically.
361+
The job forces `gravitino.client.retryTimes=0` for its Gravitino client.
362+
Custom statistics updaters and any intervening proxies must likewise avoid retrying PUT requests;
363+
an automatic resend could hide an original request that is still executing. There are no expiring
364+
leases, automatic retries, scheduling, or cooldowns.
365+
366+
Collection failures can be rerun normally: the lock is released without leaving a recovery marker.
367+
After collection and immediately before calling the updater, the job durably writes an in-progress
368+
marker. It clears the marker only after a successful update. A crash after the marker is written or
369+
an update failure leaves the marker in place, even after the operating system releases the lock.
370+
The updater interface cannot distinguish a failed request from a request still executing on the
371+
server, so these failures require recovery. The error reports the recovery file path.
372+
373+
For example, run A sends a PUT and loses its connection while the server is still processing it.
374+
An immediate rerun B could publish newer measurements before A's delayed PUT overwrites them.
375+
The file lock prevents concurrent collection across hosts; the marker prevents this overlap after
376+
an ambiguous failure. Snapshot IDs provide provenance, but the existing PUT does not enforce
377+
snapshot ordering or fencing. Automatic reruns after ambiguous writes would require server-side
378+
conditional publication or an equivalent guarantee that the earlier request has ended.
379+
380+
For recovery, stop all runners for that table and verify the previous process has terminated
381+
and any outstanding Gravitino PUT has completed or been definitively cancelled. While holding
382+
an exclusive lock on the reported file, truncate it to zero bytes and flush it to durable storage,
383+
then resume collection. Never delete or replace the file: a new inode can permit two independent
384+
locks. If the previous PUT outcome cannot be established, leave the marker intact.
385+
386+
Consumers must read both objects from one table-statistics response and use the same chosen
387+
spec key in each. A missing object or key means collection is required, not zero manifests.
388+
`IcebergManifestStatistics.fromStatistics` returns an empty result for an incomplete pair.
389+
Automatic manifest policy triggering is separate follow-up work.
390+
285391
## Rewrite Data Files
286392

287393
`builtin-iceberg-rewrite-data-files` performs the compaction itself, merging small data files into larger ones. It is what a compaction policy submits when its thresholds are crossed.

‎maintenance/jobs/build.gradle.kts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ val sparkMajorVersion = "3.5"
3636

3737
dependencies {
3838
implementation(project(":api"))
39+
implementation(project(":clients:client-java"))
3940
implementation(project(":maintenance:optimizer-api"))
4041
implementation(project(":maintenance:updaters"))
4142

@@ -56,7 +57,6 @@ dependencies {
5657
testImplementation(project(":common"))
5758
// Used to check that the placeholders of the built-in templates resolve the way the server does.
5859
testImplementation(project(":core"))
59-
testImplementation(project(":clients:client-java"))
6060
testImplementation(libs.bundles.log4j)
6161
testImplementation(libs.hadoop3.common) {
6262
exclude("org.slf4j")

‎maintenance/jobs/src/main/java/org/apache/gravitino/maintenance/jobs/BuiltInJobTemplateProvider.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.gravitino.maintenance.jobs.iceberg.IcebergRemoveOrphanFilesJob;
3030
import org.apache.gravitino.maintenance.jobs.iceberg.IcebergRewriteDataFilesJob;
3131
import org.apache.gravitino.maintenance.jobs.iceberg.IcebergRewriteManifestsJob;
32+
import org.apache.gravitino.maintenance.jobs.iceberg.IcebergUpdateManifestStatsJob;
3233
import org.apache.gravitino.maintenance.jobs.iceberg.IcebergUpdateStatsAndMetricsJob;
3334
import org.apache.gravitino.maintenance.jobs.spark.SparkPiJob;
3435
import org.slf4j.Logger;
@@ -50,6 +51,7 @@ public class BuiltInJobTemplateProvider implements JobTemplateProvider {
5051
new IcebergRewriteDataFilesJob(),
5152
new IcebergRewriteManifestsJob(),
5253
new IcebergUpdateStatsAndMetricsJob(),
54+
new IcebergUpdateManifestStatsJob(),
5355
new IcebergExpireSnapshotsJob(),
5456
new IcebergRemoveOrphanFilesJob());
5557

0 commit comments

Comments
 (0)