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.

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

fix: move repair mimetype repair step to the expensive steps #45930

Merged
merged 2 commits into from
Jun 19, 2024

Conversation

icewind1991
Copy link
Member

Looking through the entire filecache takes a while

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Jun 17, 2024
@icewind1991 icewind1991 added this to the Nextcloud 30 milestone Jun 17, 2024
@icewind1991 icewind1991 requested review from ChristophWurst, juliusknorr, a team, ArtificialOwl, yemkareems and sorbaugh and removed request for a team June 17, 2024 15:12
@icewind1991
Copy link
Member Author

/backport to stable29

@icewind1991
Copy link
Member Author

/backport to stable28

@kesselb
Copy link
Contributor

kesselb commented Jun 17, 2024

Not ideal.

Adding mime types to existing files can be important to make certain functionality work.

Could you add a hint to lib/private/Repair/RepairMimeTypes.php that any update to the file must be mentioned in our release notes so that admin's know they have to run occ maintenance:repair --include-expensive?

@joshtrichards joshtrichards added the pending documentation This pull request needs an associated documentation update label Jun 17, 2024
@icewind1991 icewind1991 added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Jun 18, 2024
@icewind1991 icewind1991 added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Jun 18, 2024
@icewind1991
Copy link
Member Author

  • Added a separate "version" to track the ran mimetype migrations
  • Added a setup check for when mimetype repair needs to be ran

@icewind1991 icewind1991 force-pushed the repair-mimetype-expensive branch from dbfd000 to 5a56726 Compare June 18, 2024 09:32
@kesselb
Copy link
Contributor

kesselb commented Jun 18, 2024

  • Added a separate "version" to track the ran mimetype migrations
  • Added a setup check for when mimetype repair needs to be ran

Thank you 👍

@icewind1991 icewind1991 force-pushed the repair-mimetype-expensive branch from 5a56726 to 262811f Compare June 18, 2024 11:58
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991 icewind1991 force-pushed the repair-mimetype-expensive branch from 262811f to e74f71b Compare June 18, 2024 12:46
Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Ideally we would split the operations that are performed for the new mimetype:

The insert into oc_mimetypes could be still in regular migration steps as it is just one insert for a new mimetype, that way it will already work for new files: https://github.com/nextcloud/server/blob/repair-mimetype-expensive/lib/private/Repair/RepairMimeTypes.php#L42-L44

Otherwise 👍

@icewind1991
Copy link
Member Author

The insert into oc_mimetypes could be still in regular migration steps as it is just one insert for a new mimetype, that way it will already work for new files

There is no need to preemptively insert the rows into oc_mimetypes, it will be done on-demand if any file with the new mimetype is uploaded before the migration is ran.

The only thing required for new files to work is the mimetype mapping being updated, and that is part of the source upgrade.

@icewind1991
Copy link
Member Author

redoing the other backports based on the 29 one

@Dexus
Copy link

Dexus commented Jun 19, 2024

/backport to stable27
/backport to stable26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews backport-request pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants