[spark-3.5] Consolidate the maintenance SQL extensions and wire them to the OFD/SE table properties - #28
Draft
mkuchenbecker wants to merge 6 commits into
Draft
[spark-3.5] Consolidate the maintenance SQL extensions and wire them to the OFD/SE table properties#28mkuchenbecker wants to merge 6 commits into
mkuchenbecker wants to merge 6 commits into
Conversation
…park-3.1) spark-3.5 previously borrowed the ANTLR grammar (via spark-3.1's generated-src srcDir) and several classes (OpenHouseCatalog, IcebergCatalogMapper, OpenhouseSparkSessionExtensions, OpenhouseSqlExtensionsAstBuilder, OpenhouseDataSourceV2Strategy, GrantableResourceTypes, Principal) from the spark-3.1 shadow jar. This coupling meant the SQL grammar could not evolve for spark-3.5 without also changing spark-3.1. Give spark-3.5 its own copy of the grammar and these classes, add the ANTLR generation task (antlr4 4.7.1, matching spark-3.1), and drop the spark-3.1 module dependency. No behavior change; runtime + itest compile against the standalone module. Co-authored-by: Copilot <[email protected]>
Adds VACUUM <table> [REMOVE ORPHAN FILES] [RETAIN n HOURS] as an OpenHouse Spark SQL extension: grammar, AST builder, logical plan, and exec node that delegates to the catalog's expire_snapshots / remove_orphan_files procedures. Orphan-file deletion is opt-in and runs first, since it needs no write quota. Squashed from linkedin#661.
…ring) Adds OPTIMIZE <table> [FULL] [REWRITE MANIFESTS]. With no clustering keys configured this is a plain bin-pack compaction; with optimize.cluster.keys set it is a sort / z-order rewrite that is incremental by default, tracked by a watermark and interval state persisted as table properties. Also compacts merge-on-read position delete files after the data rewrite, and makes the extensions parser an Iceberg ExtendedParser so sort/zorder rewrites resolve their sort order. Squashed from linkedin#662.
…nsion Read-only probe that reports how well a table is clustered to its current key selection, using the optimize.cluster.* state OPTIMIZE persists plus manifest metrics. Coverage and per-key depth are computed with distributed SQL over metadata, so the command is safe on tables with very large file counts. Only the COMPUTE CLUSTERING QUALITY variant is intercepted; ANALYZE TABLE ... COMPUTE STATISTICS still goes to Spark. Cherry-picked from linkedin#663.
… read VACUUM and OPTIMIZE invented their own property contract instead of the one the scheduled snapshot-expiration (SE) and orphan-file-deletion (OFD) jobs respect. Three defects followed. The VACUUM opt-in could never be set. The gate was `openhouse.vacuum.enabled`, but the /tables service treats `openhouse.`-prefixed keys as reserved and rejects any ALTER TABLE that touches them, so VACUUM was permanently disabled on every real OpenHouse table; the statement test passed only because it runs against a Hadoop catalog. The gate moves to `maintenance.vacuum.enabled`. VACUUM ignored the expiration policy. With no RETAIN it fell through to the Iceberg procedure defaults rather than the table's `policies.history`, so a hand-run VACUUM and the scheduled job disagreed about the same table. It now resolves maxAge x granularity and the versions cap the way TableSnapshotsExpirationTask and Operations.expireSnapshots do, falling back to the job's own 3-day default. VACUUM ignored the orphan-file job's rules. It now takes that job's 7-day default and honors `ofd.one_day_ttl.enabled`, and refuses REMOVE ORPHAN FILES on a table configured for orphan backups: there the job moves orphans into the backup directory through a delete hook the stored procedure has no equivalent of, so running the procedure would destroy files the platform expects to remain recoverable and treat the backup directory itself as orphans. Both commands now also refuse to run on a table opted out of platform maintenance (`maintenance.disabled`, `maintenance.<JOB_TYPE>.disabled`), and VACUUM refuses replica tables, since the scheduled expiration job runs on primaries only. VACUUM reports the windows it resolved as output rows. The jobs app and the Spark extensions are separate artifacts and cannot share code, so the mirrored keys and defaults live in MaintenanceProperties and are pinned by MaintenancePropertiesTest.
Pre-existing formatting violation on main (from the delta-harness commit), unrelated to this branch's changes but enough to fail `./gradlew spotlessCheck` and therefore CI. Output of `spotlessApply`; comment text is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Consolidates the four stacked spark-3.5 SQL-extension draft PRs (linkedin/openhouse linkedin#660, linkedin#661, linkedin#662, linkedin#663) onto one branch, and wires the resulting maintenance DDL to the table properties the OpenHouse maintenance jobs actually read. linkedin#661 and linkedin#662 were siblings off linkedin#660, so no branch carried all three verbs; the shared files (grammar, AST builder, parser gate, V2 strategy) are union-merged here.
VACUUM table [REMOVE ORPHAN FILES] [RETAIN n HOURS]OPTIMIZE table [FULL] [REWRITE MANIFESTS], position-delete compaction, and the IcebergExtendedParserfixANALYZE TABLE table COMPUTE CLUSTERING QUALITYChanges
Client-facing API changes. Three new SQL verbs on spark-3.5 (
VACUUM,OPTIMIZE,ANALYZE TABLE ... COMPUTE CLUSTERING QUALITY). TheVACUUMopt-in property changes fromopenhouse.vacuum.enabledtomaintenance.vacuum.enabled; since the former could never be set on a real table (see below), nothing depends on it.VACUUMnow returns(metric, value)rows reporting the retention windows it resolved.Bug fixes. The commands previously invented their own property contract, which produced three defects:
openhouse.vacuum.enabled, but the /tables service treatsopenhouse.-prefixed keys as reserved and rejects anyALTER TABLE ... SET TBLPROPERTIEStouching them, soVACUUMwas permanently disabled on every real OpenHouse table. It passed CI only because the statement test runs against a Hadoop catalog. The gate moves to themaintenance.namespace.RETAINit fell through to the Iceberg procedure defaults instead of the table'spolicies.history, so a hand-runVACUUMand the scheduled job disagreed about the same table. It now resolvesmaxAgexgranularityand applies theversionscap as a second expiration, exactly asTableSnapshotsExpirationTask/Operations.expireSnapshotsdo, falling back to the job's own 3-day default.ofd.one_day_ttl.enabled, and refusesREMOVE ORPHAN FILESon a table configured for orphan backups: there the job moves orphans into the backup directory through a delete hook the stored procedure has no equivalent of, so running the procedure would destroy files the platform expects to remain recoverable and treat the backup directory itself as orphans.VACUUMandOPTIMIZEalso now refuse to run on a table opted out of platform maintenance (maintenance.disabled, or the per-job-typemaintenance.SNAPSHOTS_EXPIRATION.disabled/maintenance.ORPHAN_FILES_DELETION.disabled/maintenance.DATA_COMPACTION.disabled), andVACUUMrefuses replica tables, since the scheduled expiration job runs on primaries only.The jobs app and the Spark extensions ship as separate artifacts and cannot share code, so the mirrored keys and defaults live in one place (
MaintenanceProperties.scala) and are pinned byMaintenancePropertiesTest.Documentation.
docs/VACUUM.mdupdated for the new gate, the default-retention table, and the refusal cases. Newdocs/OPTIMIZE.md, which records a known gap: the scheduled data-compaction job bin-packs from the persisted data-layout strategies and does not readoptimize.cluster.*, so it neither preserves the clusteringOPTIMIZEestablishes nor respects its incremental watermark.The last commit reflows one javadoc comment in
tables-test-fixtures— a pre-existingspotlessCheckviolation onmain, unrelated to these changes but enough to fail the build.Testing Done
Because the grammar now ships in the OpenHouse extension rather than in a patched Spark, the real-catalog integration tests no longer need a custom Spark build and are enabled (they were
@Disabled):VacuumTestSpark3_5,OptimizeTestSpark3_5,AnalyzeClusteringTestSpark3_5.Run on JDK 17 against stock spark-sql 3.5:
catalogTest— 72 tests, 0 failures (VacuumTestSpark3_56,OptimizeTestSpark3_52,AnalyzeClusteringTestSpark3_52, and no regressions inBranchTestSpark3_524,CatalogOperationTest15,WapIdTest10,RTASTest5, …).statementTest— 85 tests, 0 failures (VacuumStatementTest14,OptimizeStatementTest6,AnalyzeClusteringQualityStatementTest5, plus the pre-existing policy/grant suites).MaintenancePropertiesTest(12),OptimizeTableTest,AnalyzeClusteringQualityExecTest.What the new coverage pins:
MaintenancePropertiesTest— the resolver: policy window, granularity mapping,versionscap, malformed-policy failure, orphan defaults, disable switches, replica detection.VacuumStatementTest— the new gate, the policy-driven default, theversionscap, the orphan defaults, and each refusal path.VacuumTestSpark3_5— against the embedded server, the parts only a real server can exercise: themaintenance.gate is settable while theopenhouse.one is rejected, and the default window comes from the server-persistedpolicies.historyset viaALTER TABLE ... SET POLICY (HISTORY ...).Additional Information
The five feature commits are the four upstream PRs plus the wiring, each self-contained; the upstream PRs above are the smaller-PR breakdown this consolidates.
🤖 Generated with Claude Code
https://claude.ai/code/session_01V7Ydkx4dHrT8T2KPRosSko