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

Assert that explicit transaction snapshot never overwrites an older one #1494

Open
wants to merge 1 commit into
base: fb-mysql-8.0.32
Choose a base branch
from

Conversation

laurynas-biveinis
Copy link
Contributor

Explicit transaction snapshots are maintained by std::shared_ptr objects. When
the last shared pointer is destructed, the shared snapshot is released. Add
asserts that these snapshots are not held too long by checking that, whenever a
new explicit snapshot is assigned to a transaction, that it does not overwrite
an older existing one.

To be able to cover all the places and to clean up the code, make the
m_explicit_snapshot field protected instead of public, and add a few new methods
instead.

At the same time convert pointer to reference arguments, add [[nodiscard]],
shorten Rdb_explicit_snapshot::explicit_snapshot_mutex critical sections.

Explicit transaction snapshots are maintained by std::shared_ptr objects. When
the last shared pointer is destructed, the shared snapshot is released. Add
asserts that these snapshots are not held too long by checking that, whenever a
new explicit snapshot is assigned to a transaction, that it does not overwrite
an older existing one.

To be able to cover all the places and to clean up the code, make the
m_explicit_snapshot field protected instead of public, and add a few new methods
instead.

At the same time convert pointer to reference arguments, add [[nodiscard]],
shorten Rdb_explicit_snapshot::explicit_snapshot_mutex critical sections.
@facebook-github-bot
Copy link

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link

@luqun has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

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

Successfully merging this pull request may close these issues.

2 participants