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

Refactor the postgresql invite module #9302

Open
vxgmichel opened this issue Jan 8, 2025 · 0 comments
Open

Refactor the postgresql invite module #9302

vxgmichel opened this issue Jan 8, 2025 · 0 comments
Labels
A-Server Area: Parsec Server I-Postgresql Impact: Postgresql issue refactor Improve code, don't change behavior

Comments

@vxgmichel
Copy link
Contributor

The components.postgresql.invite module has not yet been migrated to the new code layout where each command gets its own dedicated module with the corresponding queries inside.

During this migration, the locking approach needs to be reconsidered.

First, the advisory lock on invitation creation can be removed as described in #9300.
Then, the common topic read lock might not need to be taken as explained by this comment in _q_retrieve_shamir_recovery_setup:

-- We do not lock the shamir topic in read here.
-- This is because topic locking is usually reserved to operations that needs to accept or reject
-- consistent timestamped data from the users, such as certificates or vlobs. Here, only the invitations
-- gets updated, which do not have to remain consistent with other timestamped data.
-- Less lock means less ways to mess up the ordering and create a hard-to-debug deadlock,
-- so we simply accept that our checks might be slightly outdated when the invitation is written.

For the same reason that we do not lock the shamir topic in read here, we might also be able to skip the locking of the common topic.

Careful still: locking the common topic should not be skipped if we perform several checks in the same transaction while making assumptions about those checks based on the results from the previous ones. Skipping the locking should only be done if the checks are independent.

However, we should keep the invitation lock which prevent concurrent update of a given invitation.

@vxgmichel vxgmichel added A-Server Area: Parsec Server I-Postgresql Impact: Postgresql issue labels Jan 8, 2025
@mmmarcos mmmarcos added the refactor Improve code, don't change behavior label Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Server Area: Parsec Server I-Postgresql Impact: Postgresql issue refactor Improve code, don't change behavior
Projects
None yet
Development

No branches or pull requests

2 participants