Additional check to avoid downloading segments from deep store #14957
Conversation
reverse merge
reverse merge
reverse merge
reverse merge
reverse merge
reverse merge
reverse merge
merge master
merge master
merge master
merge master
merge master
merge master
merge master
merge master
…t when no processing is needed
| SegmentDirectory segmentDirectory = | ||
| initSegmentDirectory(segmentName, String.valueOf(zkMetadata.getCrc()), indexLoadingConfig); | ||
| // We should first try to reuse existing segment directory | ||
| if (isDirectoryReusable(zkMetadata, segmentTier, segmentDirectory, indexLoadingConfig, schema)) { |
There was a problem hiding this comment.
Wondering if I should add a forceDownload property here!
There was a problem hiding this comment.
I don't fully follow. When CRC mismatch, we should always download a new copy from the deep store right?
There was a problem hiding this comment.
@deepthi912 I think we can try to override downloadAndLoadSegment() to do the extra checks before downloading raw segments from deep store or move those inside interface, as those extra steps (particularly initializing SegmentDirectory object) don't apply to servers managing segments on local disk.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14957 +/- ##
=============================================
- Coverage 61.75% 34.00% -27.76%
- Complexity 207 673 +466
=============================================
Files 2436 2712 +276
Lines 133233 152009 +18776
Branches 20636 23486 +2850
=============================================
- Hits 82274 51684 -30590
- Misses 44911 96147 +51236
+ Partials 6048 4178 -1870
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Continuation to : #10089
This avoids downloading from deep store when we can just simply use the existing directory.