eBay / HomeStore

Storage Engine for block and key/value stores.
Apache License 2.0
23 stars 21 forks source link

handle leave cluster event #594

Closed JacksonYao287 closed 1 day ago

JacksonYao287 commented 1 week ago

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

codecov-commenter commented 1 week ago

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 54.28571% with 16 lines in your changes missing coverage. Please review.

Project coverage is 66.60%. Comparing base (1a0cef8) to head (73bd8c2). Report is 94 commits behind head on master.

Files with missing lines Patch % Lines
src/lib/blkalloc/append_blk_allocator.cpp 0.00% 5 Missing :warning:
src/lib/replication/repl_dev/raft_repl_dev.cpp 60.00% 4 Missing :warning:
src/include/homestore/replication/repl_dev.h 0.00% 1 Missing :warning:
src/lib/blkalloc/bitmap_blk_allocator.h 0.00% 1 Missing :warning:
src/lib/blkalloc/fixed_blk_allocator.h 0.00% 1 Missing :warning:
src/lib/blkalloc/varsize_blk_allocator.h 0.00% 1 Missing :warning:
src/lib/device/vchunk.cpp 0.00% 1 Missing :warning:
...rc/lib/replication/repl_dev/raft_state_machine.cpp 91.66% 0 Missing and 1 partial :warning:
src/lib/replication/repl_dev/solo_repl_dev.h 0.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ 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 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

JacksonYao287 commented 5 days ago

like on_joining_cluster, on_leaving_cluster

this is a good suggestion, will make this change

xiaoxichen commented 5 days ago

Then in this case, should we use https://github.com/eBay/NuRaft/blob/092997ab67eb2529c5b40556e1d301c8220c315d/include/libnuraft/callback.hxx#L95 and https://github.com/eBay/NuRaft/blob/092997ab67eb2529c5b40556e1d301c8220c315d/include/libnuraft/callback.hxx#L101

more callback type can be consumed from handle_raft_event

JacksonYao287 commented 5 days ago

more callback type can be consumed from handle_raft_event

1 in this PR , I only want to handle the leave cluster case. we can create another PR for join cluster case

2 RemovedFromCluster callback will be called before the new configuration is applied , IMO, it is better to call it after the new configuration is applied, where we can guarantee the config changed is completed and the node is definitely removed from the cluster configuration

xiaoxichen commented 1 day ago

I hope the commits and commit message can be cleanup , there are two parts in this PR:

  1. consume nuraft::cb_func::Type::RemovedFromCluster callback
  2. add reset function to allocator/vchunk as a preparation for implementing m_listener->on_destroy()

The pr title/commit messages are a bit confusing.

JacksonYao287 commented 1 day ago

The pr title/commit messages are a bit confusing.

sure, will refine this commit message