[Bug]: <dav> upgrade step takes hours when server connected to large LDAP directory, or has hundreds of users

[andrewperry]

⚠️ This issue respects the following points: ⚠️

• This is a bug, not a question or a configuration/webserver/proxy issue.
• This issue is not already reported on Github OR Nextcloud Community Forum (I've searched it).
• Nextcloud Server is up to date. See Maintenance and Release Schedule for supported versions.
• I agree to follow Nextcloud's Code of Conduct.

Bug description

When upgrading from 26.0.4 to 27.0.1 there is a step in the upgrade flow that says:

"Updating <dav> ..."

which seems to query the ldap directory and for every entry in LDAP update the "System Address Book" in the 'addressbooks' table as well as create an account in the 'accounts' table with a corresponding entry in the 'ldap_user_mapping' table.

If a server is connected to an LDAP directory with thousands, tens of thousands or hundreds of thousands of users, this can take hours to recurse through and it does not seem that this is a step that should block the completion of the upgrade. Can this step please be moved to a separate recommendation, like the addition of indexes for performance, that would appear in the admin interface alongside other security & performance recommendations. This would enable the server to get out of maintenance mode quicker, rather than have the server offline for hours (or days) while tens of thousands of LDAP accounts are probed and accounts created and/or carddav 'cards' in the "system" addressbook updated (often without them needing to be as they should only be created when the user attempts to login to nextcloud with their LDAP credentials).

Steps to reproduce

1. Enable LDAP integration
2. Attempt to upgrade from Nextcloud 26.0.4 to 27.0.1
3. Tail updater.log which gives the impression the upgrade is complete but the cli is still hanging at "Updating <dav> ..."
4. Tail nextcloud.log -f to see the large number of ldap queries continuously race by for hours

Expected behavior

Upgrade should complete without the need to recurse the whole connected LDAP directory.

Installation method

Community Web installer on a VPS or web space

Nextcloud Server version

27

Operating system

Debian/Ubuntu

PHP engine version

PHP 8.1

Web server

Apache (supported)

Database engine version

MariaDB

Is this bug present after an update or on a fresh install?

Upgraded to a MAJOR version (ex. 22 to 23)

Are you using the Nextcloud Server Encryption module?

None

What user-backends are you using?

• Default user-backend (database)
• LDAP/ Active Directory
• SSO - SAML
• Other

Configuration report

No response

List of activated Apps

No response

Nextcloud Signing status

No response

Nextcloud Logs

No response

Additional info

No response

[kesselb]

cc @ChristophWurst @miaulalala

[kesselb]

Thank you 👍

Sounds like a reasonable request.

We run syncInstance from a migration¹ during the upgrade.
Indeed, this can take a while if your ldap directory is bigger.

Idea:

• Setup check to show a warning "Please run dav:sync-system-addressbook to synchronizes users to the system addressbook". Should be shown if a appconfig "pendingSystemAddressbookSync" is there.
• Change the migration: if (IUserManager.countSeenUsers > 100) then add appconfig "pendingSystemAddressbookSync" else run syncInstance.

Footnotes

1. https://github.com/nextcloud/server/blob/92c18b252c12acb06c343b11fb36f322f4d17adf/apps/dav/lib/Migration/Version1027Date20230504122946.php ↩

[kesselb]

• Copy setup check https://github.com/nextcloud/server/blob/215aef3cbdc1963be1bb6bca5218ee0a4b7f1665/apps/settings/lib/SetupChecks/PhpDefaultCharset.php

• Register the new setup check below and add it to the data response

  server/apps/settings/lib/Controller/CheckSetupController.php

  Line 907 in ab70bbd

  $ldapInvalidUuids = new LdapInvalidUuids($this->appManager, $this->l10n, $this->serverContainer);

• Register the new setup check below

  server/core/js/setupchecks.js

  Line 536 in d25d5d3

  OC.SetupChecks.addGenericSetupCheck(data, 'OCA\\Settings\\SetupChecks\\LdapInvalidUuids', messages)

• Version1027Date20230504122946 inject IUserManager $userManager and IConfig $config and store it as class property.

• Modify postSchemaChange to skip syncInstance when $userManager->countSeenUsers() is bigger than X (e.g. 100).

		if ($this->userManager->countSeenUsers() > 100) {
			$this->config->setAppValue('dav', 'pendingSystemAddressbookSync', 'yes');
			$output->warning('Skip system address book sync due the large amount of users. Please run dav:sync-system-addressbook manually.');
			return;
		}

[ChristophWurst]

We could load-balance the instance sync by dispatching a small background job for each user.

Instead of running the migration in a loop for all users at a time, the migration would dispatch a background job for every user known at upgrade time. The cron/background process will then pick up those jobs in the background, even when the system is live and outside of maintenance mode.

Downside: right after upgrade, data is slightly inconsistent.

[solracsf]

Note: this is not an LDAP only issue but a "lot of users" issue: #39769

[miaulalala]

Can you give us a guesstimate of the amount of users you have @andrewperry ?