Skip to content
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

Shutdown is still messy. Proposal to fix it (probably for Edward to implement?) #192

Open
KenBirman opened this issue Jan 20, 2021 · 2 comments

Comments

@KenBirman
Copy link
Contributor

KenBirman commented Jan 20, 2021

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:

class derecho_selfmonitor
{
public:
	~derecho_selfmonitor()
	{
                if(derecho::detached || derecho::shutdown_in_progress || derecho::is_external_client)
                       return;
		std::cerr << “Auto-shutdown sequence initiated (main thread exited without a clean Derecho disconnect)” << std::endl;
                derecho::shutdown();
	}
}

Derecho_selfmonitor  self_monitor_handle;

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:

  • detach disconnects this application from Derecho. We silently shut down our connections and threads.
  • shutdown causes the entire top-level group to shut down. All members print a single final message (see below).

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.

@songweijia
Copy link
Contributor

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().

etremel added a commit that referenced this issue Oct 1, 2021
I discovered one reason why Derecho processes often end with a
segmentation fault when attempting to shut down "cleanly" (as noted in
issues #135, #192, etc.): When a node marks itself as failed, the
SST will be told to freeze the node's own row (in process_suspicions()),
but SST::freeze() will dereference a null pointer if it is called on the
local row (res_vec has no entry for the node's own row).

The solution is to add a check for row_index == my_index in freeze(),
and also to ensure a node shuts itself down more promptly when it
detects that it has been marked as "failed" by the rest of the group.
@songweijia
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants