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.

Sign up for GitHub

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

Jump to bottom

Add batch methods in user backends #32912

Merged
merged 5 commits into from
Sep 11, 2023
Merged

Add batch methods in user backends #32912

merged 5 commits into from
Sep 11, 2023

Conversation

CarlSchwan
Copy link
Member

This allows for faster group search with significantly less DB traffic

Splited from #32201 for easier review

@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label Jun 17, 2022
@CarlSchwan CarlSchwan added this to the Nextcloud 25 milestone Jun 17, 2022
@CarlSchwan CarlSchwan requested review from blizzz, come-nc and a team June 17, 2022 09:25
@CarlSchwan CarlSchwan self-assigned this Jun 17, 2022
@CarlSchwan CarlSchwan requested review from PVince81 and removed request for a team June 17, 2022 09:25
github-advanced-security[bot]
github-advanced-security bot found potential problems Jun 17, 2022
View reviewed changes
apps/user_ldap/lib/Group_LDAP.php Fixed Show fixed Hide fixed
apps/user_ldap/lib/Group_Proxy.php Fixed Show fixed Hide fixed
lib/private/Group/Database.php Fixed Show fixed Hide fixed
lib/private/Group/Database.php Fixed Show fixed Hide fixed
lib/private/Group/Database.php Fixed Show fixed Hide fixed
lib/private/Group/Manager.php Fixed Show fixed Hide fixed
@CarlSchwan CarlSchwan force-pushed the group-backend-batch-method branch from 905f9f8 to e9ae315 Compare June 17, 2022 11:23
come-nc
come-nc requested changes Jun 20, 2022
View reviewed changes
apps/user_ldap/lib/Group_Proxy.php Outdated Show resolved Hide resolved
lib/private/Group/Database.php Outdated Show resolved Hide resolved
lib/private/Group/Database.php Outdated Show resolved Hide resolved
lib/private/Group/Database.php Outdated Show resolved Hide resolved
lib/private/Group/Manager.php Outdated Show resolved Hide resolved
lib/private/Group/Manager.php Show resolved Hide resolved
lib/private/Group/Manager.php Outdated Show resolved Hide resolved
come-nc
come-nc reviewed Jun 21, 2022
View reviewed changes
lib/public/GroupInterface.php Outdated Show resolved Hide resolved
lib/public/GroupInterface.php Outdated Show resolved Hide resolved
CarlSchwan
CarlSchwan commented Jun 21, 2022
View reviewed changes
lib/private/Group/Database.php Outdated Show resolved Hide resolved
lib/private/Group/Database.php Outdated Show resolved Hide resolved
lib/private/Group/Database.php Outdated Show resolved Hide resolved
@CarlSchwan CarlSchwan force-pushed the group-backend-batch-method branch 3 times, most recently from 49c2ffb to 918273b Compare June 23, 2022 16:02
@CarlSchwan CarlSchwan force-pushed the group-backend-batch-method branch from 918273b to c1aead3 Compare June 23, 2022 16:33
@CarlSchwan CarlSchwan force-pushed the group-backend-batch-method branch 2 times, most recently from 7a32996 to a4fa2ab Compare June 23, 2022 17:45
@CarlSchwan CarlSchwan requested a review from come-nc July 14, 2022 15:37
@CarlSchwan CarlSchwan force-pushed the group-backend-batch-method branch from a4fa2ab to ed477f9 Compare July 14, 2022 15:39
come-nc
come-nc reviewed Jul 25, 2022
View reviewed changes
apps/user_ldap/lib/Group_Proxy.php Outdated Show resolved Hide resolved
lib/private/Group/Database.php Outdated Show resolved Hide resolved
lib/private/Group/Manager.php Outdated Show resolved Hide resolved
@CarlSchwan CarlSchwan force-pushed the group-backend-batch-method branch from ed477f9 to e8badf4 Compare July 25, 2022 09:50
@github-actions
Copy link
Contributor

