eBay / NuRaft

C++ implementation of Raft core logic as a replication library
Apache License 2.0
996 stars 236 forks source link

Extract the public interface class global_mgr, so users can implement their own global_mgr functionality to optimize multi raft instance. #440

Closed byronhe closed 1 year ago

greensky00 commented 1 year ago

Hi @byronhe

I suggest you doing in the below way, rather than adding it to ctx_ and modifying existing static functions of nuraft_global_mgr which breaks the backward compatibility as well.

In ngm_singleton, add a new function:

bool create_custom(std::unique_ptr<nuraft_global_mgr> src) {
    if (internal_.get()) {
        // Already created.
        return false;
    }
    internal_ = std::move(src);
    return true;
}

In nuraft_global_mgr, add a new custom init function

nuraft_global_mgr* nuraft_global_mgr::init_custom(std::unique_ptr<nuraft_global_mgr> custom_mgr) {
    nuraft_global_mgr* mgr = ngm_singleton::get_instance().get();
    if (!mgr) {
        ngm_singleton::get_instance().create_custom(std::move(custom_mgr));
        return ngm_singleton::get_instance().get();
    }
    return mgr;
}

Make below 4 functions virtual as you did:

virtual void init_raft_server(raft_server* server);
virtual void close_raft_server(raft_server* server);
virtual void request_append(ptr<raft_server> server);
virtual void request_commit(ptr<raft_server> server);

And the custom manager should inherit nuraft_global_mgr:

class custom_global_mgr : public nuraft_global_mgr

Also please make all test pass.

byronhe commented 1 year ago

Thank you for your suggestion. as the nuraft_global_mgr class has many member variables and methods , I feel it is not suitable as an interface class. For example, the subclass we implemented did not use these member variables and methods.

in my opinion, add a global_mgr * in context can make the core raft logic independent of any global variables in multi-instance mode, which can make the code more easily adaptable to more scenarios, such as allowing multi-global_mgr-in-one-process testing.

Therefore, I have submitted a new pull request. Please review it: https://github.com/eBay/NuRaft/pull/441