laserninja opened a new pull request, #12937:
URL: https://github.com/apache/gravitino/pull/12937
### What changes were proposed in this pull request?
This continues #11216 by @ibrahimErbilen, whose commits are preserved here
with authorship intact and who is credited as co-author. That PR had gone stale
against `main` and had open review comments; this rebases it and resolves them.
- Added the built-in Iceberg maintenance job
`builtin-iceberg-rewrite-manifests`, which runs Iceberg's `rewrite_manifests`
procedure through Spark SQL. Parameters: `catalog_name`, `table_identifier`
(required), `use_caching`, `spark_conf` (optional).
- Registered it in `BuiltInJobTemplateProvider`.
Changes made on top of the original PR:
- Fixed the misplaced closing parenthesis that left the new job outside
`ImmutableList.of(...)` and broke compilation, and restored import ordering.
- Switched to `IcebergJobUtils` for SQL escaping, argument parsing, and
Spark config parsing, instead of reaching into `IcebergRewriteDataFilesJob`,
and dropped the duplicated `parseArguments`. `IcebergJobUtils` landed on `main`
after the original PR was opened, so the shared-utility extraction the reviewer
asked for already exists.
- Optional arguments now drop unresolved `{{...}}` placeholders.
`JobManager.replacePlaceholder` forwards a placeholder verbatim when the
`jobConf` key is absent, so `use_caching => {{use_caching}}` silently parsed to
`false` and disabled caching instead of leaving Iceberg's default in place.
- A `use_caching` value that is neither `true` nor `false` is now rejected,
rather than letting `Boolean.parseBoolean` turn a typo into `false`.
- Procedure output is read via `collectAsList()` and `Number`, so the job
does not depend on whether the runtime Iceberg version types the manifest
counts as `int` or `long`. Iceberg is a `compileOnly` dependency supplied by
the cluster, so that width can differ from what we compile against.
- Updated the SQL assertions for backtick-quoted catalog identifiers, which
is what `escapeSqlIdentifier` produces on current `main`, and renamed
`testJobTemplateHasMainClass` to match the property it asserts.
- Documented the job in the table maintenance CLI reference, alongside the
three existing templates.
### Why are the changes needed?
Frequent commits leave Iceberg tables with many small manifest files. Scan
planning has to open every one to decide which data files match a filter, so
planning cost grows with manifest count. `builtin-iceberg-rewrite-data-files`
addresses data file layout; this job addresses the metadata that points at it,
enabling policy-driven manifest optimization server-side.
Fix: #11196
### Does this PR introduce _any_ user-facing change?
Yes. A new built-in job template `builtin-iceberg-rewrite-manifests` is
available over the jobs REST API, with new `jobConf` keys `use_caching` and the
standard `catalog_name` / `table_identifier` / `spark_conf`. Documented in
`docs/table-maintenance-service/optimizer-cli-reference.md`.
### How was this patch tested?
- 23 unit tests in `TestIcebergRewriteManifestsJob` covering template shape,
argument parsing, `use_caching` validation, unresolved-placeholder filtering,
generated SQL, and SQL-injection escaping for both the catalog identifier and
the table string.
- `./gradlew :maintenance:jobs:test :maintenance:jobs:javadoc
:maintenance:jobs:spotlessCheck -PskipITs` - passing.
- `./gradlew :docs:build` - passing.
Not covered: no Spark-backed integration test. The two sibling jobs with
`*WithSpark` tests set a precedent, and this is worth adding, but I would
rather not bundle it into the takeover.
--
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]