Possible performance regression detected

Show Output 
2 queries added

= /remote.php/dav/files/test
= /remote.php/dav/files/test/test.txt
≠ /remote.php/dav/files/test/many_files with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
≠ /remote.php/dav/files/test/new_file.txt with 1 queries added
  + UPDATE "oc_preferences" SET "configvalue" = :dcValue1 WHERE ("userid" = :dcValue2) AND ("appid" = :dcValue3) AND ("configkey" = :dcValue4)
= /remote.php/dav/files/test/new_file.txt

@CarlSchwan
Copy link
Member Author

Failing tests are related

This was referenced May 3, 2023
Merged
Closed
Merged
@come-nc come-nc modified the milestones: Nextcloud 27, Nextcloud 28 May 9, 2023
@come-nc
Copy link
Contributor

come-nc commented May 9, 2023

Discussed this internally, should be moved to a new interface.

@come-nc come-nc added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 9, 2023
@come-nc come-nc force-pushed the group-backend-batch-method branch from 3e399f5 to ebfebc8 Compare September 5, 2023 09:23
github-advanced-security[bot]
github-advanced-security bot found potential problems Sep 5, 2023
View reviewed changes
lib/private/Group/Database.php Fixed Show fixed Hide fixed
lib/private/Group/Database.php Fixed Show fixed Hide fixed
lib/private/Group/Database.php Fixed Show fixed Hide fixed
lib/private/Group/Database.php Fixed Show fixed Hide fixed
lib/public/Group/Backend/ABackend.php Fixed Show fixed Hide fixed
lib/public/Group/Backend/ABackend.php Fixed Show fixed Hide fixed
@come-nc come-nc force-pushed the group-backend-batch-method branch 2 times, most recently from 00ccbd3 to 28b6cf3 Compare September 5, 2023 12:31
@come-nc
Copy link
Contributor

come-nc commented Sep 5, 2023

Discussed this internally, should be moved to a new interface.

Added the interface, rebased, fixed tests and psalm reports.

@szaimen szaimen removed their request for review September 5, 2023 13:43
Altahrim
Altahrim reviewed Sep 5, 2023
View reviewed changes
lib/private/Group/Database.php Outdated Show resolved Hide resolved
lib/private/Group/Database.php Show resolved Hide resolved
CarlSchwan and others added 4 commits September 5, 2023 16:56
@CarlSchwan @come-nc
Add batch methods in user backends
3270b7f 
This allows for faster group search with significantly less DB traffic

Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@come-nc
Fix psalm errors about groupExists return type
a5fa1e7 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Move new methods to a new interface in OCP
2c8b415 
This avoids breaking compatibility for group backends not based on
 ABackend abstract class.

Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc
Fix psalm spotted type errors
35069ad 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc force-pushed the group-backend-batch-method branch from 28b6cf3 to 35069ad Compare September 5, 2023 14:56
@come-nc come-nc requested a review from Altahrim September 7, 2023 07:38
Altahrim
Altahrim approved these changes Sep 7, 2023
View reviewed changes
lib/private/Group/Database.php Outdated Show resolved Hide resolved
@come-nc
Build query once instead of in-loop
d18bb7e 
Signed-off-by: Côme Chilliet <come.chilliet@nextcloud.com>
@come-nc come-nc merged commit 4f1e5bc into master Sep 11, 2023
@come-nc come-nc deleted the group-backend-batch-method branch September 11, 2023 09:11
@come-nc come-nc added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@szaimen szaimen szaimen left review comments

@Altahrim Altahrim Altahrim approved these changes

@PVince81 PVince81 Awaiting requested review from PVince81

@blizzz blizzz Awaiting requested review from blizzz

Assignees

@come-nc come-nc

Labels
4. to release Ready to be released and/or waiting for tests to finish performance 🚀
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.
5 participants
@CarlSchwan @come-nc @blizzz @Altahrim @szaimen