Skip to content

Add maintenance:repair step for file size of unencrypted files #36220

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

Open
simonspa wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add maintenance:repair step for file size of unencrypted files #36220

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
2421404
Add repair step for unencrypted_size
Jan 18, 2023
c9b6d5e
Register new repair step
Jan 18, 2023
8a03a1d
Fix psalm complaints and add log
Jan 18, 2023
768436c
Satisfy linter :)
Jan 18, 2023
6856c38
Only touch problematic rows
simonspa Jan 18, 2023
112bbd2
Fix query
Jan 18, 2023
43da5b0
RepairStep for unencrypted size: make DB query more efficient
Jan 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix query 
Signed-off-by: Simon Spannagel <[email protected]>
  • Loading branch information
Simon Spannagel committed Jan 18, 2023
commit 112bbd2aa6e2a229c7ad17a71f32f607e2cc82fe
4 changes: 2 additions & 2 deletions lib/private/Repair/RepairUnencryptedSize.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ public function run(IOutput $output) {
$update = $this->connection->getQueryBuilder();
$update->update('filecache')
->set('unencrypted_size', "0")
->where($update->expr()->eq('encrypted', "0", IQueryBuilder::PARAM_INT)
->andWhere($update->expr()->neq('unencrypted_size', "0", IQueryBuilder::PARAM_INT);
->where($update->expr()->eq('encrypted', "0", IQueryBuilder::PARAM_INT))
->andWhere($update->expr()->neq('unencrypted_size', "0", IQueryBuilder::PARAM_INT));
Copy link
Member

@juliusknorr juliusknorr Jan 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no index on neither of those columns, so we may want to add at least to the unencrypted size.

I think the most safe way to use such an index then would be:

Suggested change
->where($update->expr()->eq('encrypted', "0", IQueryBuilder::PARAM_INT))
->andWhere($update->expr()->neq('unencrypted_size', "0", IQueryBuilder::PARAM_INT));
->andWhere($update->expr()->gt('unencrypted_size', "0", IQueryBuilder::PARAM_INT));
->andWhere($update->expr()->eq('encrypted', "0", IQueryBuilder::PARAM_INT))

Copy link
Member

@juliusknorr juliusknorr Jan 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the reasoning:

  • neq may not use the index
  • the order of where statements sometimes also matters to decide if the index is applied

Copy link
Member

@juliusknorr juliusknorr Jan 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EXPLAIN of the current query does a full table scan:


explain update oc_filecache set unencrypted_size = 0 where encrypted = 0 and unencrypted_size != 0;
+------+-------------+--------------+-------+---------------+---------+---------+------+------+-------------+
| id   | select_type | table        | type  | possible_keys | key     | key_len | ref  | rows | Extra       |
+------+-------------+--------------+-------+---------------+---------+---------+------+------+-------------+
|    1 | SIMPLE      | oc_filecache | index | NULL          | PRIMARY | 8       | NULL | 8479 | Using where |
+------+-------------+--------------+-------+---------------+---------+---------+------+------+-------------+

Copy link
Contributor Author

@simonspa simonspa Jan 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reversal of arguments and using > query affects smaller number of rows:

explain update oc_filecache set unencrypted_size = 0 where unencrypted_size > 0 and encrypted = 0;
+------+-------------+--------------+-------+---------------+---------+---------+------+------+-------------+
| id   | select_type | table        | type  | possible_keys | key     | key_len | ref  | rows | Extra       |
+------+-------------+--------------+-------+---------------+---------+---------+------+------+-------------+
|    1 | SIMPLE      | oc_filecache | index | NULL          | PRIMARY | 8       | NULL | 490  | Using where |
+------+-------------+--------------+-------+---------------+---------+---------+------+------+-------------+

Copy link
Contributor Author

@simonspa simonspa Jan 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I implemented the suggested change - but adding an index to either of these columns would have to done separately, and probably run manually, right? How would that work together with an automatic repair step?


if ($update->execute()) {
$output->info('Fixed file size of unencrypted files');
Expand Down