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

Run all sql before enabling replication triggers #2539

Merged
merged 1 commit into from May 30, 2022

Conversation

mwiencek
Copy link
Member

I see no reason why replication triggers should be re-enabled before the .standalone.sql file is run. If the standalone file happens to modify data in replicated tables, then the current order causes those changes to be logged in the dbmirror pending tables, which is not ideal. I've fixed the order so that replication is re-enabled only after all migration SQL is run.

I see no reason why replication triggers should be re-enabled *before*
the .standalone.sql file is run.  If the standalone file happens to
modify data in replicated tables, then the current order causes those
changes to be logged in the dbmirror pending tables, which is not ideal.
I've fixed the order so that replication is re-enabled only after all
migration SQL is run.
Copy link
Member

@reosarevok reosarevok left a comment

Choose a reason for hiding this comment

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

Seems sensible

Copy link
Contributor

@yvanzo yvanzo left a comment

Choose a reason for hiding this comment

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

LGTM but is the test ./t/script/CheckSchemaMigration.sh still passing with this change?

Edit: Consider it as approved if the test (which is not part of automated CI) is passing. I cannot run it locally.

@mwiencek
Copy link
Member Author

Yes, I can confirm that CheckSchemaMigration.sh still passes for all three replication types. I had to modify it since the production branch now has schema 27 merged into it:

diff --git a/t/script/CheckSchemaMigration.sh b/t/script/CheckSchemaMigration.sh
index e0f05f2e7c..f9d6dbad58 100755
--- a/t/script/CheckSchemaMigration.sh
+++ b/t/script/CheckSchemaMigration.sh
@@ -59,8 +59,8 @@ fi
 
 # DB2 is the schema we want to upgrade from. Here we source it
 # from the production branch.
-git restore --source=production -- admin/sql
-git restore --source=production -- admin/InitDb.pl
+git restore --source=v-2022-04-18 -- admin/sql
+git restore --source=v-2022-04-18 -- admin/InitDb.pl
 ./admin/InitDb.pl --database $DB2 --createdb --clean --reptype $REPLICATION_TYPE "${WITH_PENDING_ARGS[@]}"
 ./admin/psql $DB2 < t/sql/initial.sql
 ./admin/psql $DB2 < admin/sql/SetSequences.sql

