-
Notifications
You must be signed in to change notification settings - Fork 214
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
Rollbacks issue in stake pool worker #1200
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 <[email protected]>
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 <[email protected]>
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: