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

Fix shared object retries #4579

Merged
merged 2 commits into from
Sep 12, 2022
Merged

Fix shared object retries #4579

merged 2 commits into from
Sep 12, 2022

Conversation

mystenmark
Copy link
Contributor

@mystenmark mystenmark commented Sep 12, 2022

Fixes #4387

The basic idea here is that transaction_input_check should always load the object versions that will be consumed by the TX, rather than the latest versions in the DB. This behavior comes for free with owned objects, since those are already specified by ObjRef. By using the shared lock table, we provide the same behavior for shared objects.

Previously, transaction input checker loads the latest version of shared
objects. The fix is to instead load the sequenced versions, but only during
certificate processing.
Copy link
Contributor

@lxfind lxfind left a comment

Choose a reason for hiding this comment

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

Could consider renaming get_input_objects in the store to something like get_latest_objects or similar

@mystenmark
Copy link
Contributor Author

Could consider renaming get_input_objects in the store to something like get_latest_objects or similar

As discussed in slack, this would be misleading, since it doesn't actually get the latest objects in the case of owned objects.

@mystenmark mystenmark merged commit e64720f into main Sep 12, 2022
@mystenmark mystenmark deleted the mlogan-fix-shared-object-replay branch September 12, 2022 20:19
InputObjectKind::MovePackage(id) => self.get_object(id)?,
InputObjectKind::SharedMoveObject(id) => match shared_locks.get(id) {
Some(version) => self.get_object_by_key(id, *version)?,
None => None,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be something like shared_locks.get(id).expect("Did not find shared lock for shared object") right?
Since this is basically an invariant violation, we should not just return None

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.

Shared object transactions may never finish the full execution flow
4 participants