@mwiencek mwiencek merged commit 714e4f6 into metabrainz:master May 30, 2022
@mwiencek mwiencek deleted the upgrade.sh-enable-reptg-last branch May 30, 2022 18:08
reosarevok added a commit that referenced this pull request May 30, 2022
* master:
  Update POT files using the production database
  Update translations from Transifex
  MBS-12328: Add #content to entity/Edits for artwork display (#2502)
  MBS-12336: Validate that Predicate::Set is being passed integers (#2509)
  MBS-12333: Block smart links: share.amuse.io (#2501)
  Run all sql before enabling replication triggers (#2539)
  MBS-12412: Fail gracefully if requesting an invalid ID in edit-cover-art (#2547)
  MBS-12387: Add titles to URL editor edit buttons (#2550)
  Bump react-table to 7.8.0 (#2551)
  MBS-12407: Add tobarandualchais.co.uk to the other dbs whitelist (#2545)
  MBS-12280 (II): Add target _blank to more edit form links (#2546)
  MBS-12413: Use uri_escape_utf8 on Data::Wikidoc (#2548)
  ExportAllTables.t: remove --nodbmirror2 flags (#2538)
  MBS-12398: Update release_meta.amazon_asin (#2543)
  Bump Flow to 0.179.0
  JSON dumps: ignore more useless primary/foreign keys (#2536)
  MBS-12404: Replace Text::Unaccent (#2542)
  MBS-12353: Actually check proposed ratings are allowed (#2511)
  MBS-8193 / MBS-12332: Wrap absurdly long lines (#2516)
  MBS-11694: Cleanup /intent/user Twitter URLs (#2519)
  MBS-8875: Improve CatNoLooksLikeASIN regexp (#2520)
  MBS-12347: Also show RG artist in autocomplete if type is null (#2517)
  MBS-12385: Add report for digital releases with mail order rels (#2535)
  MBS-12212: Add tests for event filtering
  MBS-12212: Allow filtering event list by setlist contents
  MBS-12212: Add filtering to artists' events tab
  Update react, react-dom to v18 (#2544)
  Bump Flow to 0.178.1
  Upgrade Flow to 0.178.0
  Fix spammy PG warnings in ProcessReplicationChanges
  Remove BundleReplicationPackets
  MBS-4960: Give useful error when checking donation status fails
  MBS-12361: Allow navigation from donation check page
mwiencek added a commit that referenced this pull request Jun 2, 2022
* master:
  Use flushSync to avoid render lag (fixes MBS-12424) (#2559)
  Update translations from Transifex
  MBS-12420: Support comma-separated IDs for edit search types again (#2556)
  MBS-12416: Avoid wrapping in menu links
  MBS-12415: Stop most interface wrapping
  Update POT files using the production database
  Update translations from Transifex
  MBS-12328: Add #content to entity/Edits for artwork display (#2502)
  MBS-12336: Validate that Predicate::Set is being passed integers (#2509)
  MBS-12333: Block smart links: share.amuse.io (#2501)
  Run all sql before enabling replication triggers (#2539)
  MBS-12412: Fail gracefully if requesting an invalid ID in edit-cover-art (#2547)
  MBS-12387: Add titles to URL editor edit buttons (#2550)
  Bump react-table to 7.8.0 (#2551)
  MBS-12407: Add tobarandualchais.co.uk to the other dbs whitelist (#2545)
  MBS-12280 (II): Add target _blank to more edit form links (#2546)
  MBS-12413: Use uri_escape_utf8 on Data::Wikidoc (#2548)
  ExportAllTables.t: remove --nodbmirror2 flags (#2538)
  MBS-12398: Update release_meta.amazon_asin (#2543)
  Bump Flow to 0.179.0
  JSON dumps: ignore more useless primary/foreign keys (#2536)
  MBS-12404: Replace Text::Unaccent (#2542)
  MBS-12353: Actually check proposed ratings are allowed (#2511)
  MBS-8193 / MBS-12332: Wrap absurdly long lines (#2516)
  MBS-11694: Cleanup /intent/user Twitter URLs (#2519)
  MBS-8875: Improve CatNoLooksLikeASIN regexp (#2520)
  MBS-12347: Also show RG artist in autocomplete if type is null (#2517)
  MBS-12385: Add report for digital releases with mail order rels (#2535)
  MBS-12212: Add tests for event filtering
  MBS-12212: Allow filtering event list by setlist contents
  MBS-12212: Add filtering to artists' events tab
  Update react, react-dom to v18 (#2544)
  Bump Flow to 0.178.1
  Upgrade Flow to 0.178.0
  Fix spammy PG warnings in ProcessReplicationChanges
  Remove BundleReplicationPackets
  MBS-4960: Give useful error when checking donation status fails
  MBS-12361: Allow navigation from donation check page
reosarevok added a commit that referenced this pull request Jun 7, 2022
* beta:
  Update POT files using the production database
  Update translations from Transifex
  MBS-12437: Limit overflow-wrap even more (#2561)
  Use flushSync to avoid render lag (fixes MBS-12424) (#2559)
  Update translations from Transifex
  MBS-12420: Support comma-separated IDs for edit search types again (#2556)
  MBS-12416: Avoid wrapping in menu links
  MBS-12415: Stop most interface wrapping
  Update POT files using the production database
  Update translations from Transifex
  MBS-12328: Add #content to entity/Edits for artwork display (#2502)
  MBS-12336: Validate that Predicate::Set is being passed integers (#2509)
  MBS-12333: Block smart links: share.amuse.io (#2501)
  Run all sql before enabling replication triggers (#2539)
  MBS-12412: Fail gracefully if requesting an invalid ID in edit-cover-art (#2547)
  MBS-12387: Add titles to URL editor edit buttons (#2550)
  Bump react-table to 7.8.0 (#2551)
  MBS-12407: Add tobarandualchais.co.uk to the other dbs whitelist (#2545)
  MBS-12280 (II): Add target _blank to more edit form links (#2546)
  MBS-12413: Use uri_escape_utf8 on Data::Wikidoc (#2548)
  ExportAllTables.t: remove --nodbmirror2 flags (#2538)
  MBS-12398: Update release_meta.amazon_asin (#2543)
  Bump Flow to 0.179.0
  JSON dumps: ignore more useless primary/foreign keys (#2536)
  MBS-12404: Replace Text::Unaccent (#2542)
  MBS-12353: Actually check proposed ratings are allowed (#2511)
  MBS-8193 / MBS-12332: Wrap absurdly long lines (#2516)
  MBS-11694: Cleanup /intent/user Twitter URLs (#2519)
  MBS-8875: Improve CatNoLooksLikeASIN regexp (#2520)
  MBS-12347: Also show RG artist in autocomplete if type is null (#2517)
  MBS-12385: Add report for digital releases with mail order rels (#2535)
  MBS-12212: Add tests for event filtering
  MBS-12212: Allow filtering event list by setlist contents
  MBS-12212: Add filtering to artists' events tab
  Update react, react-dom to v18 (#2544)
  Bump Flow to 0.178.1
  Upgrade Flow to 0.178.0
  Fix spammy PG warnings in ProcessReplicationChanges
  Remove BundleReplicationPackets
  MBS-4960: Give useful error when checking donation status fails
  MBS-12361: Allow navigation from donation check page
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants