Rollbacks issue in stake pool worker #1200
Assignees
Comments
iohk-bors bot
added a commit
that referenced
this issue
Dec 18, 2019
1201: Fix pool registration rollback r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #1200 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - commit 18852c0 modify rollback property to capture the rollback in-within epoch problem. It now fails - commit 3a4d4fd store registration with full slot and not only epoch to allow correct rollback! This is however a DB breaking change which requires migration... Yet, there's no way to migrate from an existing database to this new schema. Since however this is a preferable solution and, re-syncing the pools data is cheap, we'll simply discards any existing database that has an invalid format and, resync from scratch :) - commit f21f4e3 add a regression test to show that migration from an old database doesn't work out of the box Currently fails with: ``` PersistError Database migration: manual intervention required. The unsafe actions are prefixed by '***' below: CREATE TEMP TABLE \"pool_registration_backup\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"margin\" INTEGER NOT NULL,\"cost\" INTEGER NOT NULL, PRIMARY KEY (\"pool_id\")); INSERT INTO \"pool_registration_backup\"(\"pool_id\",\"margin\",\"cost\") SELECT \"pool_id\",\"margin\",\"cost\" FROM \"pool_registration\"; *** DROP TABLE \"pool_registration\"; CREATE TABLE \"pool_registration\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"margin\" INTEGER NOT NULL,\"cost\" INTEGER NOT NULL, PRIMARY KEY (\"pool_id\")); INSERT INTO \"pool_registration\" SELECT \"pool_id\",\"slot\",\"margin\",\"cost\" FROM \"pool_registration_backup\"; DROP TABLE \"pool_registration_backup\";" ``` - commit 440e099 handle migration error in stake pool db layer As described in the previous commit. - commit 112927b better assertions on logs message for db migration tests # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <matthias.benkort@gmail.com>
iohk-bors bot
added a commit
that referenced
this issue
Dec 18, 2019
1201: Fix pool registration rollback r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #1200 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - commit 18852c0 modify rollback property to capture the rollback in-within epoch problem. It now fails - commit 3a4d4fd store registration with full slot and not only epoch to allow correct rollback! This is however a DB breaking change which requires migration... Yet, there's no way to migrate from an existing database to this new schema. Since however this is a preferable solution and, re-syncing the pools data is cheap, we'll simply discards any existing database that has an invalid format and, resync from scratch :) - commit f21f4e3 add a regression test to show that migration from an old database doesn't work out of the box Currently fails with: ``` PersistError Database migration: manual intervention required. The unsafe actions are prefixed by '***' below: CREATE TEMP TABLE \"pool_registration_backup\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"margin\" INTEGER NOT NULL,\"cost\" INTEGER NOT NULL, PRIMARY KEY (\"pool_id\")); INSERT INTO \"pool_registration_backup\"(\"pool_id\",\"margin\",\"cost\") SELECT \"pool_id\",\"margin\",\"cost\" FROM \"pool_registration\"; *** DROP TABLE \"pool_registration\"; CREATE TABLE \"pool_registration\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"margin\" INTEGER NOT NULL,\"cost\" INTEGER NOT NULL, PRIMARY KEY (\"pool_id\")); INSERT INTO \"pool_registration\" SELECT \"pool_id\",\"slot\",\"margin\",\"cost\" FROM \"pool_registration_backup\"; DROP TABLE \"pool_registration_backup\";" ``` - commit 440e099 handle migration error in stake pool db layer As described in the previous commit. - commit 112927b better assertions on logs message for db migration tests # Comments <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Context
Relates to #1181.
Steps to Reproduce
Expected behavior
Actual behavior
Resolution
Modified the database (in a non backward-compatible) way to store slot id instead of mere epoch number allowing for handling rollbacks correctly ==> Fix pool registration rollback #1201
Handled db unsafe migration for stake pools by re-creating the pool database from scratch if an old database is encountered (forcing a re-sync) ==> Fix pool registration rollback #1201
QA
Regression test added to make sure that migrations from previous versions work seamlessly: https://github.com/input-output-hk/cardano-wallet/blob/cf72aea27790efa8dc2f388d4c314ade85b8a0e4/lib/core/test/unit/Cardano/Pool/DB/SqliteSpec.hs#L57-L84
Modify the rollback property to work with slot id instead of epoch, failed initially, green after the fix: https://github.com/input-output-hk/cardano-wallet/blob/cf72aea27790efa8dc2f388d4c314ade85b8a0e4/lib/core/test/unit/Cardano/Pool/DB/Properties.hs#L383-L414
The text was updated successfully, but these errors were encountered: