[#881] fix: Ensure LocalStorageMeta disk size is correctly updated when events are processed #902
Conversation
Codecov Report
@@ Coverage Diff @@
## master #902 +/- ##
============================================
+ Coverage 55.24% 57.09% +1.84%
- Complexity 2201 2202 +1
============================================
Files 333 313 -20
Lines 16449 14091 -2358
Branches 1307 1308 +1
============================================
- Hits 9087 8045 -1042
+ Misses 6851 5605 -1246
+ Partials 511 441 -70
... and 21 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@zuston Could you review this pr? Because you are more familiar with this part of code. |
Oh, yes, let me take a look in the next few days |
|
I noticed this whilst benchmarking Uniffle. It makes the settings related to disk capacity almost void and this fix fixed it for me |
|
@awdavidson Could you tell me your company? Do you use Uniffle in your production environment? |
|
@awdavidson There is conflict in the branch 0.7. Could you raise a pr for branch 0.7? |
…ted when events are processed (apache#902) Ensure all events are marked as understorage, this will result to the LocalStorageMeta being updated when events are processed. Currently LocalStorageMeta is only update with metrics from the first event in a given shuffleId and partitionId, the first event updates metrics because there is no entry in `partitionsOfStorage` and the event get marked as `underStorage`, however, for future events in the same shuffleId and partitionId `selectStorage` returns the storage and does not mark the event as `underStorage` so when `updateWriteMetrics` is called, `event.getUnderStorage()` returns null and `storage.updateWriteMetrics(metrics);` is skipped. As metrics are not updated correctly, `LocalStorage.canWrite` will not return the correct result. No. Added unit test which covers multi events for the same shuffleId and partitionId
What changes were proposed in this pull request?
Ensure all events are marked as understorage, this will result to the LocalStorageMeta being updated when events are processed.
Why are the changes needed?
Currently LocalStorageMeta is only update with metrics from the first event in a given shuffleId and partitionId, the first event updates metrics because there is no entry in
partitionsOfStorageand the event get marked asunderStorage, however, for future events in the same shuffleId and partitionIdselectStoragereturns the storage and does not mark the event asunderStorageso whenupdateWriteMetricsis called,event.getUnderStorage()returns null andstorage.updateWriteMetrics(metrics);is skipped.As metrics are not updated correctly,
LocalStorage.canWritewill not return the correct result.Does this PR introduce any user-facing change?
No.
How was this patch tested?
Added unit test which covers multi events for the same shuffleId and partitionId