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

[3.2] SHiP flush logs on write #928

Merged
merged 1 commit into from
Mar 31, 2023
Merged

Conversation

heifner
Copy link
Member

@heifner heifner commented Mar 30, 2023

Flush the state_history_plugin logs to disk at the end of each write to make it more likely that a kill -9 or crash leaves valid logs.

Resolves #596

@heifner heifner requested review from spoonincode and linh2931 March 30, 2023 12:30
@heifner heifner added the OCI Work exclusive to OCI team label Mar 30, 2023
@@ -263,6 +263,9 @@ class state_history_log {
const uint32_t num_blocks_in_log = _end_block - _begin_block;
fc::raw::pack(log, num_blocks_in_log);
}

log.flush();
index.flush();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this have any performance impact? But correctness is more important and this is only on ship node. It should be OK.

Do we have tests which repeatedly kill and shutdown ship?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there is a performance impact. Likely fairly small.
We do not test kill -9 with SHiP as there is no guarantee it will work. If the kill -9 happens during or between the flushes then this will not help. However, during manual testing I was able to kill -9 and recover from snapshot. Since most of the time is outside the write/flush this does help.

@heifner heifner merged commit 4359bc0 into release/3.2 Mar 31, 2023
@heifner heifner deleted the GH-596-ship-flush-3.2 branch March 31, 2023 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCI Work exclusive to OCI team
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants