Repository navigation
Expose Public API for Iceberg Side Input Cache - #40140
jrmccluskey merged 7 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #40140 +/- ##
============================================
+ Coverage 55.09% 58.42% +3.33%
- Complexity 1722 13592 +11870
============================================
Files 1076 2577 +1501
Lines 171812 269247 +97435
Branches 1262 11076 +9814
============================================
+ Hits 94652 157314 +62662
- Misses 74891 105982 +31091
- Partials 2269 5951 +3682
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Assigning reviewers: R: @chamikaramj for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
…erg/IcebergIO.java Co-authored-by: Ahmed Abualsaud <65791736+ahmedabu98@users.noreply.github.com>
| } | ||
|
|
||
| PCollectionView<Map<String, SerializableTableSpec>> metadataView = null; | ||
| if (getUsingSideInputTableCache()) { |
There was a problem hiding this comment.
Can we also throw if the sub-options are set without side-input enabled?
| .apply( | ||
| "EvolveSchemaMidExecution", | ||
| ParDo.of(new EvolveSchemaMidExecutionDoFn(catalogConfig, tableId.toString()))) | ||
| .setRowSchema(v2BeamSchema); |
There was a problem hiding this comment.
This updates the table before the record reaches the sink or the metadata view step, so TableMetadataDriver's first poll is still after the update happens.
We'd need an initial record to pass through, then a second record that triggers the update.
| if (table.schema().findField("city") == null) { | ||
| table.updateSchema().addColumn("city", Types.StringType.get()).commit(); | ||
| } |
There was a problem hiding this comment.
I'm realizing we can't really use schema evolution to prove this because PCollection schemas are fixed. We won't be able to test with two different rows
We can use partition spec evolution (e.g. table.updateSpec().addField("city").commit();). And validate after the pipeline runs by loading the table and verifying that:
- two DataFiles were written (one for each row)
- the first DataFile has the old spec (unpartitioned)
- the second DataFile has the new spec (e.g. "city")
Can use this to check added files:
SnapshotChanges.builderFor(table).build().addedDataFiles();
There was a problem hiding this comment.
Okay I think that's updated correctly
…#40248) * Expose Public API for Iceberg Side Input Cache (#40140) * Expose Public API for Iceberg Side Input Cache * Update sdks/java/io/iceberg/src/main/java/org/apache/beam/sdk/io/iceberg/IcebergIO.java Co-authored-by: Ahmed Abualsaud <65791736+ahmedabu98@users.noreply.github.com> * Streamline API, remove redundant checks * Check polled table metric * Evolve Iceberg schema mid-execution in streaming side input cache test * Throw if sub-options are enabled without side input cache enabled * test with partition spec evolution --------- Co-authored-by: Ahmed Abualsaud <65791736+ahmedabu98@users.noreply.github.com> * Address wrap-up items for Iceberg side input table cache (#40222) * Address wrap-up items for Iceberg side input table cache - Bind historical sort orders unchecked via SortOrderParser.fromJson(schema, node, getOrderId()) in SerializableTableSpec. - Expose Hadoop Configuration via IcebergCatalogConfig.getHadoopConfiguration() and propagate to SerializableTableSpec.getFileIO(conf) through SideInputTable in write transforms. - Add worker-local transient Guava cache to ExtractTableIdsDoFn to eliminate hot-key shuffle bottlenecks on streaming writes. - Add comprehensive unit and integration tests. * Propagate Clock to ExtractTableIdsDoFn for deterministic cache expiration in tests - Ensure TableMetadataDriver passes its Clock to ExtractTableIdsDoFn so virtual-time advancement in tests (e.g. testUnusedTablesEvictedFromStreamingCache) correctly expires worker-local table ID caches instead of using wall-clock time. - Fixes testUnusedTablesEvictedFromStreamingCache flakiness on CI environments with limited concurrency. * Make streaming refresh and spec evolution tests robust against worker cache TTL - Advance ControllableTestClock on trigger rows in testMetadataRefreshedAcrossIntervals, testMetadataRefreshedAcrossIntervalsAsSideInput, and testMetadataRefreshedAcrossIntervalsAsSideInputWithMultipleTables so worker-local table ID caches expire as intended across virtual intervals. - In testStreamingSpecEvolutionWithoutPipelineRestart, wait for the worker-local cache TTL (500ms) to elapse after spec evolution before emitting the second element. * Declare jackson-databind dependency in Iceberg IO module - Add library.java.jackson_databind to dependencies in sdks/java/io/iceberg/build.gradle to resolve usedUndeclaredArtifacts dependency analysis warning caused by JsonNode usage in SerializableTableSpec. * Avoid potential flakes in EvolveSpec DoFn, address nits * Simplify worker-local cache in TableMetadataDriver using LinkedHashMap LRU - Replace Guava Cache, CacheBuilder, and Ticker in ExtractTableIdsDoFn with a standard LinkedHashMap-based LRU cache capped at 10,000 entries. - Track lastEmitted timestamp per table identifier and emit when (now - lastEmitted) >= refreshInterval / 2. - Remove getClock() and setClock() from TableMetadataDriver and its Builder to keep the AutoValue API clean and free of testing methods. - Provide testing hooks via static globalTestClock and instance setClock on ExtractTableIdsDoFn. * simplify dofn + fix unit test --------- Co-authored-by: Ahmed Abualsaud <65791736+ahmedabu98@users.noreply.github.com>
Exposes the public API for use of the iceberg side input caching pattern, adds relevant unit and integration testing.
Part of #39723
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.