PR#4081: Fix index creation - koji

koji

#4081 Fix index creation

Closed 9 months ago by tkopecek. Opened a year ago by tkopecek.

tkopecek/koji issue4080 into master

Fix index creation

Tomas Kopecek • a year ago

8104afd

schemas/schema-upgrade-1.34-1.35.sql

file modified

+3 -2

		`@@ -2,8 +2,9 @@`
		`-- from version 1.33 to 1.34`

		`BEGIN;`
		`+ -- schema changes here`
		`+ COMMIT;`

		`+ -- can't run inside transaction`
		`CREATE INDEX CONCURRENTLY IF NOT EXISTS rpminfo_nvra`
		`ON rpminfo(name,version,release,arch,external_repo_id);`
		`-`
		`- COMMIT;`

tkopecek commented a year ago

@mikem hesitating if we want to have faster migration or safer but blocking index creation.

mikem commented 9 months ago

I had the same fix in #4033

mikem commented 9 months ago

Having just the one thing outside the block (and after it) means that the only way the migration script can fail and still make changes is to fail on this index creation. We already have the if not exists clause, so it seems unlikely to fail in any reasonable scenario.

OTOH, I suppose the safest thing would be to make the default script transactional, but offer an alternative script with this optimization and document in the release notes.

Do we have a sense of how long this index creation will take for a large system? (granted I think all the large systems we deal with have already applied this index).

mikem commented 9 months ago

Breadcrumb: index comes from #4026

tkopecek commented 9 months ago

It is still relatively short. I've tried with ours and it is about ~1 minute.

mikem commented 9 months ago

If an instance as big as ours is just one minute, let's keep it all transactional

mikem commented 9 months ago

https://pagure.io/fork/mikem/koji/commits/pr4081updates

mikem commented 9 months ago

@tkopecek does my update here look ok? If so, want to pull in? Or should I file a different PR?

tkopecek commented 9 months ago

Let's create a new one. There are also some conflicts with current master.