Repository navigation
Conversation
…e merging When a partial update is merged into a record carrying every reader field, SparkRecordMergingUtils built the merged schema by converting the merged struct type back to a HoodieSchema. That conversion drops field defaults (for example the -1 default of the position-merging row index column), so the merged schema is unequal to the reader schema while Avro gives both the same hash. Every schema-keyed lookup holding both (the record context schema cache, HoodieInternalRowUtils.getCachedSchema, the merged-schema cache, isPartial) then compared the two schemas field by field, once per merged record. Reuse the reader schema as the merged schema when all reader fields are present, and let HoodieSchema.hashCode delegate to the Avro schema's cached hash code.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #20269 +/- ##
============================================
+ Coverage 80.62% 80.63% +0.01%
- Complexity 35055 35062 +7
============================================
Files 2552 2552
Lines 143400 143402 +2
Branches 17454 17455 +1
============================================
+ Hits 115615 115631 +16
+ Misses 19867 19856 -11
+ Partials 7918 7915 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This branch has not been deployed
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.
Describe the issue this Pull Request addresses
closes #20268
Merging a partial update into a full record built the merged schema by converting the Spark struct type back to a
HoodieSchema, which drops field defaults (for example the-1default of the position-merging row index column). That schema is unequal to the reader schema but has the same Avro hash, so every schema-keyed lookup on the merge path (LocalHoodieSchemaCache,HoodieInternalRowUtils.getCachedSchema, the merged-schema cache,isPartial) compared the two field by field once per merged record. #12949 removed this class of cost by interning schemas; interning does not help here because the two schemas are not equal.Summary and Changelog
When the merged fields are all the reader fields,
SparkRecordMergingUtils.getCachedMergedSchemanow returns the reader schema itself; partial-only merges are unchanged.HoodieSchema.hashCodedelegates to the Avro schema's cached hash instead ofObjects.hash. NewTestSparkRecordMergingUtilscovers the full-plus-partial, full-update and partial-plus-partial merges (two cases fail on master).Impact
Lower CPU and allocation when reading or compacting MOR file groups with partial-update log records on Spark. Single-thread merge benchmark (31-field reader schema, 2-column partial records): 2367 to 486 ns and 13.8 KB to 0.7 KB allocated per merged record. Merged values are unchanged.
Risk Level
low. A full newer record is now returned as is instead of copied, so in event-time ordering
deltaMergereturns empty (keep the buffered record) when the buffered full record wins;TestBufferedRecordMergeris updated for that. The kept record has the same values, ordering value and delete flag, and keeps its own operation and key.TestPartialUpdateForMergeIntopasses.Documentation Update
none
Contributor's checklist