Skip to content

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.

@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.

@luqun
Copy link
Contributor

luqun commented Oct 7, 2024

@laurynas-biveinis , could you do a rebase?

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.
@laurynas-biveinis laurynas-biveinis force-pushed the cleanup-explicit-snapshot branch from 70dfe2a to 0583c62 Compare October 8, 2024 15:17
@facebook-github-bot
Copy link

@laurynas-biveinis has updated the pull request. You must reimport the pull request before landing.

@laurynas-biveinis
Copy link
Contributor Author

@luqun , rebased

@facebook-github-bot
Copy link

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

@facebook-github-bot
Copy link

This pull request has been merged in d76fb0d.

@laurynas-biveinis laurynas-biveinis deleted the cleanup-explicit-snapshot branch October 14, 2024 06:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

3 participants