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

[4.0 -> 5.0] hardening resource monitor manager plugin shutdown handling #1783

Merged
merged 7 commits into from
Oct 17, 2023

Conversation

linh2931
Copy link
Member

#1485 reports occasional resource manager plugin test failures. The failure is shown as
/__w/leap/leap/plugins/resource_monitor_plugin/test/test_resmon_plugin.cpp(147): fatal error: in "resmon_plugin_tests/startupNormal": unexpected exception thrown by plugin_startup({"/tmp"})
without any other additional information.

Further investigation reveals the scheduled monitor timer task is not cancelled during plugin shutdown and currently monitor task might be still running before the thread exits. This PR hardens the plugin shutdown process.

Merges release/4.0 into release/5.0 including #1774

Resolves #1485

linh2931 and others added 7 commits October 13, 2023 16:01
Those tests were intended to verify the duration of space_monitor_loop.
In essence they tested Boost's expires_from_noa, which was not
necessary.
The tests themselves were hacky and took uncessary 50 seconds.
* removes unnecessary check of info logging "Creating and st
arting monitor thread"
* reduces time to wait for nodeos startup from 120 seconds to 10 seconds
* no need to call cancel timer explicitly.
* no need to use mutex for timer as when timer is used on the main
  thread, resource monitor thread has already stopped.
…ning

[4.0] hardening resource monitor manager plugin shutdown handling
@linh2931 linh2931 merged commit 92fa264 into release/5.0 Oct 17, 2023
21 checks passed
@linh2931 linh2931 deleted the merge_resmon_fix_to_5_0 branch October 17, 2023 14:02
@heifner heifner added this to the Leap v5.0.0-stable milestone Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants