[Remote Store] Integrate remote segment store in peer recovery flow #6664
Conversation
Gradle Check (Jenkins) Run Completed with:
|
|
Build is failing due to flaky test: #6665 |
Signed-off-by: Sachin Kale <kalsac@amazon.com>
Signed-off-by: Sachin Kale <kalsac@amazon.com>
sachinpkale
force-pushed
the
peer-recovery-integration
branch
from
dd1f138
to
0889239
Compare
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Sachin Kale <kalsac@amazon.com>
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Sachin Kale <kalsac@amazon.com>
sachinpkale
requested review from
reta,
anasalkouz,
andrross,
Bukhtawar,
CEHENKLE,
dblock,
gbbafna,
setiah,
kartg,
kotwanikunal,
mch2,
nknize,
owaiskazi19,
Rishikesh1159,
ryanbogan,
saratvemulapalli,
shwetathareja and
dreamer-89
as code owners
sachinpkale
requested review from
tlfeng,
VachaShah and
xuezhou25
as code owners
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Sachin Kale <kalsac@amazon.com>
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Sachin Kale <kalsac@amazon.com>
Gradle Check (Jenkins) Run Completed with:
|
| syncSegmentsFromRemoteSegmentStore(overrideLocal, true); | ||
| } | ||
|
|
||
| /** | ||
| * Downloads segments from remote segment store. | ||
| * @param overrideLocal flag to override local segment files with those in remote store | ||
| * @param refreshLevelSegmentSync last refresh checkpoint is used if true, commit checkpoint otherwise | ||
| * @throws IOException if exception occurs while reading segments from remote store | ||
| */ | ||
| public void syncSegmentsFromRemoteSegmentStore(boolean overrideLocal, boolean refreshLevelSegmentSync) throws IOException { |
There was a problem hiding this comment.
Can sync segments be agnostic of the source like peer node or remote store?
There was a problem hiding this comment.
Looks quite challenging to me as the code paths are completely different . Would like to know @sachinpkale's view as well .
Also the paths are different in other parts like restore as well . So there is consistency across all parts .
There was a problem hiding this comment.
As @gbbafna mentioned, this method is not specific to peer recovery. We are using the same method in restore as well as failover flow.
There was a problem hiding this comment.
In this change, we make sure that segments are downloaded from the remote store instead of the source node. Remote store is not replacing the source node. I have not given much thought to this yet but given the peer recovery flow, it would be much bigger change.
There was a problem hiding this comment.
The method name itself doesn't provide an abstraction. Lets open a follow up issue to clean this up ?
There was a problem hiding this comment.
The method name itself doesn't provide an abstraction. Lets open a follow up issue to clean this up ?
Okay, will create a tracking issue.
There was a problem hiding this comment.
Created issue: #6831
There was a problem hiding this comment.
Changes look good to me . We can merge once @Bukhtawar's concerns are addressed .
Gradle Check (Jenkins) Run Completed with:
|
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #6664 +/- ##
============================================
+ Coverage 70.60% 70.76% +0.16%
- Complexity 59151 59254 +103
============================================
Files 4810 4810
Lines 283502 283510 +8
Branches 40884 40885 +1
============================================
+ Hits 200153 200626 +473
+ Misses 66869 66436 -433
+ Partials 16480 16448 -32
... and 512 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
…pensearch-project#6664) Signed-off-by: Sachin Kale <kalsac@amazon.com> Signed-off-by: Valentin Mitrofanov <mitrofmep@gmail.com>
Description
Issues Resolved
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.