Skip to content

fix: skip repair RemoveBrokenProperties if already run#61409

Open
kyteinsky wants to merge 2 commits into
masterfrom
fix/skip-RemoveBrokenProperties-if-already-run
Open

fix: skip repair RemoveBrokenProperties if already run#61409
kyteinsky wants to merge 2 commits into
masterfrom
fix/skip-RemoveBrokenProperties-if-already-run

Conversation

@kyteinsky

@kyteinsky kyteinsky commented Jun 18, 2026

Copy link
Copy Markdown
Contributor
  • Resolves: #

Summary

The repair step removes null characters from the properties table. The PR which added this also made sure no further entries are inserted with null characters so it should be safe to run once and then skip it for the later upgrades.
The need for this arises from the fact that valuetype column is compared which does not have an index and should not have it since it's not used anywhere else, but it results in a long scan of the DB whenever the repair step runs.

The step is marked expensive but expensive repair steps still run in maintenance mode and cause a long downtime.

It was added in #49528

TODO

  • ...

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: kyteinsky <kyteinsky@gmail.com>
@kyteinsky kyteinsky requested a review from a team as a code owner June 18, 2026 13:02
@kyteinsky kyteinsky requested review from ArtificialOwl, artonge, leftybournes and salmart-dev and removed request for a team June 18, 2026 13:02
@ChristophWurst

Copy link
Copy Markdown
Member

I'm wondering if this one-time migration should have just been a background job. Downside of moving it there is that it runs outside the maintenance window. Not sure if that is problematic.

@SebastianKrupinski

Copy link
Copy Markdown
Contributor

I'm wondering if this one-time migration should have just been a background job. Downside of moving it there is that it runs outside the maintenance window. Not sure if that is problematic.

TBH, I think that might be a better solution, If anyone ever needs to rerun this, it would be easier to remember just to trigger the migration again, which creates a new job.

@kyteinsky

Copy link
Copy Markdown
Contributor Author

Downside of moving it there is that it runs outside the maintenance window. Not sure if that is problematic.

maybe the initial query that reads the complete oc_properties table could be a little heavy when the instance is live but looks good otherwise to me.

for the re-run possibility, it can be run again with occ maintenance:repair --include-expensive after setting the config key to false.

Signed-off-by: kyteinsky <kyteinsky@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants