Fix race condition between provision step and BifrostService::start #2533
Conversation
|
@tillrohrmann I still need to check why the CI is failing while things seems to be working locally, meanwhile you already can take a look if I overlooked something else in this approach. |
muhamadazmy
force-pushed
the
pr2533
branch
2 times, most recently
from
02ea5fa to
f4e6ed6
Compare
crates/node/src/lib.rs
Outdated
| let value = metadata_store_client | ||
| .read_modify_write(key, |current| match current { | ||
| None => Ok(initial_value.clone()), | ||
| Some(logs) => { | ||
| if logs.version() == Version::MIN && logs.num_logs() == 0 { | ||
| let builder = initial_value.clone().into_builder(); | ||
| // make sure version is incremented to MIN + 1 | ||
| Ok(builder.build()) | ||
| } else { | ||
| Err(ReadModifyWriteError::FailedOperation(AlreadyInitialized)) | ||
| } | ||
| } | ||
| }) | ||
| .await; |
There was a problem hiding this comment.
The check for num_logs() is a bit extra but in general it looks correct. I would suggest to improve this a bit by checking if the current_value == the initial_value in case we were trying to overwrite our own previous write. In that case we can just return success instead of bumping up the version.
There was a problem hiding this comment.
Done, but because Logs does not implement PartitalEq, I compared the config instead since this is what we care about. Instead of adding PartialEq everywhere.
There was a problem hiding this comment.
Thanks for fixing this race condition @muhamadazmy. LGTM. +1 for merging.
Summary: During initialization of metadata. Try to resolve conflict with bifrost initialization by recreating the logs iff it still at MIN version and has an empty chains Fixes restatedev#2486
Fix race condition between provision step and BifrostService::start
Summary:
During initialization of metadata. Try to resolve conflict with bifrost initialization
by recreating the logs iff it still at MIN version and has an empty chains
Fixes #2486
Stack created with Sapling. Best reviewed with ReviewStack.