Flink: Bump DynamicWriteResultSerializer version after format change - #17315
Flink: Bump DynamicWriteResultSerializer version after format change#17315wangyum wants to merge 2 commits into
Conversation
|
cc @aiborodin @pvary |
|
Could you please help me what is the user facing issue and which Iceberg versions are affected? Also what will happen with the users if we fix this now? |
|
The user-facing issue is:
|
If we fix this now, then what will happen when a user runs a job with 1.11.0, and upgrades to the fixed version? |
a59bec8 to
761ab24
Compare
Both 1.10 and 1.11 works if apply this patch.
|
|
I'm afraid, there was a mistake with one of our bugfixes. @mxm: Could you please review? |
|
@wangyum |
mxm
left a comment
There was a problem hiding this comment.
I'm also struggling to understand how the described issue could have occurred. DynamicWriteResultSerializer is used between DynamicWriter and DynamicWriteResultAggregator to serialize / deserialize DynamicWriteResult.
DynamicWriter flushes WriteResults when the checkpoint barrier arrives. It is stateless. DynamicWriteResultAggregator emits DynamicCommittables which are derived from DynamicWriteResult, but use their own serializer which has proper versioning. It also does not checkpoint any state.
The only plausible scenario where the serializer could pose an issue is with unaligned checkpoints where we checkpoint the in-flight data. We generally do not recommend using unaligned checkpoints because it changes fundamental stream assumptions, e.g. in a channel, where element A is sent before element B, A is processed first. That said, Flink version upgrades should generally not be performed through checkpoints but through savepoints which are aligned by nature.
Maybe you can give more context @wangyum?
Summary
#14810 changed the serialized format of
DynamicWriteResultSerializerby adding aspecIdfield, but did not bumpgetVersion()from1. This breaks theSimpleVersionedSerializerversioning contract and makes version 1 ambiguous:WriteTargetlayout (tableName, branch, schemaId, specId, upsertMode, equalityFields) tagged as version 1TableKey+specIdlayout also tagged as version 1 (the version was not bumped)This causes checkpoint/savepoint restore failures:
StreamCorruptedException: invalid stream header: 00000000OutOfMemoryError(hugeequalityFieldscount from misaligned bytes)The sibling
DynamicCommittableSerializerwas correctly versioned in the same commit (bumped to V2 withdeserializeV1/deserializeV2). This was simply missed.Fix
getVersion()to 2 — new checkpoints are tagged V2, eliminating future ambiguitydeserializeV1WriteTarget— reads the old 1.10.xWriteTargetformatdeserializeV2— reads the currentTableKey+specIdformat (same body as the olddeserialize(1,...))isV1NewFormat— sniffs V1 bytes to distinguish 1.10.x from 1.11.0, then delegates to the appropriate deserializerHeuristic: distinguishing the two V1 layouts
After
tableName+branch(common to both layouts), the method skips 4 bytes and checks if the next 2 are0xACED(Java serialization magic):specId, the next 2 bytes are the start of the Java-serializedWriteResultstream (0xACED) → correctly identified as new format, delegates todeserializeV2schemaId, the next 2 bytes are the upper 2 bytes ofspecId. Since spec IDs are sequentially assigned starting from 0, they are always < 65536, so the upper 2 bytes in big-endian are0x0000→ correctly identified as old format, usesdeserializeV1WriteTargetA false positive would require
specId >= 0xACED0000(~2.9 billion), which is impossible in practice.Upgrade paths after fix
deserializeV1WriteTargetdeserializeV2deserializeV2Testing
testRoundtrip— verifies V2 serialize → deserialize roundtrip (existing, usesgetVersion()which now returns 2)testUnsupportedVersion— verifies unknown versions throw (existing, unchanged)testDeserializeV1Format— constructs old V1-format bytes (WriteTarget layout withspecId=3) and verifies correct deserializationtestBuggyV1BytesRestoredWithFixedDeserializer— constructs 1.11.0 V1-format bytes (TableKey + specId, tagged version 1) and verifies the fixed deserializer correctly restores themtestOldV1FormatWithZeroSpecIdNotMisdetected— edge case: old V1 format withspecId=0must not be misidentified as new format by the heuristicAll tests pass on
iceberg-flink-1.20,iceberg-flink-2.0, andiceberg-flink-2.1.Closes #17314
AI Disclosure