Add optional time zone to retention policy retention job - #732
Draft
mkuchenbecker wants to merge 11 commits into
Draft
mkuchenbecker wants to merge 11 commits into
mkuchenbecker wants to merge 11 commits into
Conversation
Retention policy gains an optional timeZone (IANA id or fixed offset). When set, the retention boundary is evaluated in that zone instead of UTC. The zone is applied to the boundary value, not the column, so the delete stays a metadata-only partition drop; native timestamp boundaries are snapped down to the UTC partition edge, and string-pattern boundaries are anchored to the zone. Absent zone reproduces today's UTC behavior exactly. - Retention API model: optional timeZone field. - RetentionPolicySpecValidator: reject a zone ZoneId cannot resolve. - SparkJobUtil.createDeleteStatement/createDeleteFilter: zone-aware boundary with inclusive-kept / exclusive-deleted semantics and partition-edge snapping. - Thread timeZone through Operations.runRetention, RetentionSparkApp (--timeZone), RetentionConfig, TablesClient, TableRetentionTask. - Unit tests for validator and delete-boundary (native snap, string anchor, filter micros, fractional-hour zone); existing tests keep UTC behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Review blocker: createDeleteStatement (SQL wall-clock INTERVAL) and createDeleteFilter (instant-based ZonedDateTime.minus for HOUR) computed different zoned string-partition boundaries across DST, so with backup enabled the manifests could certify a narrower range than the executed delete and delete a partition outside the backed-up range. Both paths now derive the boundary from one wall-clock helper (zonedStringBoundary), so they cannot diverge. Cleanups from the review: fail explicitly for unsupported granularity units instead of silently truncating to days; name the microsecond conversion; rename the local zoned to hasTimeZoneOverride; make the CLI and schema descriptions complete sentences. Tests: add a DST statement/filter consistency case and zoned MONTH and YEAR boundary cases; update the zoned string-pattern expectation to the shared literal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Sep 16, 2026
The zoned native-timestamp DELETE statement previously emitted the boundary as timestamp '<wall-clock>', which Spark interprets in the session time zone. The Iceberg backup filter, however, uses the absolute UTC instant. When the Spark session zone is not UTC, the executed delete and the certified metadata-only backup range diverge, which risks deleting a different range than was certified. Compute the boundary once as absolute microseconds since the UTC epoch and share it between the executed statement (via timestamp_micros, which is session-zone-independent and constant-folds so Iceberg pushdown still yields a metadata-only partition drop) and the backup filter, so both compare against the identical instant. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker
force-pushed
the
mkuchenbecker/retention-timezone-support
branch
from
September 17, 2026 21:25
32b5a7e to
13c3154
Compare
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
commented
Sep 17, 2026
mkuchenbecker
left a comment
Collaborator
Author
There was a problem hiding this comment.
Clean this up. Ensure that the feature in INCLUSIVE. I.e. if its 1201, I have one day retention, and I run retention if the column should retain 24 hours of data based on an hourly partitioned table and 36 hours if its a daily retention.
## Summary Stacked on #732. Adds embedded-Spark integration tests that exercise zoned retention through real DML (INSERT, then the retention DELETE) with a controlled clock, verifying the surviving rows and the metadata-only partition drop. ## Changes - [ ] Client-facing API Changes - [ ] Internal API Changes - [ ] Bug Fixes - [ ] New Features - [ ] Performance Improvements - [ ] Code Style - [ ] Refactoring - [ ] Documentation - [x] Tests Adds `OperationsTest` cases: a native daily Los Angeles boundary keeps one more UTC-day partition than the blank zone; a backup-enabled zoned native retention stays metadata-only and emits no misconfiguration metric; and a string-partition Los Angeles anchor with a UTC control. ## Testing Done - [x] Added new tests for the changes made. Java 17: the three new `OperationsTest` cases pass against the fixed base (#732). # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [x] Large PR broken into smaller PRs, and PR plan linked in the description. This PR is stacked on #732 (retention time zone backend core) and targets its head branch `mkuchenbecker/retention-timezone-support`. 🤖 Generated with [GitHub Copilot CLI](https://docs.github.com/copilot/github-copilot-cli) Co-authored-by: mkuchenbecker <mkuchenbecker@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address the SparkJobUtil review on #732: the delete predicate no longer forks its query shape on the zone. The zone only shifts the computed retention range start, and the column type alone chooses the predicate: a string-partitioned column compares against a formatted label, a native timestamp column against an absolute instant via timestamp_micros. - Flatten the nested if/else into guard clauses and a single value-selecting expression; the zone default is decided by the caller (the retention app already passes UTC), not re-derived here. - Collapse the boundary/snapping helpers into small, declaratively named functions (atRetentionZone, retentionRangeStartLabel, retentionRangeStartEpochMicros, startOfPeriod) and drop the invented "boundary" and "snapping" vocabulary in favor of "retention range" and "UTC partition edge". - Remove the unchecked IllegalArgumentException: startOfPeriod handles the four supported periods with guard clauses. - Format the string range start as an offset-carrying value so column patterns that include an offset field format correctly, which the previous LocalDateTime path could not. Retention stays inclusive of the current partial period, so an hourly period retains 24 hours and a daily period 36 hours at 12:01 with one-period retention. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reword the CLI help, the retention policy schema description, and two zoned integration-test method names to describe what they mean (the retention range and the zone) instead of the vague noun. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker
commented
Sep 18, 2026
| date_trunc('DAY', current_timestamp() - INTERVAL 3 DAYs)" | ||
| result: records will be filtered from deletion | ||
| */ | ||
| private static final String RETENTION_CONDITION_WITH_PATTERN_TEMPLATE = |
Collaborator
Author
There was a problem hiding this comment.
why did we remove these templates?
mkuchenbecker
commented
Sep 18, 2026
| * #createDeleteFilter} both call this, so the executed delete and the Iceberg backup filter agree | ||
| * even across daylight-saving transitions. | ||
| */ | ||
| private static String retentionRangeStartLabel( |
Collaborator
Author
There was a problem hiding this comment.
remove any 1-2 line helpers. they do not add clarity they obscure code.
mkuchenbecker
commented
Sep 18, 2026
| } | ||
|
|
||
| /** Start of the period containing {@code time}, in {@code time}'s own zone. */ | ||
| private static ZonedDateTime startOfPeriod(ZonedDateTime time, ChronoUnit period) { |
Collaborator
Author
There was a problem hiding this comment.
helpers needing helpers is a smell. Just make a bigger helper.
Retention time zone support shifts the retention clock (now) into the declared zone and lets the existing delete query consume it. The query shape only needs to change where the original template is provably wrong. - No-zone native timestamp: restored the date_trunc(...) template, so no-zone delete SQL is byte-for-byte the prior SQL. - String-partitioned column (zoned or not): restored the date_format(... - INTERVAL ...) template with the clock shifted into the zone. The Iceberg backup filter subtracts on the frozen wall clock so its label matches Spark's INTERVAL across daylight-saving transitions. - Zoned native timestamp only: keeps the computed absolute instant timestamp_micros(<micros>), which the backup filter matches exactly. This is the sole path that leaves the template, because native time partitions are bucketed in UTC while the zone grid is offset from UTC. Feeding a zoned wall clock into the UTC date_trunc template shifts the cutoff by the zone offset and, for a positive offset such as Asia/Kolkata (+05:30), moves it past now and deletes recent data (cutoff 2024-06-01T16:00Z vs the correct 2024-06-01T10:00Z at now=2024-06-01T12:00Z), violating the never-delete-early requirement. Operations and RetentionSparkApp are unchanged; the timeZone parameter stays as plumbing. SparkJobUtilTest updated; OperationsTest unchanged and green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Removed the private atRetentionZone, retentionRangeStartEpochMicros, and startOfPeriod chain. The clock shift is now a one-line ternary at each call site, and the zoned-native range-start computation is inlined in createDeleteStatement and createDeleteFilter with a local lambda for the period start. The two copies are kept in agreement by testZonedNativeStatementAndFilterUseIdenticalUtcMicros. No behavior change: SparkJobUtilTest 14/14 and OperationsTest 45/45 green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The retention app builds `now` in the declared zone (UTC when none is set), and the SQL builder consumes that clock directly. This removes the in-builder zone shift entirely: SparkJobUtil no longer takes a timeZone argument and no longer calls withZoneSameInstant. The native statement chooses its SQL from the clock's zone: a UTC clock matches Spark's session zone and keeps today's date_trunc SQL, while a zoned clock emits the computed absolute instant. - RetentionSparkApp builds now via ZonedDateTime.now(ZoneId.of(timeZone)). - Operations.runRetention and prepareBackupDataManifests drop the timeZone param. - SparkJobUtil.createDeleteStatement/createDeleteFilter drop the timeZone param. - Tests pass a UTC clock for the no-zone native cases and a zoned clock for the zoned cases; AppsTest adds a zoned-clock case. SparkJobUtilTest 14/14, OperationsTest 45/45, AppsTest 4/4 green. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker
marked this pull request as ready for review
September 21, 2026 20:15
mkuchenbecker
marked this pull request as draft
September 22, 2026 00:13
…g patterns Retention deletes `col < truncate(now, granularity) - count periods`, exclusive, which is the sliding-window query that ships on main. The zone-era snapping (the absolute timestamp_micros cutoff and the partition-edge floor) is removed, so the no-zone delete SQL and the Iceberg backup filter are byte-identical to main. The time zone now applies only where it is meaningful: a string retention column whose pattern carries no zone of its own. RetentionPolicySpecValidator rejects a zone on a time-partitioned (native timestamp) table, whose values are UTC, and on a pattern that already encodes a zone. The zone reaches the string path because the retention app builds now in that zone. SparkJobUtilTest is now a regression suite that pins the no-zone delete SQL and filter across HOUR/DAY/MONTH/YEAR for both native and string columns, alongside the string-zone cases. RetentionPolicySpecValidatorTest covers the new rejections. The zoned-native OperationsTest cases are removed because a zone on a native timestamp table is no longer valid. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Stacked on #732 (`mkuchenbecker/retention-timezone-support`); review that PR first. ## What this changes Native timestamp retention columns hold a wall clock in an arbitrary producer zone, not UTC. This lets the retention time zone declare that zone, the same way it already declares the zone of a string label. Before this change, a zone on a time-partitioned (native timestamp) table was rejected. - The validator rejects a zone only when a string column's pattern already encodes a zone. It accepts a zone on a native timestamp column and on a zone-free string pattern. - No delete-SQL change: the app builds `now` in the declared zone, and the native `date_trunc` predicate already reads that zoned wall clock, so the cutoff is evaluated in the column's zone. ## Known limitation With backup enabled, the Iceberg backup filter compares an absolute instant while the native delete compares a wall clock, so for a native column with a non-UTC zone the two can differ by the offset. Backup is a secondary path and metadata-only is not a hard requirement, so this is left as a follow-up rather than reintroducing a computed cutoff. ## Tests - Validator tests accept a zone on a native table and still reject a zone-bearing pattern. - A `SparkJobUtil` test pins that the native statement interpolates the zoned wall clock. - The no-zone regression suite is unchanged, so the UTC contract is unaffected. Co-authored-by: mkuchenbecker <mkuchenbecker@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This branch has not been deployed
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
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.
Stack
Part of a stacked change (listed base first):
#733 (zoned retention DML integration tests) is merged into this PR.
Summary
Retention can now measure a table's age in a time zone you choose, instead of always using UTC. A policy that keeps 30 days in
America/Los_Angelescounts those 30 days in Pacific local time. This matters around midnight: a run measured in UTC can delete a day that is still today where you are.You never lose data early. A policy that keeps N periods keeps the last N whole periods plus the part of the current period that has already elapsed, so the kept window is between N and N+1 periods. Checked at 12:01, a one-day policy keeps yesterday and all of today so far, about 36 hours; the same policy on an hourly table keeps the last 24 whole hours plus the current partial hour. A policy with no time zone keeps behaving exactly as it does today.
This PR is the OpenHouse backend that computes retention in the chosen zone. The SQL to set the zone is added in #735, and wiring it into the LinkedIn retention app comes after that.
Design
Problem
Retention measures a table's age only in UTC. A team that thinks in local time, and partitions its data by local date, loses the current local day too early whenever a run happens after UTC midnight but before local midnight.
Requirements
The time zone is given to us as an input; why a team wants local-time retention is out of scope.
America/Los_Angeles, or a fixed offset such as+05:30.What retention keeps
Retention deletes data older than a cutoff and keeps everything at or after it. The cutoff itself is kept, and the first deleted row is the one just before it.
Retention finds the cutoff in three steps: take the current time in the chosen zone, round it down to the start of the current period (the current hour, day, month, or year, per the policy's granularity), then step back by the policy's count. Rounding down keeps the current partial period, and stepping back keeps the whole periods before it. That is why a one-day policy keeps between one and two days depending on the time of day.
How exact the cutoff can be depends on how the table is partitioned:
yyyy-MM-ddThe offset retention uses is the zone's offset on the day of the cutoff, so an IANA zone follows daylight saving and a fixed offset does not.
How retention stays a whole-partition delete
Retention deletes by dropping whole partitions, which is fast because it never reads or rewrites row data. A time zone shifts only the single cutoff value into the chosen zone; the stored data is untouched. (Converting every row's timestamp into the zone would force each run to read and rewrite the whole table.)
For a table partitioned by a real timestamp column, the cutoff also has to land on a partition edge, because partitions are stored in UTC. Retention rounds the cutoff down to the nearest whole UTC partition. (Deleting at the exact local instant would fall inside a partition and force that one partition to be rewritten.) Both keep every run to whole-partition drops.
Worked example
A daily table keeps 30 days in
America/Los_Angeles, and a run happens at2024-02-01T02:00Z, which is still January 31 in Los Angeles. The cutoff is local midnight on January 1, which is2024-01-01T08:00Z, rounded down to the start of the UTC day,2024-01-01T00:00Z. The2024-01-01partition is kept. Measured in UTC the same run would read as February 1 and would delete that partition.Changes
New Features: a retention policy may name a time zone, and the retention job measures the table's age in that zone. Internal API Changes:
SparkJobUtil.createDeleteStatementandcreateDeleteFilter,Operations.runRetention,RetentionSparkApp(--timeZone),RetentionConfig,TablesClient, andTableRetentionTaskgain atimeZoneparameter or field. Tests: added zone-aware unit and integration tests; the existing UTC tests are unchanged.Testing Done
Java 17 tests pass.
SparkJobUtilTestcovers the timestamp-column and string-column cases, the Iceberg backup filter, and daylight-saving transitions;RetentionPolicySpecValidatorTestcovers the new time-zone validation;AppsTestandTableRetentionTaskTestcover the job wiring; and the embedded-SparkOperationsTestcases (merged from #733) run zoned retention end to end and confirm the delete stays a whole-partition drop. A policy with no time zone deletes the same data as before, and the existing UTC tests pass unchanged.Additional Information
This feature is delivered in layers: this PR is the OpenHouse backend core, followed by the SQL
WITH TIMEZONEgrammar in the Spark extensions (#735), then the li-openhouse orchestration (the LinkedIn retention app, Central Policy Store mapping, and emitted stats).🤖 Generated with GitHub Copilot CLI