Open KenBirman opened 3 years ago
Many of these shutdown tasks are covered by Group::leave(), ViewManager::leave(), and destructors of the two classes. But some clean up operations are missing like sst::lf_destroy() and rdmc::shutdown(). Edward suggests to add them in ViewManager::leave().
The current shutdown process in the main branch still prints error messages in a given situation. It turns out that the persistence_manager
member of the Group
class will flush the data to files before it exists. Meanwhile, the other group components like rpc_manager
and view_manager
are still active. However, the flush workload for different Cascade/Derecho members varies a lot. So some member processes exited earlier than the others, which are still flushing in the destructor of persistence_manager
. Therefore, the view_manager
thread in those slow processes will detect node failure and report errors. I fixed this by introducing a barrier in Group::leave()
so that the view will be maintained until the flushing has finished system-wide (Commit 85f72e9).
A better solution could be disabling all heartbeats () on leave()
.
For future improvement: Group
class member attributes are forming cyclic dependencies, which results in non-deterministic access to released resources during the shutdown process. We should go over the attribute/field list, make a dependency tree for all its members, and refactor the code accordingly for better code management.
This is a trivial proposal, but turns out to be important to our users (notably the AFRL funding people who have supported us for several years).
The issue: If a process is a member of the top-level group and exits "abruptly", but without crashing (like by return from main), our handling is messy and often causes error messages. Users think Derecho is broken. If all members exit, we can be VERY messy right now.
Solution: Leverage C++ destructors for static objects. In the C++ standard, destructors for static objects are called when a program() exits normally (abnormal exit won't necessarily do this, but I'm not worried about that case).
We would add a simple class to Derecho:
You can test this... you'll see that for any normal shutdown, the destructor gets called.
Then we offer people a derecho::detach() and a derecho::shutdown() API:
As you can see, my proposal is that the destructor in this static object will cause derecho::shutdown to be called automatically if you didn't call derecho::detach.
derecho::shutdown would use the simple 2-phase approach: 1) P2P RPC to every member of top-level group, “shutdown_request”. 2) With a mutex, all members print "Derecho shutdown requested", but only once (e.g. if the same call occurs multiple times, we print only the first time), then sets shutdown_in_progress = true, then replies “ok”. 3) After shutdown_in_progress is set, we inhibit all prints and all spdlog messages from Derecho. 4) After all replied “ok”, P2P send to every member: “shutdown now”. On receipt of this, they exit. 5) As soon as the P2P sends have completed, the initiator exits.
Additionally: 6) Inhibit all aspects of Edward's new view logic once shutdown_in_progress is true: We don't want to mess up our logs at the very last moment.