Skip to content

fix(iceberg): publish version hint after metadata copy - #663

Merged
xe-nvdk merged 2 commits into
Basekick-Labs:mainfrom
bferanmi806-sketch:fix/636-version-hint-publish-order
Sep 1, 2026
Merged

xe-nvdk merged 2 commits into
Basekick-Labs:mainfrom
bferanmi806-sketch:fix/636-version-hint-publish-order

Conversation

@bferanmi806-sketch

Copy link
Copy Markdown
Contributor

Fixes #636

writeVersionHint could advance version-hint.text even when reading or publishing the corresponding v<N>.metadata.json copy failed. That could leave directory-based Iceberg readers pointing at a metadata version that was not available.

Only advance the hint after the matching metadata copy succeeds, preserving the last known-good hint while the existing reconciliation retry path retries publication.

Adds a regression that keeps the hint writable while making the newer metadata unavailable, verifies the old hint is preserved, then verifies recovery publishes and advances to the new version.

Also updates the 2026.09.1 release notes.

@xe-nvdk xe-nvdk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this — it's a well-built fix, and welcome as a first-time contributor! 🎉

Verified locally (maintainer review):

  • The fix matches #636's prescribed shape exactly: the hint now publishes only after the matching v<N>.metadata.json copy succeeds, okAll=false keeps the retry path armed, and the deferred noteHintFailure still fires on the early return.
  • Bonus correctness: treating warehouseRelKey failure as a failure (instead of silently skipping the copy but still writing the hint) closes the same inconsistency from another angle. The permanent outside-warehouse configuration is unaffected — it's handled earlier by parseVersionAndMetaDir → warn-once → return true.
  • Regression test verified failing pre-fix (reverted exporter.go to main, test fails with hint changed after metadata failure: got "2", want "1"; restored, passes). The recovery leg is a nice touch. Full internal/iceberg suite, go vet, and gofmt all clean.
  • CI workflow run approved (first-time contributor gate).

One change requested: the release-notes entry needs to move from RELEASE_NOTES_2026.09.1.md to RELEASE_NOTES_2026.09.2.md. 26.09.1 is frozen and releases Monday from an already-cut branch, so this merge lands in the 26.09.2 patch (October) — where the rest of the iceberg audit issues (#632–#639) are targeted. The 26.09.2 file already exists on main; the same text works as-is under a ## Bug fixes section there.

After that rebase/edit this looks ready to me — final approval/merge to the maintainer.

@xe-nvdk xe-nvdk added this to the 26.09.2 milestone Aug 29, 2026
@bferanmi806-sketch

Copy link
Copy Markdown
Contributor Author

Thanks @xe-nvdk, really appreciate the detailed review and the welcome! I’ll move the release note to 26.09.2 and update the branch.

I enjoyed working on this. Once this is sorted, I’d be happy to pick up another issue from the Iceberg audit if there’s one you think would be useful to tackle next.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

medium(iceberg): version-hint.text is updated even when the v<N>.metadata.json copy failed — breaks working directory readers

2 participants