[default values] Tables: apply server-supplied per-column defaults at read time - #645
Conversation
c6641c7 to
bc2d69c
Compare
9b5e906 to
14c82c3
Compare
7c276f0 to
fdfaf48
Compare
d4ac8fd to
7df59f3
Compare
shanthoosh
left a comment
There was a problem hiding this comment.
Looks good overall. Have few minor clarification questions/comments.
7df59f3 to
747c7ef
Compare
… GetTableResponseBody and capture it client-side (#644) ## Summary Adds a generic, server-stamped, per-table client **`config`** map to `GetTableResponseBody` and captures it in `OpenHouseTableOperations` so subclasses can read it. It follows the **Iceberg REST `LoadTableResponse.config` convention** — a `Map<String,String>` of client-side behavior overrides the server controls at runtime, without re-rolling the slow-to-upgrade Java client fleet. **No behavior change**: the map is null/empty until a server stamps it, and the base client has nothing to act on. ## Changes - [x] Client-facing API Changes - [x] Internal API Changes - [ ] Bug Fixes - [x] New Features - [x] Tests **Client-facing API Changes** - READ_ONLY, nullable `config` field (`Map<String,String>`) on `GetTableResponseBody`, modeled on the Iceberg REST load-table `config`. Namespaced keys (e.g. `openhouse.read-bridge`); clients ignore keys they do not understand. - Why a response field: `doRefresh` already fetches a live `GetTableResponseBody` on every table load (and on commit responses) but discarded all but `getTableLocation()` — it is the natural, already-present, server-controlled, zero-staleness delivery channel. The generated client sets `FAIL_ON_UNKNOWN_PROPERTIES = false`, so a new response field cannot break older clients (and unknown config keys are simply carried). Same additive, nullable, READ_ONLY pattern as `sortOrder` etc. **Internal API Changes** - `OpenHouseTableOperations.doRefresh` keeps the full response and stashes `config` in an `AtomicReference`, exposed to subclasses via the new `protected currentConfig()`. READ_ONLY + side-channel: never sent back on writes. - `TablesMapper` (the table DTO serializer into gettableresponsebody) ignores `config` (`@Mapping(target = "config", ignore = true)`) — it is stamped separately, not sourced from `TableDto`. **New Features** - A flat string map keyed by namespaced keys means new features become new `config` entries rather than an API/schema change or client regen. ## Testing Done - [x] Added new tests. Compile/codegen verified (`:services:tables`, `:client:tableclient`, `:integrations:java:iceberg-1.2:openhouse-java-runtime`); the client regenerates with `getConfig()` returning `Map<String,String>`. `OpenHouseTableOperationsTest` covers: `currentConfig()` null before refresh; `doRefresh` captures/clears `config`; the REST-style string map deserializes from a response and tolerates unknown fields. ## Stack **#644 (channel)** → #645 (read-bridge mechanism) → [li-openhouse #2166](https://github.com/linkedin-multiproduct/li-openhouse/pull/2166) (li column-default source from avro.schema.literal) This is the substrate PR — intentionally behaviorless on its own. ## Next steps (this PR) - [ ] Review + merge first — it is the base of the stack; #645 and #2166 depend on it. This PR delivers only the channel; it stays inert until two further pieces exist, both **required**, and both delivered by the next PR in the stack: - **A server stamp** to populate `config` (it is null until something stamps it) — required, delivered by #645 via `ReadBridgeConfigResolver` + the `ColumnDefaultsSource` seam. - **A client consumer** that reads `currentConfig()` and acts on it — required, delivered by #645's read-bridge metadata overlay, driven by the column defaults that #2166 supplies. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
747c7ef to
112cdab
Compare
112cdab to
2f2fccb
Compare
e6ae89e to
2f66d83
Compare
e96bcf1 to
3df3d5c
Compare
@mkuchenbecker we actually need both. there was a mistake in the PR description, corrected it. |
## Summary Problem: I have a feature which I want to ramp on the server. but I also don't want to prevent table owners from self-serve opting in. A generic function to handle that overlap doesn't exist today. Extend the existing `TableFeatureToggle` with self-service table overrides while preserving its server-managed targeting API. An explicit `<featureId>.enabled=true|false` table property wins. When the property is absent, activation delegates to the server toggle. Server rules now support trailing-`*` prefix matching independently for database and table names. ## Changes - [ ] Client-facing API Changes - [x] Internal API Changes - [ ] Bug Fixes - [x] New Features - [ ] Performance Improvements - [ ] Code Style - [ ] Refactoring - [ ] Documentation - [x] Tests Adds a binary-compatible default method to `TableFeatureToggle`: ```java isFeatureActivatedWithOverride(TableDto tableDto, String featureId) ``` It is deliberately **not** an overload of `isFeatureActivated`. The two carry different safety contracts, and a distinct name makes the difference visible at the call site: authorization gates such as `enable_mor` decide whether a user may write a preserved table property, so they must keep using the server-only `isFeatureActivated(String, String, String)`. The override-honoring form reads a property the gated user can write. An override that is neither `true` nor `false` fails closed: it is logged and the feature is treated as inactive. The gate is evaluated on the table-load path, so throwing would turn a typo like `read-bridge.enabled=flase` into a `400` and make the table unloadable. Extends the existing toggle rule matcher while preserving exact and `*` matching: - `tracking.events` matches exactly. - `tracking_*.events_*` matches database and table prefixes. - `*.*` matches every table. ## Testing Done - [ ] Manually Tested on local docker setup. Please include commands ran, and their output. - [x] Added new tests for the changes made. - [ ] Updated existing tests to reflect the changes made. - [ ] No tests added or updated. Please explain why. If unsure, please feel free to ask for help. - [ ] Some other form of testing like staging or soak time in production. Please explain. Ran: ```shell JAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew \ :services:tables:test \ --tests 'com.linkedin.openhouse.tables.toggle.TableFeatureToggleTest' \ :services:housetables:test \ --tests 'com.linkedin.openhouse.housetables.mock.WildcardTableToggleRuleMatcherTest' \ -x CopyGitHooksTask ``` All 12 focused tests passed, covering server fallback, explicit opt-in and opt-out, fail-closed handling of unparseable overrides, exact matching, wildcard matching, and paired database/table prefix matching. # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [x] Large PR broken into smaller PRs, and PR plan linked in the description. This is an independent OSS foundation for the read-bridge stack in #645 and the corresponding `li-openhouse` implementation PRs. Feature-specific default derivation remains outside this PR. Note for reviewers: the matcher change widens any existing `table_toggle_rule` row whose pattern ends in `*` but is not exactly `*`. Those previously matched nothing. Worth auditing HTS before merge.
Adds the open-source read-bridge feature on top of the per-table `config` channel (linkedin#644): - ColumnDefaultsSource: the single open-source/closed-source seam (field-id -> Iceberg single-value JSON). Open-source default is a no-op lambda in ApiConfig; a deployment overrides this bean (e.g. li-openhouse, from avro.schema.literal). - ReadBridgeConfigResolver: server-side encoder that stamps each default as a flat namespaced config entry (openhouse.read-bridge.column-default.<fieldId> = single-value JSON) — no envelope/POJO; the config map carries the structure. - ReadBridge (client): decodes those entries and applies the overlay at metadata-load time (the column-default transform is a marked TODO, and the place further V3 features get backported). Keeps loadMetadata to one call. Behaviorless until a ColumnDefaultsSource is supplied; fail-closed throughout. Co-authored-by: Cursor <cursoragent@cursor.com>
A known openhouse.read-bridge.column-default.* entry is produced by the server encoder from a typed JsonNode keyed by an integer field-id, so its suffix always parses as an int and its value always round-trips through readTree. A decode failure is therefore an encoder bug or transport corruption, not an expected input -- skipping it would silently read NULL instead of the column's default and hide a real defect. Throw instead. Unknown keys (a newer server feature this client doesn't recognize) are still ignored, preserving forward compatibility. Update javadoc/comments to the encoder round-trip rationale and note the guarantee covers well-formedness, not default-to-schema correctness (a write-time concern). Tests updated to assert fail-loud on bad field-id and unparseable value, plus forward-compat skip of unknown keys.
The fail-loud change updated ReadBridge but left two docs stating the old fail-closed behavior: - ColumnDefaultsSource told implementers they "must never throw", the opposite of the policy, and unimplementable alongside validating that a declared default binds to its column. Restate it as the capability-gap vs invariant-violation split: an empty map means nothing to bridge, while a declared-but-unhonorable default throws. - OpenHouseTableOperations.loadMetadata still claimed "unparseable config leaves the raw metadata untouched", which no longer holds: ReadBridge throws on a malformed known entry and loadMetadata is its only caller. Comment-only; no behavior change. Co-authored-by: Cursor <cursoragent@cursor.com>
3df3d5c to
86c62f6
Compare
mkuchenbecker
left a comment
There was a problem hiding this comment.
Restricting changing the default mitigates a major concern. Is there a problem doing so?
|
@mkuchenbecker we can do that, no problem. I have a follow up PR #678 that prevents schema from evolving to drop the default values. once they exist on table, they are there forever. this is exactly what the upstream datasource has in their contract, as well as Hive. so no change in behavior for customer needed. It is also how the behavior exists in V3: once initial-defaults is set, it cannot be removed or changed while that column is in the table. |
…eberg's retry path (#668) ## Summary > [!IMPORTANT] > **Stacked on #645** — please review that first. Until it merges, the diff > here also shows its commits; the only commit belonging to this PR is > `ReadBridge: mark bridge failures unrecoverable on Iceberg's retry path`. > Once #645 merges this collapses to a small client-runtime + test diff. Draft until then. `loadMetadata` is the loader `BaseMetastoreTableOperations` wraps in `Tasks.retry(20)` with exponential backoff. That retry exists for the metadata **file read**, which fails transiently — a network blip, a file not yet visible — and can succeed on a later attempt. Decode and apply are deterministic: a malformed config fails the same way every attempt. Leaving those failures retryable therefore gave a deterministic error the retry policy of a transient one: - 21 attempts, ~87s of backoff (100 + 400 + 1600 + 5000×17 ms) - 21 re-reads and re-parses of the metadata file from storage - to reproduce the error already available on the first attempt Every reader of the table pays it simultaneously, so one malformed entry becomes a read stall plus a storage read storm instead of a fast, clear error. The retry loop also *loses* the real error: it reports whatever the last attempt threw. ## Changes - [ ] Client-facing API Changes - [x] Internal API Changes - [x] Bug Fixes - [ ] New Features - [ ] Performance Improvements - [ ] Code Style - [x] Refactoring - [ ] Documentation - [x] Tests Keep `from` / `apply` inside `loadMetadata` (decode **before** the file read so a bad config never touches storage), and wrap `IllegalStateException` as Iceberg's `Tasks.UnrecoverableException`. Iceberg already stops `Tasks.retry` on that type, so no `doRefresh` decode field or `AtomicReference<ReadBridge>` is needed. | | when | retryable | |---|---|---| | `ReadBridge.from(config)` | before file IO in `loadMetadata` | no (`UnrecoverableException`) | | metadata file read / parse | inside `loadMetadata` | yes | | `bridge.apply(metadata)` | after file IO in `loadMetadata` | no (`UnrecoverableException`) | ## Testing Done - [ ] Manually Tested on local docker setup. - [x] Added new tests for the changes made. - [x] Updated existing tests to reflect the changes made. - [ ] No tests added or updated. - [ ] Some other form of testing like staging or soak time in production. New regression test `testMalformedConfigFailsBeforeTouchingStorage` asserts a `Tasks.UnrecoverableException` whose cause is `IllegalStateException`, **and** `verifyNoInteractions(mockFileIO)` — failing before any storage access is the property that matters. ```shell JAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew \ :integrations:java:iceberg-1.2:openhouse-java-itest:test \ --tests '*ReadBridge*' \ --tests '*OpenHouseTableOperationsTest.testMalformed*' \ --tests '*OpenHouseTableOperationsTest.testDoRefresh*' ``` ## Additional Information - [ ] Breaking Changes - [ ] Deprecations - [x] Large PR broken into smaller PRs, and PR plan linked in the description. Kept out of #645 deliberately, so that PR stays as reviewed and this defect is reviewable on its own. Downstream stack (rebased onto this tip): #674 → #675.
## Summary [Build and Publish Python](https://github.com/linkedin/openhouse/actions/runs/32384180035) on `main` fails at `make package-check`: ``` InvalidDistribution: Invalid distribution metadata: '2.5' is not a valid metadata version ``` hatchling 1.30 emits Metadata-Version 2.5. Released twine (6.2) and PyPI still only accept 2.4, so the dataloader wheel is built and then rejected. Same failure on the #645 merge; Java publish is unaffected. Cap `[build-system] requires` to `hatchling>=1.27,<1.30`. Lift the cap once twine and PyPI accept 2.5. ## Changes - [ ] Client-facing API Changes - [ ] Internal API Changes - [x] Bug Fixes - [ ] New Features - [ ] Performance Improvements - [ ] Code Style - [ ] Refactoring - [ ] Documentation - [ ] Tests Pin only. No dataloader runtime change. ## Testing Done - [ ] Manually Tested on local docker setup. Please include commands ran, and their output. - [ ] Added new tests for the changes made. - [ ] Updated existing tests to reflect the changes made. - [x] No tests added or updated. Please explain why. If unsure, please feel free to ask for help. - [ ] Some other form of testing like staging or soak time in production. Please explain. Local `uv build` + `uv run --extra dev twine check dist/*`: - wheel METADATA is `Metadata-Version: 2.4` - twine: PASSED on sdist and wheel No unit-test change: this is a build-backend pin, not library behavior. # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [ ] Large PR broken into smaller PRs, and PR plan linked in the description.
…674) ## Summary Depends on [#668](#668) (stacked on [#645](#645)). Move **column-default rollout policy** into OpenHouse so a deployment only supplies the defaults themselves. | Concern | Owner after this PR | |---|---| | Feature id, self-serve property, wire keys | OpenHouse (`ReadBridgeConfigResolver`) | | Per-table ramp / kill | OpenHouse (`TableFeatureToggle` + table property) | | “What are this table’s defaults?” | Deployment (`ColumnDefaultsSource`, e.g. LI ASL) | **Ids (one token, capability-scoped — not bare `read-bridge`):** - Feature id: `read-bridge.column-default` - Self-serve property: `read-bridge.column-default.enabled` - Config prefix: `openhouse.read-bridge.column-default.<fieldId>` ## Resolve flow On get/create/update, `withConfig` calls `ReadBridgeConfigResolver.resolve(tableDto)`: 1. **No source** (`ColumnDefaultsSource.NONE`) → empty config, **no HTS call** 2. **Ramp off** (property `false`, or HTS not `ACTIVE`, or HTS error) → empty config; source not consulted 3. **Ramp on** → ask source → stamp `openhouse.read-bridge.column-default.<fieldId> = <single-value-json>` Self-serve override (`isFeatureActivatedWithOverride`): - property `true` / `false` → table wins, **HTS skipped** - property absent → exact HTS lookup `(databaseId, tableId, featureId)` - unparseable property → fail closed (off) ## Changes - [x] Internal API Changes — `ApiConfig` wires resolver via `ObjectProvider<ColumnDefaultsSource>` (no `@ConditionalOnMissingBean` noop / no `@Primary`); `resolve(TableDto)` only - [x] New Features — capability-specific ramp + self-serve `*.enabled` override; fail-open on toggle lookup errors - [x] Refactoring — `ColumnDefaultsSource` is data-only; ramp is not the deployment’s job - [x] Documentation — javadoc spells exact-match HTS (no glob `*` / `*` today) and when HTS is hit - [x] Tests — `ReadBridgeConfigResolverTest` covers inert/no-source, fail-open, opt-in/out, empty source, stamp, handler `config`; `ReadBridgeColumnDefaultE2ETest` is HTTP create/get with a stub source (property / HTS / fail-closed) ## Why these choices - **Capability-scoped id** — column defaults must ramp independently of future bridges (e.g. deletion vectors). Bare `read-bridge` / `v3-read-bridge` left free for a later superset. - **ObjectProvider → `NONE`** — avoids competing default beans when a deployment registers its source with `@Bean`. - **Fail-open on HTS errors** — safe here because not bridging = today’s NULL reads. Must **not** be copied to capabilities where skipping is incorrect. - **No cluster kill-switch property** — HTS row or table property is the switch (no redeploy). ## Ops notes - HTS match is **exact** `(databaseId, tableId, featureId)` via `BaseTableFeatureToggle`. There is **no** glob `*` / `*` fleet rule yet; fleet ramp means per-table `ACTIVE` rows and/or the table property. - When `read-bridge.column-default.enabled` is **absent**, resolve can hit HouseTables on the **read path**. When the property is present, HTS is skipped. ## Testing Done - [x] Added new tests for the changes made. - [x] Updated existing tests to reflect the changes made. - Local: `./gradlew :services:tables:test --tests '*ReadBridgeConfigResolverTest' --tests '*ReadBridgeColumnDefaultE2ETest'` ## Additional Information - [x] Large PR broken into smaller PRs, and PR plan linked in the description. ### Stack 1. [#645](#645) — substrate (seam + encode + decode hook) 2. [#668](#668) — mark bridge failures unrecoverable on Iceberg’s retry path 3. **This PR** — policy / ramp in OpenHouse 4. [#675](#675) — `ReadBridge.apply` client overlay LI follow-ups (data-only source): [#2204](https://github.com/linkedin-multiproduct/li-openhouse/pull/2204) → [#2203](https://github.com/linkedin-multiproduct/li-openhouse/pull/2203) ### Rollback Plan Revert this PR. Deployments without a `ColumnDefaultsSource` still stamp nothing. Toggle fail-open keeps reads at today’s NULL behavior if HTS is unhealthy.
* Bump linkedin/iceberg 1.5 version to 1.5.2.17 (linkedin#647) ## Summary Picks up com.linkedin.iceberg v1.5.2.17 (linkedin/iceberg#254), which propagates the delete file replication factor to ORC delete files. Adds spark-3.5 sparkitest coverage exercising the configuration through the full OpenHouse stack via SQL: the write.delete-file-replication table property round-trips through the tables service, and merge-on-read DELETE on an ORC table produces ORC position delete files through the replication-aware write path, for both the table property and the spark.sql.iceberg.delete-file-replication session conf routes. Verification is metadata-based (delete_files manifests) because the itest classpath mixes shaded and unshaded iceberg, which breaks ORC position delete reads with a TypeDescription ClassCastException. ## Changes - [ ] Client-facing API Changes - [ ] Internal API Changes - [ ] Bug Fixes - [X] New Features - [ ] Performance Improvements - [ ] Code Style - [ ] Refactoring - [ ] Documentation - [ ] Tests For all the boxes checked, please include additional details of the changes made in this pull request. ## Testing Done <!--- Check any relevant boxes with "x" --> - [x] Manually Tested on local docker setup. Please include commands ran, and their output. - [x] Added new tests for the changes made. - [x] Updated existing tests to reflect the changes made. - [x] No tests added or updated. Please explain why. If unsure, please feel free to ask for help. - [ ] Some other form of testing like staging or soak time in production. Please explain. For all the boxes checked, include a detailed description of the testing done for the changes made in this pull request. # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [ ] Large PR broken into smaller PRs, and PR plan linked in the description. For all the boxes checked, include additional details of the changes made in this pull request. --------- Co-authored-by: Shanthoosh Pazhanjur Venkataraman <pvs@Shanthooshs-MacBook-Air.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Bump WebClient/codec max-in-memory buffer from 20MB to 40MB (linkedin#649) # Problem & Solution Overview The Tables Service's WebClient to the HouseTables Service (HTS) enforces a fixed in-memory buffer limit for decoding responses. `GET /hts/tables/query?databaseId=<db>` returns the full unpaginated table list for a database in a single response. For databases with large table counts (e.g. `u_daajob`, ~30.6k active tables and growing), the aggregate response payload crossed the existing 20MB ceiling, causing: ``` org.springframework.core.io.buffer.DataBufferLimitException: Exceeded limit on max bytes to buffer : 20971520 ``` This bumps the buffer limit from 20MB to 40MB, following the same pattern as linkedin#258 (which bumped 10MB -> 20MB), across all services that configure this limit: - `client/secureclient/.../WebClientFactory.java` - `services/tables/.../MainApplicationConfig.java` - `services/tables/src/main/resources/application.properties` - `services/housetables/src/main/resources/application.properties` - `services/jobs/src/main/resources/application.properties` **Note: this is a temporary mitigation, not a permanent fix.** The underlying `OpenHouseInternalCatalog.listTables()` -> HTS `/hts/tables/query?databaseId=<db>` call path is unpaginated (`findAllByDatabaseIdIgnoreCase`), so response size scales linearly with table count per database. Simply raising the buffer ceiling only buys headroom until the next database crosses it. The durable fix is to paginate this call path (HTS already supports paginated `findAllByFilters`/`listTables(page, size, ...)` for other endpoints) so response size is bounded regardless of database size. # Testing Done - Verified all `IN_MEMORY_BUFFER_SIZE` / `spring.codec.max-in-memory-size` references updated consistently (grepped repo-wide). - No behavior change other than the raised limit; no new tests needed as this mirrors linkedin#258's precedent. - Local pre-commit spotless formatting check passed. **End-to-end local repro against the real code path**, run via `./gradlew dockerUp -Precipe=oh-only` (Tables Service + HouseTables Service, H2 in-memory DB): 1. Exercised the exact production call path from the incident: `POST /v1/databases/{databaseId}/tables/search` (`TablesController.searchTables`) -> `OpenHouseInternalCatalog.listTables()` -> `HouseTableRepositoryImpl.findAllByDatabaseId()` -> HTS `GET /hts/tables/query?databaseId=...`. 2. **Reproduced the failure at small scale**: temporarily set `IN_MEMORY_BUFFER_SIZE` / `spring.codec.max-in-memory-size` to `1MB` (mirroring the pre-fix state's ratio, scaled down for a fast local test), loaded 4,000 synthetic `UserTable` rows (~347 bytes/row observed) into a single database via the HTS `PUT /hts/tables` API, then called the search endpoint. Got the **identical exception** as production: ``` org.springframework.web.reactive.function.client.WebClientResponseException: 200 OK from GET http://openhouse-housetables:8080/hts/tables/query?databaseId=u_loadtest; nested exception is org.springframework.core.io.buffer.DataBufferLimitException: Exceeded limit on max bytes to buffer : 1048576 ``` 3. **Validated the fix**: doubled the buffer to `2MB` (same ratio as this PR's 20MB -> 40MB change), rebuilt only the Tables Service, restarted it (HTS data, and its 4,000 rows, untouched/verified intact), and re-ran the identical search request. Result: `HTTP 200`, all 4,000 rows returned (1.39MB response) -- confirming the buffer bump resolves this exact failure mode. 4. Torn down the stack and removed all scratch containers/images/worktrees afterward; no changes left behind in the shared environment. * Paginate HTS list-tables queries to avoid WebClient buffer limit (linkedin#648) # Problem & Solution Overview `OpenHouseInternalCatalog#listTables(Namespace)` issued a single unpaginated HTS query (`findAllByDatabaseId` / `findAll`) to list every table in a database. For databases with a large number of tables, the serialized HTS response can exceed the WebClient in-memory buffer limit configured in `MainApplicationConfig`, causing a `DataBufferLimitException` that surfaces to callers as an HTTP 500 on `GET /tables?databaseId=...`. This was observed in production for a database that grew past ~30k active tables — the per-table JSON payload (dominated by `metadataLocation` path length) pushed the total response size over the (then 20MB) ceiling. **Relationship to linkedin#649**: linkedin#649 bumped the buffer limit 20MB -> 40MB as a stopgap mitigation, explicitly noting in its description that this is temporary and the durable fix is to paginate this call path. This PR is that durable fix: `listTables(Namespace)` now internally pages through HTS using the already-existing paginated repository methods (`findAll(Pageable)` / `findAllByDatabaseId(databaseId, Pageable)`, page size 1000), accumulating all pages before returning. This bounds every individual HTTP response regardless of how many tables a database has -- independent of whatever the buffer ceiling is set to -- while preserving the existing `List<TableIdentifier>` return type and behavior of the unpaginated API. # Testing Done - `./gradlew :iceberg:openhouse:internalcatalog:compileJava :services:tables:compileJava` — compiles cleanly. - `./gradlew :iceberg:openhouse:internalcatalog:test` — all tests pass (11/11 in `OpenHouseInternalCatalogTest`, including 4 new tests covering single-page, multi-page, empty-database, and empty-namespace pagination paths), no regressions. - **End-to-end local repro against the real code path**, run via Docker Compose (`infra/recipes/docker-compose/oh-only`, Tables Service + HouseTables Service, H2 in-memory DB): 1. Built `services:tables`/`services:housetables` jars from this branch and rebuilt the Docker images (Dockerfiles copy prebuilt jars, so this step is required to pick up code changes). 2. Seeded 55,000 synthetic `UserTable` rows into a single database directly via the HTS `PUT /hts/tables` API (bypassing full Iceberg table creation), with realistic `metadataLocation` path lengths. 3. Called `POST /v1/databases/{databaseId}/tables/search` (`TablesController.searchTables` -> `OpenHouseInternalCatalog.listTables`) against the **pre-fix** jars first: reproduced the exact production failure, `DataBufferLimitException: Exceeded limit on max bytes to buffer : 20971520`. 4. Re-ran the identical request against the **post-fix** jars: `HTTP 200`, all 55,000 results returned (18.3MB response, aggregated internally over 55 paginated HTS calls of 1000 rows each) -- confirming the fix resolves the failure regardless of buffer size. - Risk: low — change is isolated to internal list-tables aggregation logic; public method signature and semantics are unchanged. * Reject CREATE OR REPLACE (RTAS) on a locked table (linkedin#650) ## Summary `CREATE OR REPLACE ... AS SELECT` (RTAS) is not blocked on a locked table. The table lock is enforced only on the normal write/update path: in `IcebergSnapshotsServiceImpl.putIcebergSnapshots` and the new condition added for RTAS never checks a locked table. SOP should be to unlock the table before RTAS. ## Fix Check `isTableLocked` before the replace-vs-update split so it applies to both paths. A locked table now rejects RTAS with `LOCKED_TABLE_OPERATION` (HTTP 400), consistent with a normal write. ## Testing Done - **Unit:** `IcebergSnapshotsServiceTest.testReplaceCommitOnLockedTableThrowsException` — a replace commit on a locked table throws `UnsupportedClientOperationException`. Full class passes. - **Integration (e2e):** `SnapshotsControllerTest.testReplaceCommitRejectedOnLockedTable` — creates a table, locks it via REST, attempts RTAS, and asserts HTTP 400 with a "locked state" message. - `./gradlew :services:tables:test` (JDK 17) targeted runs pass; Spotless clean on the module. Integration tests will be added in li-openhouse to follow. --------- Co-authored-by: mkuchenbecker <mkuchenbecker@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix RENAME COLUMN silent no-op (linkedin#651) ## Summary `ALTER TABLE ... RENAME COLUMN` is a silent no-op: it returns success but the column keeps its old name. `BaseIcebergSchemaValidator.normalizeSchemaCasingToTable` rewrites every write-schema field name back to the table's spelling by Iceberg field id — including for a genuine rename — so the write schema compares equal to the table schema, `sameSchema` short-circuits, and schema validation is skipped. The rename is dropped without any error. This is a regression from linkedin#558, which added the casing normalizer to support case-insensitive writes. ## Fix Only normalize when the write name differs from the table name by case alone (`equalsIgnoreCase`). A genuine rename is left intact, so it flows to `validateWriteSchema` and is rejected loudly (`InvalidSchemaEvolutionException`) instead of being silently reverted. Case-insensitive writes are preserved. ## Testing Done - **Manual:** - Darwin was used to manually spot check the issue exists in prod. - **Unit:** `BaseIcebergSchemaValidatorTest.normalizeSchemaCasingToTable_preservesGenuineRename_notRevertedToOldName` - **Integration (e2e):** `RepositoryTest.testColumnRenameIsRejectedNotSilentlyDropped` — a rename through `save()` now throws `InvalidSchemaEvolutionException` Future: - li-openhouse Integration tests --------- Co-authored-by: mkuchenbecker <mkuchenbecker@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Revert "Paginate HTS list-tables queries to avoid WebClient buffer limit (linkedin#648)" (linkedin#665) # Problem & Solution Overview This reverts commit e80d45d (linkedin#648). linkedin#648 changed `OpenHouseInternalCatalog#listTables(Namespace)` to internally paginate HTS "list all" queries (page size 1000) instead of issuing a single unpaginated request, in order to avoid `DataBufferLimitException` on very large databases. In production, this caused `GET /v1/databases` (empty-namespace list-tables path) to fan out into many sequential HTS calls per request instead of one. For a deployment with a large number of databases/tables, this significantly increased request volume against HTS, saturating its JDBC connection pool (Hikari) and causing collateral failures on unrelated HTS calls (e.g. `HikariPool-1 - Connection is not available, request timed out after 30000ms`, `java.io.EOFException: connection was unexpectedly lost`, `PrematureCloseException: Connection prematurely closed BEFORE response`). This PR reverts the pagination change to restore the prior unpaginated `listTables` behavior while a safer fix (e.g. bounded concurrency, smarter page sizing, or pushing pagination to the client-facing paginated APIs only) is designed. # Testing Done - `git revert e80d45d` applied cleanly with no conflicts against current `main` (two unrelated commits, linkedin#650 and linkedin#651, merged on top since linkedin#648; neither touches this file). - `./gradlew :iceberg:openhouse:internalcatalog:compileJava` compiles cleanly after the revert. - Risk: low — this is a straight revert of a recent, isolated change back to previously-running production behavior (pre-linkedin#648). Follow-up work is needed to re-address the original `DataBufferLimitException` motivating linkedin#648, without triggering HTS connection-pool exhaustion. * Honor table feature toggles during creation (linkedin#664) ## Summary Table creation filtered feature-gated preserved properties because `allowKeyInCreation` called the advised `isKeyPreservedForTable` method through Spring self-invocation, bypassing the feature-toggle aspect. Invoke the table-aware check from the repository's Spring proxy before applying the create-only allowlist fallback. This makes CREATE consistent with ALTER while retaining support for extension-defined properties that are writable only during creation. ## Changes - [x] Bug Fixes - [x] Tests - Evaluate `isKeyPreservedForTable` through the proxied `PreservedKeyChecker` during creation. - Retain `allowKeyInCreation` as the fallback for create-only preserved properties. - Add a Spring integration regression test using a real active table feature toggle. - Update the default-file-format test to model preserved/toggled state on the table-aware method. ## Testing Done - [x] Added new tests for the changes made. - [x] Updated existing tests to reflect the changes made. - [x] Some other form of testing like staging or soak time in production. Please explain. - Verified the new regression test fails without the production change and passes with it. - `:services:tables:test` — 463 tests passed. - `:services:tables:spotlessCheck` - `:services:tables:checkstyleMain` - `:services:tables:checkstyleTest` - Manually reproduced the original behavior against a live OpenHouse namespace: feature-gated properties survived ALTER but were silently filtered during CREATE. # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [ ] Large PR broken into smaller PRs, and PR plan linked in the description. * Add Renovate to sync linkedin/iceberg 1.2.x and 1.5.x lines (linkedin#635) ## Summary This adds **Renovate** — an open-source tool that watches dependency versions and opens a pull request when a newer one is available — and points it at the LinkedIn build of Iceberg (`com.linkedin.iceberg`, the table library OpenHouse depends on). A scheduled job runs it every hour on GitHub's own machines. OpenHouse uses two versions of this library at the same time: one in the `1.2.x` series (any version starting with `1.2.`) and one in the `1.5.x` series. The tool keeps each series on its own newest version and never lets one cross into the other. First-run result, confirmed locally without opening any pull requests: - `iceberg-core`: `1.2.0.17` → `1.2.0.18` - `iceberg-core`: `1.5.2.11` → `1.5.2.15` ## Changes - [x] New Features - [x] Documentation - `.github/renovate.json` — the rules above. - `.github/workflows/renovate.yml` — the hourly scheduled job. - `docs/development/renovate-iceberg-sync.md` — setup and reasoning. ## Testing Done - [x] No tests added or updated. Ran Renovate locally in a mode that reads the repo and looks up versions but opens no pull requests; it proposed exactly the two updates above, each staying inside its own series. Also checked `renovate.json` with Renovate's official config validator. ## Additional Information - [ ] Breaking Changes - [ ] Deprecations **Optional:** adding a repository secret named `RENOVATE_TOKEN` lets the update pull requests run their tests automatically. GitHub will not start test runs on a pull request opened with a workflow's built-in credentials (a guard against workflows triggering each other), so without this secret the pull requests still open but you start their tests by hand. A personal access token or a GitHub App token works. It is not required to merge this change. This covers the two main version numbers only. The separately pinned `iceberg-aws` (`1.2.0.6`) is left out on purpose and can be added the same way later. * Add self-service overrides to table feature toggles (linkedin#666) ## Summary Problem: I have a feature which I want to ramp on the server. but I also don't want to prevent table owners from self-serve opting in. A generic function to handle that overlap doesn't exist today. Extend the existing `TableFeatureToggle` with self-service table overrides while preserving its server-managed targeting API. An explicit `<featureId>.enabled=true|false` table property wins. When the property is absent, activation delegates to the server toggle. Server rules now support trailing-`*` prefix matching independently for database and table names. ## Changes - [ ] Client-facing API Changes - [x] Internal API Changes - [ ] Bug Fixes - [x] New Features - [ ] Performance Improvements - [ ] Code Style - [ ] Refactoring - [ ] Documentation - [x] Tests Adds a binary-compatible default method to `TableFeatureToggle`: ```java isFeatureActivatedWithOverride(TableDto tableDto, String featureId) ``` It is deliberately **not** an overload of `isFeatureActivated`. The two carry different safety contracts, and a distinct name makes the difference visible at the call site: authorization gates such as `enable_mor` decide whether a user may write a preserved table property, so they must keep using the server-only `isFeatureActivated(String, String, String)`. The override-honoring form reads a property the gated user can write. An override that is neither `true` nor `false` fails closed: it is logged and the feature is treated as inactive. The gate is evaluated on the table-load path, so throwing would turn a typo like `read-bridge.enabled=flase` into a `400` and make the table unloadable. Extends the existing toggle rule matcher while preserving exact and `*` matching: - `tracking.events` matches exactly. - `tracking_*.events_*` matches database and table prefixes. - `*.*` matches every table. ## Testing Done - [ ] Manually Tested on local docker setup. Please include commands ran, and their output. - [x] Added new tests for the changes made. - [ ] Updated existing tests to reflect the changes made. - [ ] No tests added or updated. Please explain why. If unsure, please feel free to ask for help. - [ ] Some other form of testing like staging or soak time in production. Please explain. Ran: ```shell JAVA_HOME=$(/usr/libexec/java_home -v 17) ./gradlew \ :services:tables:test \ --tests 'com.linkedin.openhouse.tables.toggle.TableFeatureToggleTest' \ :services:housetables:test \ --tests 'com.linkedin.openhouse.housetables.mock.WildcardTableToggleRuleMatcherTest' \ -x CopyGitHooksTask ``` All 12 focused tests passed, covering server fallback, explicit opt-in and opt-out, fail-closed handling of unparseable overrides, exact matching, wildcard matching, and paired database/table prefix matching. # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [x] Large PR broken into smaller PRs, and PR plan linked in the description. This is an independent OSS foundation for the read-bridge stack in linkedin#645 and the corresponding `li-openhouse` implementation PRs. Feature-specific default derivation remains outside this PR. Note for reviewers: the matcher change widens any existing `table_toggle_rule` row whose pattern ends in `*` but is not exactly `*`. Those previously matched nothing. Worth auditing HTS before merge. * [dataloader] Improve exception handling (linkedin#654) ## Summary **Problem:** OpenHouse DataLoader errors could not be reliably correlated with the corresponding Tables Service request. Timestamp-based correlation is noisy, and authentication failures were represented as generic `OSError`s and retried as if they were transient I/O failures. **Solution:** assign a unique `X-Request-ID` to every outbound catalog HTTP request and expose it through shared, typed catalog exceptions. The exception hierarchy centralizes request-ID formatting and distinguishes authentication, authorization, not-found, transport, HTTP, and malformed-response failures. ## Changes - [x] Client-facing API Changes - [x] Internal API Changes - [x] Bug Fixes - [ ] New Features - [ ] Performance Improvements - [ ] Code Style - [x] Refactoring - [ ] Documentation - [x] Tests Details: - Added a central request-aware HTTP session that generates a fresh UUID for every prepared request. - Added shared exception types: - `OpenHouseCatalogError` - `OpenHouseRequestError` - `OpenHouseTransportError` - `OpenHouseHTTPError` - `OpenHouseAuthenticationError` - `OpenHouseAuthorizationError` - `OpenHouseNoSuchTableError` - `OpenHouseInvalidResponseError` - Centralized `X-Request-ID` rendering and exposed it as `exception.request_id`. - Preserved compatibility with callers catching PyIceberg `NoSuchTableError`. - Classified 401/403 and other non-transient 4xx failures as non-retryable. - Kept transport failures, HTTP 408/429, and 5xx responses retryable. ## Testing Done - [ ] Manually Tested on local docker setup. Please include commands ran, and their output. - [x] Added new tests for the changes made. - [x] Updated existing tests to reflect the changes made. - [ ] No tests added or updated. Please explain why. If unsure, please feel free to ask for help. - [x] Some other form of testing like staging or soak time in production. Please explain. Validation performed: - `make verify` - Ruff lint passed - Ruff formatting passed - Mypy passed - **272 unit tests passed** - Added coverage for unique request IDs, typed 401/403/404/500 errors, transport errors, malformed JSON, missing/empty `tableLocation`, retryable failures, and non-retryable authentication failures. - Used a known request ID against the deployed OpenHouse route and confirmed it appeared in the Nginx access log; follow-up proxy configuration work is tracked separately to preserve the same value through Ambassador. # Additional Information - [ ] Breaking Changes - [ ] Deprecations - [ ] Large PR broken into smaller PRs, and PR plan linked in the description. * Surgical scheduler logs for jobs observability (phase 1.5) (linkedin#646) ## Summary Phase 1.5 only from `jobs-observability-plan.md` §10 — localized log lines, no new classes or entry-point changes: - **`OH_SCHED_START`** in `JobsScheduler.main`: echoes `numParallelJobs`, poller/submitter counts, poll/timeout settings (replaces brittle `values.yaml case()` for cap/deadline KQL). - **`OH_SCHED_ELIGIBLE`** in `OperationTasksBuilder`: stable eligible-count token alongside existing `metadata fetched count` log. - **Queued-timeout poll exit** in `OperationTask`: appends `lastObservedState` + `executionId` to distinguish genuine GGW queue timeout vs poll-lag give-up on terminal jobs. ~35 lines across 3 files. Phase 2 (OTEL gauges, heartbeat sampler, DLQ counters) intentionally deferred. ## Test plan - [x] Compiles; no test signature changes - [ ] Post OSS→LI bump: grep cron pod logs for `OH_SCHED_START`, `OH_SCHED_ELIGIBLE`, `lastObservedState=` on queued-timeout lines Co-authored-by: Cursor <cursoragent@cursor.com> * Update dependency com.linkedin.iceberg:iceberg-core to v1.2.0.20 (linkedin#667) This PR contains the following updates: | Package | Change | [Age](https://docs.renovatebot.com/merge-confidence/) | [Confidence](https://docs.renovatebot.com/merge-confidence/) | |---|---|---|---| | [com.linkedin.iceberg:iceberg-core](https://redirect.github.com/linkedin/iceberg) | `1.2.0.19` → `1.2.0.20` |  |  | --- ### Release Notes <details> <summary>linkedin/iceberg (com.linkedin.iceberg:iceberg-core)</summary> ### [`v1.2.0.20`](https://redirect.github.com/linkedin/iceberg/releases/tag/v1.2.0.20) [Compare Source](https://redirect.github.com/linkedin/iceberg/compare/v1.2.0.19...v1.2.0.20) <sup><sup>*Changelog generated by [Shipkit Changelog Gradle Plugin](https://redirect.github.com/shipkit/shipkit-changelog)*</sup></sup> ##### 1.2.0.20 - 2026-07-31 - [0 commit(s)](https://redirect.github.com/linkedin/iceberg/compare/v1.2.0.20...v1.2.0.20) by - No notable improvements. No pull requests (issues) were referenced from commits. </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://redirect.github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4yODguMCIsInVwZGF0ZWRJblZlciI6IjQzLjI4OC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Merge existing policies on CREATE OR REPLACE (RTAS) (linkedin#652) ## Summary `CREATE OR REPLACE ... AS SELECT` (RTAS) silently dropped the table's policies. The replace path rebuilt the `policies` table property purely from the incoming request, so a replace that omitted policies wiped the existing retention, sharing, PII column tags, replication, and history, even though ordinary user table properties survived. ## Fix Policies are table metadata that a replace must not silently drop. Before the replace properties are built, the existing table's policies are merged with the request's policies. The merge is based on the existing policies, so any plane that the request does not explicitly provide is carried forward from the existing table, and each plane that the request does provide overrides the existing value. ## Merge behavior The table below describes how each policy plane behaves during a replace. | Plane | When the request provides it | When the request omits it | |---|---|---| | `retention` | The request value is applied. | The existing value is carried forward. | | `replication` | The request value is applied. | The existing value is carried forward. | | `history` | The request value is applied. | The existing value is carried forward. | | `lockState` | The request value is applied. | The existing value is carried forward. | | `columnTags` | The request map replaces the existing map in full. This is an overwrite, not a per-key merge. | The existing map is carried forward. An omitted field and an empty map are treated the same way. | | `sharingEnabled` | Not applicable, because this is a primitive boolean and a provided value cannot be distinguished from an omitted one. | The existing value is carried forward. | Two consequences follow from this behavior. First, because column tags use overwrite semantics and an empty map is treated the same as an omitted field, a replace cannot clear all column tags. Clearing tags is done with `ALTER TABLE ... MODIFY COLUMN ... UNSET TAG`. Second, because `sharingEnabled` is a primitive boolean with no unset state, its value is always preserved across a replace. Sharing is changed with `ALTER TABLE ... SET POLICY (SHARING=...)`. Spark RTAS has no policy clause, so it always sends a request with no policies, and the entire existing policies object is carried forward unchanged. A partial policy payload can only arrive from a client that calls the REST API directly. This behavior is consistent with the intent of RTAS, which should preserve table properties so that a replace does not require re-granting access to the same entity. ## Testing Done The REST level partial payload path is exercised through `RepositoryTest`, which is the layer that can send a partial `Policies` object. Spark cannot reach this path because it always sends a request with no policies. - `testReplaceMergesExistingPolicies` replaces a table without policies and asserts that the retention policy survives. - `testReplaceAppliesRequestedPolicies` asserts that a retention policy provided on the request is applied. - `testReplaceWithPartialPoliciesPreservesSharing` sends a partial payload containing only retention and asserts that `sharingEnabled` stays true while the new retention is applied. - `testReplaceWithPartialPoliciesPreservesOmittedPlanes` overrides only retention and asserts that the omitted history plane is carried forward. - `testReplaceWithPartialPoliciesPreservesColumnTags` sends a payload that provides retention but omits column tags, and asserts that the existing column tag is carried forward. - `testReplaceOverwritesColumnTags` sends a new column tag map and asserts that it replaces the existing map in full, dropping the previous tag. Black box coverage is exercised through `RtasPolicyPreservationTest` against an embedded OpenHouse server driven by Spark SQL. It asserts that retention, sharing, the PII column tag, and history all survive a `REPLACE TABLE ... AS SELECT`. The existing `SnapshotsControllerTest.testPutSnapshotsReplaceCommit` still passes, which confirms that a replace on a table that never had policies still yields none. `./gradlew :services:tables:test` and `:integrations:spark:spark-3.1:openhouse-spark-itest:catalogTest` pass on JDK 17, and Spotless is clean on the module. --------- Co-authored-by: mkuchenbecker <mkuchenbecker@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add metrics for HCR misconfigured tables (linkedin#672) * [default values] Tables: apply server-supplied per-column defaults at read time (linkedin#645) ## Summary Adds the open-source read-bridge feature on top of the per-table `config` channel (linkedin#644): the OH server stamps per-column initial-defaults onto `GetTableResponseBody.config`, and the Java client overlays them at metadata-load time so a column added after data exists reads its declared default instead of NULL (a v2→v3 read-time bridge). The whole feature is open source; it exposes exactly **one** deployment seam: - **`ColumnDefaultsSource`** (interface) — the single open-source/closed-source line: `field-id -> Iceberg single-value JSON`. The open-source default is a no-op lambda in `ApiConfig`; a deployment overrides this bean (li-openhouse derives values from `avro.schema.literal`). - **`ReadBridgeConfigResolver`** (server) — stamps each default as a flat, namespaced config entry: `openhouse.read-bridge.column-default.<fieldId> = <single-value-json>`. No envelope/POJO — the REST `config` string map carries the structure directly. - **`ReadBridge`** (client) — decodes those entries and applies the read-time overlay in `OpenHouseTableOperations.loadMetadata` (the `withInitialDefault`/`withSchemaOverlay` transform is a marked TODO). `loadMetadata` stays one delegating call. ## Scope (engines) - **Spark 3.1 (iceberg-1.2) and Spark 3.5 (iceberg-1.5): both covered by this PR.** The client code lives under `integrations/java/iceberg-1.2/openhouse-java-runtime`, but it is not 1.2-only: the 1.5 runtime compiles the same source, because its `build.gradle` adds the 1.2 module's source dirs to its own sourceSet (`srcDirs += project(':integrations:java:iceberg-1.2:openhouse-java-runtime').sourceSets.main.java.srcDirs`). So `ReadBridge` and the `loadMetadata` hook are built into both runtimes, and there is no separate 1.5 port. Neither line applies Iceberg v3 `initial-default` natively, so both need the bridge. - **Flink: separate follow-up PR** (if/when a Flink read path needs the overlay). ## Stack linkedin#644 (config channel, merged) → **linkedin#645 (read-bridge mechanism)** → [li-openhouse #2204](https://github.com/linkedin-multiproduct/li-openhouse/pull/2204) (the `ColumnDefaultsSource` seam) → [li-openhouse #2203](https://github.com/linkedin-multiproduct/li-openhouse/pull/2203) (derive the defaults from `avro.schema.literal`) Rebased onto `main` now that linkedin#644 has merged, so the diff is the read-bridge change only. ## Next steps (this PR) - [ ] **Bump the linkedin/iceberg forks so the overlay APIs exist on both lines.** Verified with `javap` against the pinned jars: `TableMetadata.withSchemaOverlay` is absent from `1.2.0.19`, and `1.5.2.15` has neither `withSchemaOverlay` nor `NestedField.initialDefault` (the backport linkedin#642 brought to the 1.2 line). Nothing in this PR uses them — the decode path is only Jackson + `TableMetadata` — but `ReadBridge.apply` cannot be implemented until they land. - [ ] Implement the overlay in `ReadBridge.apply`. It must cover **every** schema-id in `schemasById` that carries a bridged field-id, not just the current schema: Iceberg resolves a scan's schema from the snapshot's own `schemaId` (`SnapshotUtil.schemaFor`), so time-travel, tag, and non-main-branch reads would otherwise return NULL while latest reads return the default. `withSchemaOverlay` takes a multi-schema map for exactly this. ## Testing Done - `:services:tables` + iceberg-1.2 runtime compile (JDK 17), and repo-wide `spotlessCheck` green. - `ReadBridgeConfigResolverTest` (server): each default round-trips as a flat `openhouse.read-bridge.column-default.<fieldId>` entry; empty when the source supplies nothing. - `ReadBridgeTest` (client): decodes the flat entries by field-id; **fails loud** (`IllegalStateException`, with the offending `key=value`) on a malformed *known* `column-default.*` entry, since the server encoder guarantees an int field-id and a value that round-trips through `readTree` — so a decode failure is an encoder bug or transport corruption, not an expected input. *Unknown* keys are ignored, preserving forward compatibility. - `OpenHouseTableOperationsTest`: config capture and deserialization. --------- Co-authored-by: Cursor <cursoragent@cursor.com> * Apply spotless formatting after upstream merge Mechanical reformat of two fork-only files under the merged toolchain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011y5X1T3FiUozZFLwAmkw19 --------- Co-authored-by: shanthoosh <santhoshvenkat1988@gmail.com> Co-authored-by: Shanthoosh Pazhanjur Venkataraman <pvs@Shanthooshs-MacBook-Air.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Vishnu Kamana <vkamana@linkedin.com> Co-authored-by: mkuchenbecker <mkuchenbecker@users.noreply.github.com> Co-authored-by: Christian Bush <chrisbush747@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Levi Jiang <jjiang1110@gmail.com>
#678) ## Summary Depends on [#674](#674) (policy), stacked on [#668](#668) → [#645](#645). [#679](#679) (`ReadBridge.apply`) restacks on this PR. Read-bridge overlays must not persist. This PR puts the drop on the **server** *before* apply lands, so there is no deployable cut where overlays are applied but not stripped. Default-aware clients send `initial-default` on stamped field-ids (the handshake). The server gates Type 1 / Type 2, then strips those keys before Iceberg commit. Unstamped ids keep the writer's defaults. Until #679, apply is still a no-op; PUTs only carry `initial-default` if the writer set it. The server is already safe: drop is a no-op when the handshake is absent, and Type 2 fail-closes unaware rewrites on ramped tables. ## How strip protection works | Gate | When | Result | |---|---|---| | Type 1 | Ramped table; a previously stamped field-id is still in the schema but missing from incoming stamps | 400 `COLUMN_DEFAULT_REMOVED` | | Type 2 | Ramped rewrite (`replace` / `overwrite` on main) while previous stamps are nonempty | 400 `COLUMN_DEFAULT_REWRITE` unless each remaining id's `initial-default` JSON equals the stamp | | Unusable | Ramped write; source throw / unreadable schema / unreadable snapshots | 400 `COLUMN_DEFAULT_UNUSABLE` | | Drop | After gates pass | Remove `initial-default` on stamped ids (same `findParents("id")` walk as the client) | Unramped tables are a no-op. GET `resolve()` still fail-opens; only the write path is fail-closed. Awareness is `JsonNode.equals` against the stamped config, not key presence. OSS never parses ASL. `ColumnDefaultsSource` is the seam (`NONE` in OSS); LinkedIn fills it. Type 1 compares stamped field-id sets from the resolver. ## Changes - [x] New Features — `ReadBridgeStripProtection.prepare` on PUT table / snapshots - [x] Internal API Changes — resolver write APIs `stampedColumnDefaults` / `isRampedForCommit` - [x] Tests — unit Type 1/2/unusable, mock bean for `MockTablesApplication`, HTTP e2e that GET `initialDefault()` is null after overlay PUT ## Testing Done - [x] Added new tests for the changes made. - [x] Updated existing tests to reflect the changes made. - Local (Java 17): `./gradlew :services:tables:test --tests '*ReadBridgeStripProtectionTest' --tests '*ReadBridgeConfigResolverTest' --tests '*ReadBridgeColumnDefaultE2ETest'` ## Additional Information - [x] Large PR broken into smaller PRs, and PR plan linked in the description. ### Stack 1. [#645](#645) — substrate (seam + encode + decode hook) 2. [#668](#668) — decode off Iceberg’s retry path 3. [#674](#674) — policy / ramp in OpenHouse 4. **This PR** — handshake on PUT, Type 1/2, drop before persist 5. [#679](#679) — `ReadBridge.apply` (merge after this PR) 6. [#681](#681) — Spark catalog itest (overlay) ### Rollback Plan Revert this PR. After #679, clients send overlays; without the server drop they persist in Iceberg metadata.
Summary
Adds the open-source read-bridge feature on top of the per-table
configchannel (#644): the OH server stamps per-column initial-defaults ontoGetTableResponseBody.config, and the Java client overlays them at metadata-load time so a column added after data exists reads its declared default instead of NULL (a v2→v3 read-time bridge).The whole feature is open source; it exposes exactly one deployment seam:
ColumnDefaultsSource(interface) — the single open-source/closed-source line:field-id -> Iceberg single-value JSON. The open-source default is a no-op lambda inApiConfig; a deployment overrides this bean (li-openhouse derives values fromavro.schema.literal).ReadBridgeConfigResolver(server) — stamps each default as a flat, namespaced config entry:openhouse.read-bridge.column-default.<fieldId> = <single-value-json>. No envelope/POJO — the RESTconfigstring map carries the structure directly.ReadBridge(client) — decodes those entries and applies the read-time overlay inOpenHouseTableOperations.loadMetadata(thewithInitialDefault/withSchemaOverlaytransform is a marked TODO).loadMetadatastays one delegating call.Scope (engines)
integrations/java/iceberg-1.2/openhouse-java-runtime, but it is not 1.2-only: the 1.5 runtime compiles the same source, because itsbuild.gradleadds the 1.2 module's source dirs to its own sourceSet (srcDirs += project(':integrations:java:iceberg-1.2:openhouse-java-runtime').sourceSets.main.java.srcDirs). SoReadBridgeand theloadMetadatahook are built into both runtimes, and there is no separate 1.5 port. Neither line applies Iceberg v3initial-defaultnatively, so both need the bridge.Stack
#644 (config channel, merged) → #645 (read-bridge mechanism) → li-openhouse #2204 (the
ColumnDefaultsSourceseam) → li-openhouse #2203 (derive the defaults fromavro.schema.literal)Rebased onto
mainnow that #644 has merged, so the diff is the read-bridge change only.Next steps (this PR)
javapagainst the pinned jars:TableMetadata.withSchemaOverlayis absent from1.2.0.19, and1.5.2.15has neitherwithSchemaOverlaynorNestedField.initialDefault(the backport Bump linkedin/iceberg 1.2 to 1.2.0.19 (NestedField column-default APIs) #642 brought to the 1.2 line). Nothing in this PR uses them — the decode path is only Jackson +TableMetadata— butReadBridge.applycannot be implemented until they land.ReadBridge.apply. It must cover every schema-id inschemasByIdthat carries a bridged field-id, not just the current schema: Iceberg resolves a scan's schema from the snapshot's ownschemaId(SnapshotUtil.schemaFor), so time-travel, tag, and non-main-branch reads would otherwise return NULL while latest reads return the default.withSchemaOverlaytakes a multi-schema map for exactly this.Testing Done
:services:tables+ iceberg-1.2 runtime compile (JDK 17), and repo-widespotlessCheckgreen.ReadBridgeConfigResolverTest(server): each default round-trips as a flatopenhouse.read-bridge.column-default.<fieldId>entry; empty when the source supplies nothing.ReadBridgeTest(client): decodes the flat entries by field-id; fails loud (IllegalStateException, with the offendingkey=value) on a malformed knowncolumn-default.*entry, since the server encoder guarantees an int field-id and a value that round-trips throughreadTree— so a decode failure is an encoder bug or transport corruption, not an expected input. Unknown keys are ignored, preserving forward compatibility.OpenHouseTableOperationsTest: config capture and deserialization.