laserninja commented on code in PR #13717:
URL: https://github.com/apache/gravitino/pull/13717#discussion_r4261551129
##########
clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClientConfiguration.java:
##########
@@ -66,8 +66,13 @@ public class GravitinoClientConfiguration {
public static final String CLIENT_SOCKET_TIMEOUT_MS =
GRAVITINO_CLIENT_CONFIG_PREFIX + "socketTimeoutMs";
+ /** Disables automatic transport retries, so ambiguous writes are surfaced
to the caller. */
+ public static final String CLIENT_DISABLE_AUTOMATIC_RETRIES =
Review Comment:
Changed in 9aa13fc7d to gravitino.client.retryTimes: a nonnegative integer,
with 0 disabling retries and default 1 preserving Apache HttpClient's existing
behavior. The collector forces 0 because a transparently retried PUT could
succeed while the original request is still executing, allowing that older
request to overwrite a later cycle. Added HTTP-boundary tests for 0, 1, and 2
retries plus invalid-value checks.
##########
docs/table-maintenance-service/optimizer-cli-reference.md:
##########
@@ -282,6 +283,95 @@ table, so it must be asked for explicitly.
Its `jobConf` is documented in
[Configuration](./optimizer-configuration.md#job-submission-configuration).
+## Update Manifest Statistics
+
+`builtin-iceberg-update-manifest-stats` is a separate Spark job that collects
two table-level
+statistics, including for partitioned tables:
+
+| Statistic | Object entry for each decimal spec ID |
+| --- | --- |
+| `custom-manifest-number-by-spec` | Long manifest count |
+| `custom-avg-manifest-size-by-spec` | Double average manifest size in bytes |
+
+The job reads manifest metadata without scanning file statistics. It works
after partition
+evolution, including for specs whose partition fields are absent from older
files.
+`builtin-iceberg-update-stats` and the `submit-update-stats-job` CLI retain
their existing behavior;
+they do not collect manifest statistics. Submit the new template directly
through the jobs API.
+
+Each table cycle pins one current snapshot, calls `allManifests` once, and
groups data and
+delete manifests by `partitionSpecId`. Every defined spec is included. A spec
with no manifests,
+including a table without a snapshot, produces count `0` and average `0.0`.
+The job also publishes `custom-manifest-snapshot-id` in the same update (`-1`
without a snapshot).
+Snapshot IDs identify measurements; they are not monotonically increasing
version numbers.
+
+The template accepts `catalog_name`, `table_identifier`,
`coordination_directory`,
+`updater_options`, and optional
+`spark_conf`, alongside the standard Spark template settings. There is no
`spec_id` filter:
+collection always replaces the complete maps for the table.
+`updater_options` is a JSON string containing `gravitino_uri` and `metalake`;
it can also select
+a `statistics_updater` implementing the existing table statistics update
operation.
+For example:
+
+```bash
+curl -X POST http://localhost:8090/api/metalakes/test/jobs/runs \
+ -H 'Content-Type: application/json' \
+ -d '{
+ "jobTemplateName": "builtin-iceberg-update-manifest-stats",
+ "jobConf": {
+ "catalog_name": "rest_catalog",
+ "table_identifier": "db.t1",
+ "coordination_directory": "/mnt/gravitino-maintenance-locks",
+ "updater_options":
"{\"gravitino_uri\":\"http://localhost:8090\",\"metalake\":\"test\"}";,
+ "spark_master": "local[2]",
+ "spark_executor_instances": "1",
+ "spark_executor_cores": "1",
+ "spark_executor_memory": "1g",
+ "spark_driver_memory": "1g",
+ "catalog_type": "rest",
+ "catalog_uri": "http://localhost:9001/iceberg";,
+ "warehouse_location": ""
+ }
+ }'
+```
+
+For example, one collection can produce complete maps `{"0": 620, "1": 120}`
and
+`{"0": 10485760.0, "1": 4194304.0}`. The collector owns both statistic names
and publishes
+them together with the snapshot ID in one existing PUT/update call. Old map
entries are
+replaced, not merged. Other statistic names are unchanged. No PATCH or merge
API is needed.
+
+### Shared coordination and recovery
Review Comment:
Agreed that collection failures should be rerunnable. In 9aa13fc7d the
durable marker is now written only after collection, immediately before
invoking the updater. Added a real Spark/Iceberg regression that fails
collection, fixes the cause, and reruns successfully without recovery.
The remaining coordination covers multiple runner hosts for the same table.
The lock spans snapshot selection through publication. Recovery is limited to
failures/crashes after the publication marker: if A sends a PUT and loses its
connection, the server may still finish it after rerun B publishes newer
statistics. The existing PUT has no conditional snapshot check/fencing, and the
updater interface cannot prove that a failed write has stopped. Removing that
guard would allow stale overwrites. I expanded the docs with this example; all
269 relevant tests passed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
