Discover and upload Iceberg tables alongside Hudi - #189
Closed
tiennguyen-onehouse wants to merge 9 commits into
Closed
Discover and upload Iceberg tables alongside Hudi#189tiennguyen-onehouse wants to merge 9 commits into
tiennguyen-onehouse wants to merge 9 commits into
Conversation
Extends LakeView to find Iceberg tables (metadata/*.metadata.json) in addition to Hudi tables (.hoodie/) and upload their current metadata.json to the control plane for parsing. Iceberg upload is a parallel orchestrator rather than a Hudi-coupled branch: the active/archived timeline, hoodie.properties bootstrap, and LSM manifest plumbing don't apply. Abstractions: - TableFormatDetector SPI with HudiTableFormatDetector + IcebergTableFormatDetector implementations. TableDiscoveryService iterates registered detectors; the first match determines the format and tags the Table. - New IcebergMetadataUploaderService sibling of TableMetadataUploaderService. Reuses OnehouseApiClient, PresignedUrlFileUploader, AsyncStorageClient. - TableDiscoveryAndUploadJob.dispatchUpload partitions discovered tables by format and runs the two uploaders concurrently. Wire: - TableFormat enum (HUDI, ICEBERG) added to api/models/request. - Table model carries tableFormat (default HUDI for backward compat). - InitializeSingleTableMetricsCheckpointRequest gains tableFormat (nullable; server treats absent as HUDI). TableType (COW/MOR) stays @nonnull; Iceberg uploads pass COW as a meaningless placeholder since the server discriminates on tableFormat first. Depends on idls PR #1939 for ObservedTableFormat on the proto side. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LakeviewSyncTool builds TableDiscoveryService and TableDiscoveryAndUploadJob manually (no Guice in the sync-tool entry path), so the constructor signature changes from the parent commit broke its compile. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tiennguyen-onehouse
left a comment
tiennguyen-onehouse
left a comment
Contributor
Author
There was a problem hiding this comment.
/push-image iceberg-metrics-pr2-lakeview-may13
Adds the LakeView half of the fast-path that lets us skip the per-cycle S3 LIST on Iceberg tables when the control plane already knows the current metadata.json URI (e.g. from AWS Glue's metadata_location parameter on the catalog entry). - New TableHint POJO and Database.tableHints map (keyed on tableId). Older YAML versions don't carry this field; Jackson leaves it null and discovery falls through to the existing listing-based path. - Table model gains metadataLocationHint (optional). - TableDiscoveryService merges per-database tableHints into a single tableId -> hint map and attaches metadataLocationHint to each Table it discovers, when the tableId matches a hint. - IcebergMetadataUploaderService.uploadIfNewMetadataJson branches on the hint: if set, derive the filename, compare against the checkpoint, and PUT directly to the hint URI. If the checkpoint already matches, no-op without touching S3 at all. Falls back to the existing LIST behavior when no hint is provided. The control-plane producer (gw-agent MetricsExtractorFileUpdater) does not yet emit the new field into the YAML — that needs a lakeview-config artifact version bump on the gateway-controller side. Once that lands, the fast-path activates automatically; until then, tableHints stays null and behavior is unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tiennguyen-onehouse
left a comment
tiennguyen-onehouse
left a comment
Contributor
Author
There was a problem hiding this comment.
/push-jar iceberg-metrics-pr2-lakeview-may13
tiennguyen-onehouse
left a comment
tiennguyen-onehouse
left a comment
Contributor
Author
There was a problem hiding this comment.
/push-jar iceberg-lakeview-may13
tiennguyen-onehouse
left a comment
tiennguyen-onehouse
left a comment
Contributor
Author
There was a problem hiding this comment.
/push-jar iceberg-lakeview-may13-v2
…tion
Concern 2: the previous IcebergTableFormatDetector matched any directory
containing a sub-directory named "metadata", which produced false positives
across customer warehouses (Spark checkpoint dirs, custom layouts, schema
folders, etc.). Replace the SPI ordering with per-Database declarative
routing — each Database in the parser YAML now declares its tableFormat
(default HUDI for backward compat), and TableDiscoveryService picks the
single matching detector for that database. The Iceberg detector also
becomes strict: requires metadata/ AND at least one *.metadata.json inside
it (one extra LIST during discovery only, not per upload cycle).
Concern 1: pickLatestMetadataJson lex-sorted filenames, so for the Hadoop
catalog naming v{N}.metadata.json (unpadded), v2.metadata.json beat
v10.metadata.json — a stale pointer. Resolve in three tiers: catalog hint,
then metadata/version-hint.text when present (canonical Iceberg lookup),
then numeric-aware sort on the leading integer. Empty metadata/ now
increments a NO_SUCH_KEY failure counter instead of silently returning
true, so phantom tables surface in dashboards.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tiennguyen-onehouse
left a comment
tiennguyen-onehouse
left a comment
Contributor
Author
There was a problem hiding this comment.
/push-jar iceberg-lakeview-may13-v2
S3 ListObjectsV2 surfaces "subdirectories" as CommonPrefixes whose
Prefix string carries a trailing slash (e.g. "metadata/"). The storage
client preserves that in File.filename, so the exact-string check
"metadata".equals(filename) fails and the detector returns false even
though the table layout is a valid Iceberg root.
Concrete repro in staging (Testing-Acme org, table
s3://acme-data-2/iceberg_tables/iceberg_table_test_may13/):
- S3 list of the table base returns a single CommonPrefix "metadata/"
- The agent emits tableFormat=ICEBERG + metadataLocationHint in the
extractor YAML, the v2 lake-view image parses it fine, the
discovery walks the base path
- Detector returns false at the base, discoverTablesInPath recurses
into metadata/, no Iceberg Table is ever emitted, dispatchUpload
is called with an empty Iceberg set, IcebergMetadataUploaderService
early-returns, nothing reaches the temp bucket
Strip a single trailing slash before comparing, so both "metadata" and
"metadata/" are accepted. Hudi's detector dodges this naturally via
startsWith(".hoodie"); the Iceberg one switched to equals() and lost
the same tolerance.
Add a unit test that hands the detector a File with filename="metadata/"
— the shape S3 actually produces — which fails on main and passes here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tiennguyen-onehouse
left a comment
tiennguyen-onehouse
left a comment
Contributor
Author
There was a problem hiding this comment.
/push-jar iceberg-lakeview-may13-v3
tiennguyen-onehouse
left a comment
tiennguyen-onehouse
left a comment
Contributor
Author
There was a problem hiding this comment.
/push-image iceberg-lakeview-may13-v3
LakeView's InitializeTableMetricsCheckpoint request sends tableFormat as a Jackson-serialized enum value. Without an explicit @JsonProperty the wire string was the short Java identifier ("HUDI" / "ICEBERG"), which protobuf-java-util's JSON parser on external-api cannot map onto lake.TableFormat (the proto enum uses the canonical names "TABLE_FORMAT_HUDI" / "TABLE_FORMAT_ICEBERG"). The mismatch is silent: the parser falls back to enum-zero (TABLE_FORMAT_INVALID), the checkpoint is persisted without the field, and GenerateCommitMetadata UploadUrlHandler routes the table through the Hudi back-compat path. For Hudi this happens to be correct so nothing surfaces; for Iceberg the filename regex rejects the metadata.json with 400. Annotate the enum values so the JSON wire string matches the proto enum name on both sides of the contract. Java callsites remain unchanged.
dharmendersheshma
left a comment
dharmendersheshma
left a comment
Contributor
There was a problem hiding this comment.
/push-image iceberg-lakeview-may14-v1
StorageUtils.getBucketNameFromUri / getPathFromUrl were rejecting s3a:// URIs with IllegalArgumentException, breaking IcebergMetadataUploaderService for every newly-discovered iceberg table on staging-acme. The agent stamps Firestore metadataLocationHint as s3a:// (gateway-controller commit 0761ef368d, so the existing GetTableMetricsHandler OpenSearch keyword filter matches), so every metadata.json upload was throwing inside PresignedUrlFileUploader → S3AsyncStorageClient.streamFileAsync → StorageUtils.getBucketNameFromUri. Extend OBJECT_STORAGE_URI_PATTERN to accept the s3a:// scheme alongside s3:// — both resolve to the same bucket/key, and the AWS SDK only ever sees the parsed bucket + key, so this is purely a parser-tolerance change. Test: StorageUtilsTest now asserts s3a://bucket/path/to/file parses to the same bucket name and path as s3://, plus the bare-bucket edge case.
dharmendersheshma
left a comment
dharmendersheshma
left a comment
Contributor
There was a problem hiding this comment.
/push-image iceberg-lakeview-may14-v2
Code review fixes: - IcebergMetadataUploaderService: increment the table-processing failure counter on the four non-exception failure returns (checkpoint fetch, init, presigned-URL, upsert) so API-level failures show on dashboards, not just exceptions. - Document that the numeric pickLatestMetadataJson fallback can select an uncommitted metadata.json; hint / version-hint.text are authoritative. - Note that thenComposeAsync uses the default pool intentionally (non- blocking glue) and that metadataLocationHint only applies to #tableId- pinned base paths, not auto-discovery. SonarCloud: - S6204: Stream.collect(toList()) -> Stream.toList() in IcebergMetadataUploaderService and TableDiscoveryService (drop now- unused Collectors imports). - S5411: avoid Boolean unboxing in processTable (Boolean.FALSE.equals). - S6068: remove useless eq() matchers in TestIcebergTableFormatDetector. - S5843 (regex complexity) skipped: reducing it 21->20 without loosening URL validation is not worth the risk for a non-gating smell. Coverage (quality gate was failing new_coverage at 41.3%): - Add instance-pipeline tests for IcebergMetadataUploaderService (uploadInstantsInTables, processTable, parse/init/upload/upsert, version-hint resolution, hint fast-path, failure branches) -> ~97% line coverage on the class. - Add dispatchUpload format-routing test to TableDiscoveryAndUploadJobTest. Docs: new docs/iceberg-support.md covering the detector SPI, tableFormat declaration, three-tier metadata.json resolution, and the cross-repo deploy ordering (idls -> gateway-controller -> LakeView). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Future CI compiles on Java 8. Revert the two Sonar S6204 Stream.toList() changes back to Collectors.toList() (toList is Java 16+), and replace Set.of / CompletableFuture.failedFuture in the new tests with Guava ImmutableSet and a local failedFuture helper. S6204 is won't-fix on this toolchain. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
dharmendersheshma
marked this pull request as ready for review
May 22, 2026 05:31
This was referenced May 22, 2026
Contributor
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
Teaches LakeView to detect Iceberg table roots (the
metadata/folder) in addition to Hudi (.hoodie/), and upload the currentmetadata.jsonpointer file to the Onehouse temp bucket on each iteration. The control plane parses the snapshot summary (which carries cumulativetotal-records,total-files-size,total-data-files, per-snapshot deltas, and thesnapshot-log[]history) — gateway-controller side is a separate PR.See
docs/iceberg-support.mdfor the full design.Abstractions
TableFormatDetectorSPI withHudiTableFormatDetector(existing.hoodie/check moved out ofTableDiscoveryService.isHudiTableFolder) andIcebergTableFormatDetector(matches when a directory listing contains ametadata/subdirectory and that subdirectory holds a*.metadata.json).TableDiscoveryServicepicks the single detector matching eachDatabase's declaredtableFormatand tags each discoveredTablewith its format.IcebergMetadataUploaderServiceas a sibling ofTableMetadataUploaderServicerather than a branch inside it. Hudi's active/archived timeline distinction,hoodie.propertiesbootstrap, and LSM manifest plumbing don't apply to Iceberg, so a separate service is clearer than a branching megaclass. It reusesOnehouseApiClient,PresignedUrlFileUploader,AsyncStorageClient, andStorageUtilsdirectly.TableDiscoveryAndUploadJob.dispatchUploadpartitions discovered tables by format and runs the two uploaders concurrently; bothrunOnceandprocessTablesroute through it.Wire
TableFormatenum (HUDI,ICEBERG) inapi/models/request/, JSON-aliased to the proto enum names (TABLE_FORMAT_HUDI/TABLE_FORMAT_ICEBERG) so the control-plane parser accepts the value.Tablemodel gainstableFormat(defaults toHUDIfor backward compat).InitializeSingleTableMetricsCheckpointRequestgains a nullabletableFormat. Server treats absent asHUDI.tableType(COW/MOR) stays@NonNullto preserve the existing wire contract; Iceberg init passesCOWas a meaningless placeholder since the server discriminates ontableFormatfirst.s3a://accepted byOBJECT_STORAGE_URI_PATTERNalongsides3://(Gluemetadata_locationscheme).Selecting the current
metadata.jsonExactly one file is uploaded per iteration — the current
metadata.json— resolved through three paths in order of preference:metadataLocationHintfrom the parser YAML's table hints (catalog-driven, e.g. Gluemetadata_location): PUT the file directly, skip the listing. Only applies to#tableId-pinned base paths.metadata/version-hint.text(Hadoop catalog): its integer content names the currentv{N}.metadata.jsonunambiguously.vprefix) is the version number — correct for bothv{N}.metadata.json(Hadoop) and00000-<uuid>.metadata.json(Hive/Glue/Spark) without depending on zero-padding.metadata.jsonif neither a hint norversion-hint.textis present; paths 1–2 are authoritative.Checkpoint tracks the last uploaded filename; if it hasn't changed since last run, the iteration is a no-op.
Deployment ordering⚠️
This change spans three repos and must roll out in order:
TableFormatproto enum /TableMetricsCheckpointfield (idls PR #1939,80d81701).IcebergCommitMetadataParserparses the uploadedmetadata.jsonand emits to OpenSearch.If LakeView ships before the server understands
tableFormat, the server-side protobuf JSON parser drops the unknown enum value and persistsTABLE_FORMAT_INVALID, routing Iceberg uploads through the Hudi back-compat fallback. Do not deploy this until the running control plane toleratestableFormat=ICEBERGcleanly. See the deploy-ordering section ofdocs/iceberg-support.md.Test plan
./gradlew :lakeview:test --tests "ai.onehouse.metadata_extractor.*"green locallyIcebergMetadataUploaderService(~97% line) + detector pairs +dispatchUploadroutingTableDiscoveryServiceTest/TableDiscoveryAndUploadJobTestpass after constructor signature changess3://aadsharma-quanton-test/spark4_iceberg_variant.db/t_variant_1/once control-plane parser lands🤖 Generated with Claude Code