handle leave cluster event #594
Conversation
JacksonYao287
force-pushed
the
handle-config-change
branch
from
8f40737 to
2f8425f
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 @@
## master #594 +/- ##
===========================================
+ Coverage 56.51% 66.60% +10.09%
===========================================
Files 108 109 +1
Lines 10300 10772 +472
Branches 1402 1472 +70
===========================================
+ Hits 5821 7175 +1354
+ Misses 3894 2893 -1001
- Partials 585 704 +119 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
xiaoxichen
left a comment
xiaoxichen
left a comment
There was a problem hiding this comment.
I think we need more evidence to expose the "raft implementation details" to HO.
As the raft_cluster_config_ptr_t is a ptr to nuraft::cluster_config which is very much implementation related structure.
I think the interpretation should be in HS and call back to listener with specific callback, like on_joining_cluster, on_leaving_cluster, etc.
this is a good suggestion, will make this change |
|
Then in this case, should we use more callback type can be consumed from |
JacksonYao287
force-pushed
the
handle-config-change
branch
from
2f8425f to
57e49bf
Compare
1 in this PR , I only want to handle the leave cluster case. we can create another PR for join cluster case 2 |
| // 2 well , leave_cluster will propose a config change log, when this log is committed, this node will be removed | ||
| // from the cluster configuration. so, this callback will only be called on the removed node, which is no longer in | ||
| // the cluster configuration | ||
| virtual void on_leave_cluster(const group_id_t& group_id){ |
There was a problem hiding this comment.
is there a reason we can't bump conan version to 6.6 and make this pure virtual function, and when make HomeObject PR, consume 6.6 homestore to consume this change?
There was a problem hiding this comment.
the only concern is that before homeobject implements this function, all other PR of homeobject will get a build failure in CI build.
JacksonYao287
force-pushed
the
handle-config-change
branch
from
fb9f751 to
5b3833b
Compare
JacksonYao287
changed the title
|
I hope the commits and commit message can be cleanup , there are two parts in this PR:
The pr title/commit messages are a bit confusing. |
sure, will refine this commit message |
1 consume nuraft::cb_func::Type::RemovedFromCluster callback 2 add reset function to allocator/vchunk as a preparation for implementing m_listener->on_destroy()
JacksonYao287
force-pushed
the
handle-config-change
branch
from
5b3833b to
73bd8c2
Compare
1 consume nuraft::cb_func::Type::RemovedFromCluster callback 2 add reset function to allocator/vchunk as a preparation for implementing m_listener->on_destroy()
4 participants
in nuobject, when a member is moved out of a raft group, we should clear the pg related resource in the member , which will be done in the config change handler. this PR aims to add this interface, which is also helpful for other use case to reclaim their application layer resource