Skip to content

Conversation

@tcitworld
Copy link
Member

Summary

In OC\User\Manager::createUserFromBackend the newly created user is read again using getUserObject($uid, $backend) but that can cause causal read issues (wrote in DB primary, not yet in secondary).

In OC\User\Database user backend the user cache is unset after the insert, so it can't be used by getRealUID() (which is called by getUserObject()).

To avoid that we make sure the user cache is repopulated in a transaction. This doesn't change the total number of queries made.

Checklist

@tcitworld tcitworld added this to the Nextcloud 26 milestone Dec 2, 2022
@tcitworld tcitworld requested a review from a team December 2, 2022 16:54
@tcitworld tcitworld self-assigned this Dec 2, 2022
@tcitworld tcitworld requested review from ArtificialOwl, blizzz and juliusknorr and removed request for a team December 2, 2022 16:54
@tcitworld tcitworld changed the title Create the database user in a transaction Create the users from database backend in a transaction Dec 5, 2022
@juliusknorr
Copy link
Member

/rebase

@tcitworld tcitworld force-pushed the create-user-transaction branch from 0ba3e5c to a0be051 Compare January 12, 2023 10:42
@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@szaimen
Copy link
Contributor

szaimen commented Mar 13, 2023

What is missing here?

@juliusknorr
Copy link
Member

Just a merge it seems.

@juliusknorr
Copy link
Member

/rebase

@juliusknorr juliusknorr added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 15, 2023
In OC\User\Manager::createUserFromBackend the newly created user is read
using getUserObject($uid, $backend) but that can cause causal read
issues (wrote in DB primary, not yet in secondary).

In OC\User\Database user backend the user cache is unset after the
insert, so it can't be used by getRealUID() (which is called by
getUserObject()).

To avoid that we make sure the user cache is repopulated in a
transaction.

Signed-off-by: Thomas Citharel <[email protected]>
@nextcloud-command nextcloud-command force-pushed the create-user-transaction branch from a0be051 to f7e65b1 Compare March 15, 2023 11:07
@juliusknorr juliusknorr merged commit c7c1133 into master Mar 15, 2023
@juliusknorr juliusknorr deleted the create-user-transaction branch March 15, 2023 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to release Ready to be released and/or waiting for tests to finish bug feature: users and groups

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants