Repository navigation
feat: send Glue VersionId on Hive table updates (GLUE_SEND_VERSION_ID) - #150
Merged
Merged
Conversation
Add an opt-in GLUE_SEND_VERSION_ID flag (default off) that makes the GlueSync listener read the current Glue VersionId and pass it on UpdateTable requests for non-Iceberg (Hive) tables, so AWS records requestParameters.versionId on the emitted CloudTrail event. Consumers of that event stream can then identify and fetch the pre-update table version for ALTER_TABLE handling. Glue enforces VersionId as a compare-and-swap, so a concurrent writer fails with ConcurrentModificationException. The retry/fallback control flow is extracted into OptimisticUpdateRetry (pure, independently tested): refresh the version and retry up to a bound, then fall back to an unconditional update so the sync never regresses below today's last-write-wins behaviour (surfaced via the version_conflict_fallback outcome tag). When the flag is on, Hive tables are forced to skipArchive=false so the prior version is retained and the carried VersionId resolves via GetTableVersion; an explicit per-table apiary.gluesync.skipArchive property still wins. Iceberg tables are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Previously a non-EntityNotFound failure of the getTable read (throttle, timeout, access denied, etc.) added by the versionId path propagated as a sync failure for that event — a new failure surface vs the single-call behaviour before the flag. The read is best-effort, so readVersionId now returns null on any AmazonClientException (other than EntityNotFound, which still propagates so the caller creates the missing table), and the update proceeds unconditionally. Surfaced via a new version_unavailable_fallback outcome tag. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…come retryOnConflict now returns the result of whichever branch ran (Supplier<T>), so each branch returns its own UpdateOutcome directly instead of signalling "version unavailable" through a mutable boolean[] captured in the lambda. Also add an end-to-end test proving that, with GLUE_SEND_VERSION_ID on, an ALTER_TABLE for a table missing in Glue (EntityNotFoundException from the getTable version read) still drives a create via updateOrCreateTable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the bounded retry loop and the OptimisticUpdateRetry helper. On a ConcurrentModificationException (or when the versionId cannot be read), do a single unconditional update instead of retrying. This keeps the key guarantee — the sync never regresses below the listener's last-write-wins behaviour — without the retry machinery, since a conflict carries no correctness signal for a mirror and the unconditional write reaches the same end state a manual resync would. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
In prod the flag is set globally, so every table update takes the versionId path. Add integration tests (real GluePartitionService, flag on) proving the request shape for the flows the flag actually changes: - Hive ALTER_TABLE on an existing table -> versionId set + skipArchive=false - Iceberg ALTER_TABLE -> unaffected (no getTable, no versionId, skipArchive=true) - CREATE_TABLE on an existing table (AlreadyExists -> update) -> versionId set These exercise the real skipArchive wiring that the GlueTableService unit tests mock away. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jamespfaulkner
force-pushed
the
feat/gluesync-send-versionid-single-attempt
branch
from
October 7, 2026 12:43
b129386 to
97b15dd
Compare
javsanbel2
reviewed
Oct 7, 2026
| this(glueClient, gluePrefix, defaultSkipArchive, false); | ||
| } | ||
|
|
||
| public GluePartitionService(AWSGlue glueClient, String gluePrefix, Boolean defaultSkipArchive, |
Contributor
There was a problem hiding this comment.
It doesn't exist versioning on glue partitions, so we don't need any change in this class?
Contributor
Author
There was a problem hiding this comment.
Good point. The GLUE_SKIP_ARCHIVE was in this class but I've moved the new prop to the table service.
javsanbel2
reviewed
Oct 7, 2026
| * UpdateTable event does not carry a versionId.</li> | ||
| * </ul> | ||
| */ | ||
| public enum UpdateOutcome { |
Contributor
There was a problem hiding this comment.
Put this in an external file class
…ched Address PR #150 review: - Send the Glue VersionId for all table types, not just Hive (Javier: versionId is preferable to scanning versions/metadata_location for the pre-update state). Removes the Iceberg filter. - Leave GLUE_SKIP_ARCHIVE / shouldSkipArchive entirely in GluePartitionService, reverted to main (zero diff). The feature no longer overrides SkipArchive; archiving is governed by GLUE_SKIP_ARCHIVE as before (already false in all dronefly envs, so the emitted versionId resolves; versionId never gates or creates versions). - Extract DeleteOutcome and UpdateOutcome into their own files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jamespfaulkner
changed the title
javsanbel2
reviewed
Oct 8, 2026
| public UpdateOutcome update(Table table) { | ||
| UpdateTableRequest updateTableRequest = new UpdateTableRequest() | ||
| .withSkipArchive(skipArchive) | ||
| .withSkipArchive(gluePartitionService.shouldSkipArchive(table)) |
Contributor
There was a problem hiding this comment.
why glue table calls glue partition serivce for this?
Contributor
Author
There was a problem hiding this comment.
This was already there. Just now rather than creating a boolean its inlined
javsanbel2
approved these changes
Oct 8, 2026
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.
What
Adds an opt-in
GLUE_SEND_VERSION_IDflag (default off) to the GlueSync listener. When enabled, the listener reads the current GlueVersionId(viaGetTable) and passes it onUpdateTablerequests for non-Iceberg (Hive) tables, so AWS recordsrequestParameters.versionIdon the emitted CloudTrailUpdateTableevent.Why
The emitted
versionIdis the version the table was at immediately before the update. Consumers of the Glue/CloudTrail event stream can use it to deterministically identify and fetch the pre-update table version (viaGetTableVersion) when handlingALTER_TABLEevents, instead of guessing. Today noversionIdis emitted, so that pre-update state can't be resolved reliably.How
GlueTableService.update()— for Hive tables, reads the currentVersionIdand sets it on theUpdateTable. Iceberg tables are skipped (identified bymetadata_location), avoiding the extraGetTableand optimistic-locking overhead.VersionIdas a compare-and-swap, so a concurrent writer fails the write withConcurrentModificationException; in that case — or if the currentVersionIdcan't be read (any Glue failure other than the table not existing) — the listener does a single unconditional update instead, matching the listener's existing last-write-wins behaviour when the flag is off. Only that one event loses itsversionId. There is no retry loop: a conflict carries no correctness signal for a mirror, and the unconditional write reaches the same end state a manual resync would.glue_listener_eventcounter'soutcometag asversion_conflict_fallback(CAS conflict) andversion_unavailable_fallback(version unreadable).versionIdto resolve to fetchable content, the prior version must be archived, so when the flag is on, Hive tables are forced toskipArchive=false. An explicit per-tableapiary.gluesync.skipArchiveproperty still wins; Iceberg tables keepskipArchive=true(avoids table-version explosion).EntityNotFoundExceptionfrom the version read propagates soupdateOrCreateTablecreates the table, exactly as before the flag existed.Behaviour is unchanged when the flag is unset (default), and the flag is honoured identically by the listener and the resync CLI.
Testing
GlueTableServiceTest— versionId set/skipped (Hive vs Iceberg vs disabled); conflict → single unconditional fallback; unreadable version → unconditional fallback;EntityNotFoundExceptionpropagates for create-on-missing.GluePartitionServiceTest—skipArchiveprecedence (per-table override > Hive-retain-when-enabled > env default > built-in) and env-var parsing.ApiaryGlueSyncTest— end-to-end: with the flag on, anALTER_TABLEfor a table missing in Glue still drives a create.mvn verifygreen (142 tests, 0 failures), checkstyle/license clean.Docs
README (
GLUE_SEND_VERSION_IDenv var + "Table update VersionId" section + updated SkipArchive precedence) and CHANGELOG updated.