-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add replace member changes. Perist membership details. #216
Conversation
ee75415
to
c8d6a16
Compare
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #216 +/- ##
==========================================
- Coverage 68.69% 65.18% -3.52%
==========================================
Files 30 32 +2
Lines 1581 1772 +191
Branches 163 191 +28
==========================================
+ Hits 1086 1155 +69
- Misses 408 522 +114
- Partials 87 95 +8 ☔ View full report in Codecov by Sentry. |
c8d6a16
to
b8c259c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from my side and waiting for the testing result
As discussed in the meeting, we need to re-think the flow with the learner changes. My feeling is we need to wait till the learner flip to follower (voter) then do the step 3
and setp 4
Likely we need to hook up to the
to get notification on cluster member change especially learner flipped to follower. |
b8c259c
to
bca8193
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please if you can rebase.
|
||
uint32_t i{0}; | ||
for (auto const& m : pg->pg_info_.members) { | ||
hs_pg->pg_sb_->members[i].id = m.id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HomeObject struct pg_members
is identical to homestore::replica_member_info
as of now.
As homestore::replica_member_info
is the only information that follower can get regarding a PG, I doubt there is a possibility HomeObject::pg_members
can diverse from homestore::replica_member_info
Thinking about consolidate them together, not blocking, more for discussion
bca8193
to
68eb07d
Compare
|
I think it is same when the out_member has down. But if the out_member is healthy , i.e PG move for balancing/tech refresh etc, keeping the redundancy has advantage. If we dont wait till new_member catch up before deleting out_member, we made us degrade to 2 replica. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Created a story for this. eBay/HomeStore#573 This has to be optional depending on use case. Sometimes, its better to evict immediately to free up space. |
feel free to merge and as you said that can track by other ticket. |
No description provided.