perf(core): Add index for jobs last_checked, reserved_at, time_sensitive

[provokateurin]

• Resolves: [Bug]: Missing column in database index on query for time sensitive background jobs #46126

Summary

Replaces the two existing indices with a single shared index saving some time on queries previously using both separate indices.

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)

[nickvergessen]

Suggested change

	# Add updated index
	if (!$table->hasIndex('job_last_reserved_sensitive')) {
	$table->addIndex(['last_checked', 'reserved_at', 'time_sensitive'], 'job_last_reserved_sensitive');
	}

This should be done only on new installations and otherwise the change in core/Application.php takes care?
The best to achieve this is modifying the original migration that added the old indexes (you should also comment them out with a reference that Version31000Date20240819122840.php deletes them).

[provokateurin]

So just keep the old indices?

[nickvergessen]

No, deleting is fine. Just not creating new ones

[ChristophWurst]

\OC\Core\Migrations\Version13000Date20170718121200::changeSchema still creates the old index. Comment it out and add the new index with a comment as well saying that the indices were changed later on

[ChristophWurst]

Adding an index can be slow if there is a lot of data. Would it be possible to use https://docs.nextcloud.com/server/latest/developer_manual/basics/storage/migrations.html#replacing-indices instead?

[nickvergessen]

yeah, that's the first change she did in core/Application

[provokateurin]

Ah I didn't know about the possibility to replace indices.

[provokateurin]

So should I replace or only add the index in Application.php?

[nickvergessen]

Correct way is:

1. Add $event->addMissingIndex( section in core/Application.php
2. Add migration that deletes the old index
3. Comment out the code that created the old index with a reference to the migration from 2. (could be in another migration file or in core/Application.php (like here)
4. Find the migration file that creates the table (or the last one that creates all necessary columns for your index) and add the index there.

[AndyScherzinger]

See https://github.com/nextcloud/server/pull/46974/files#diff-a13c08ddffcec08aa3dfe030b79869cd4b164cf35f9a4fff0e22dbe9848b552eR20 for examples, DB migrations for 30+ should be annotated for the new 30+ feature to work 👍

[ChristophWurst]

Correct way is:

1. Add `$event->addMissingIndex(` section in `core/Application.php`

2. Add migration that deletes the old index

3. Comment out the code that created the old index with a reference to the migration from 2. (could be in another migration file or in `core/Application.php` (like here)

4. Find the migration file that creates the table (or the last one that creates all necessary columns for your index) and add the index there.

I'm skeptical about this because it will leave instances in an unindexed state after the upgrade, until the admins apply optional indices.
We have the index replacement APIs for this case. The old index is not great but better than no index. Later on the admin can replace the index for best performance.

So my counter proposal is

1. Adjust the original migration to comment out the old index and add the new
2. Drop the old optional index logic
3. Advertise the new index as replacement for the old, if it exists