fix(dav): Mark removal of dav object properties as expensive

[ChristophWurst]

• Resolves: Nextcloud upgrade taking hours on large installations. This is because the oc_properties table has indices for userid+propertypath and propertypath. The DELETE query filters on propertyname and propertyvalue. There are no other queries during normal operation that would benefit from an index, so it's not a valuable alternative.

Summary

Moves the repair step from a synchronour repair step to an expensive ones. These are executed via occ.

TODO

• Change registration
• Test repair without expensive steps ✔️ step doesn't run anymore
• Test repair with expensive steps ✔️ step runs

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

[ChristophWurst]

@tcitworld correct me if my assumption is wrong but we have had this step since #30368 so at late upgrades the repair step will not remove a lot of rows.

[ChristophWurst]

/backport to stable31

[ChristophWurst]

/backport to stable30

[ChristophWurst]

/backport to stable29

[ChristophWurst]

I'm not a huge fan of using dav app classes in server but there is no other way right now. #50390 tracks the possibility to declare expensiveness directly in info.xml.

[juliusknorr]

Make sense, though the query still does not seem to use an index. Can we be sure that the delete does not cause locks on the table for too long or may cause other problems?

[ChristophWurst]

Correct, the query does not use an index.

This is because the oc_properties table has indices for userid+propertypath and propertypath. The DELETE query filters on propertyname and propertyvalue. There are no other queries during normal operation that would benefit from an index, so it's not a valuable alternative.

Adding an index would probably also take a couple of hours, and be counterproductive for normal operations outside of the repair step. Let me double check for any queries that could benefit from the query nevertheless.

[ChristophWurst]

The most fitting index would be one on propertyname, propertypath and userid:

\OCA\DAV\DAV\CustomPropertiesBackend::getUserProperties -> either userid+propertypath OR propertyname+propertypath+userid
\OCA\DAV\DAV\CustomPropertiesBackend::createDeleteQuery -> propertyname+propertypath+userid
\OCA\DAV\DAV\CustomPropertiesBackend::createUpdateQuery -> propertyname+propertypath+uderid
\OCA\DAV\BackgroundJob\UserStatusAutomation::getAvailabilityFromPropertiesTable -> propertyname+propertypath+userid
\OCA\DAV\Db\PropertyMapper::findPropertyByPathAndName -> propertyname+propertypath+userid
\OCA\DAV\Migration\RemoveObjectProperties::run -> propertyname

Other queries that only filter by propertypath and userid would still use properties_path_index

[tcitworld]

correct me if my assumption is wrong but we have had this step since #30368 so at late upgrades the repair step will not remove a lot of rows.

That's right.

Btw, a repair step that we should skip on upgrades is #13430, which should be a one time job and not run on every upgrade.