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
pg: add schema_version and upgrade for epoch_reports #959
Conversation
224a5eb
to
7734f42
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks ok to me and was able to upgrade without problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests well. A couple of comments, but g2g.
|
||
current, err = DBVersion(db) | ||
if err != nil { | ||
return fmt.Errorf("failed to get DB version: %w", err) | ||
} | ||
log.Infof("Upgrades complete. DB is at version %d", current) | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going back to wisdom's comment
If setting the db version fails, we'll have an upgraded database with an older version number. Can avoid this by using a transaction for each upgrade, pass the transaction to each upgrade function, after the upgrade function returns without error, set the db version before committing. Any error should rollback all changes.
I seems to me like the idea was to use a single transaction for all upgrades and the db version, and not commit the transaction until the db version is recorded successfully. But the wording "using a transaction for each upgrade" seems to conflict with that assumption too, so I guess I don't know.
But wisdom's initial concern about an upgrade being performed but the version not being recorded has not been addressed, I don't think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it is addressed here: 6b4d7c7#diff-d5886fb14518c5d25332c7350f812301f9c6628f337111aa73258edac37c7472R309-R338
If anything with the actual upgrade or version setting fails, both roll back and you are at a consistent database level. That it is perhaps at an upgraded version but not the best version isn't a problem except that downgrading the software can't happen anymore. The DB is at least consistent for whatever version it is at.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we could technically wrap the loop of all the upgrades in the same tx like you're thinking, but I'm getting a bit anxious about the limits of postgresql here. I wonder if it's gonna be ok to have say 10 versions all modifying the tables heavily, essentially growing the size of the uncommitted transaction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh, dang I have to re-rig NewDEX for cancellation. It doesn't work presently to cancel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe it is addressed here: 6b4d7c7#diff-d5886fb14518c5d25332c7350f812301f9c6628f337111aa73258edac37c7472R309-R338
Ah. I see what I did wrong. Carry on.
This adds a schema_version column to the meta table, possibly creating the table if it was missing. This is the first defined upgrade to v1. This also adds a v2 upgrade that partially populates the epoch_stats table with historical data from the matches table. v2 upgrade creates epochs_report table, if it does not exist, and populates the table with partial historical data from the epochs and matches table. This includes match volumes, high/low/start/end rates, but does not include the booked volume statistics in the book_buys* and book_sells* columns since this data requires a book snapshot at the time of matching to generate. Test upgradeDB from v0 with DB snapshots from the following cases: - v0 DB with no meta table, as on 0.2+pre master presently - v0 DB with a meta table with state_hash from release-0.1 - v0 with meta table and a ton of matches for v2 upgrade The db snapshot archive with matches is quite large (server/db/driver/pg/dcrdex_test_db_v0-release-0.1-matches.sql.gz), but it is a good test as it has numberous epochs and matches, pseudo- randomly generated with loadbot running on release-0.1.
Upgrade on dex-test.ssgen.io with current commit went smoothly:
Very few actual trade matches on testnet, but tons of epochs. |
This adds DB scheme versioning to the postgresql backend (server/db/driver/pg) and implements two upgrades:
meta
table, remove thestate_hash
column and addschema_version
. Thestate_hash
column was used prior to swapper resume from db #856, and was left for this upgrade.epoch_reports
table (if not exists) and import match volume data for each epoch in theepochs
table by reading match data from thematches
table.Notes about the v2 upgrade (
epoch_reports
data import):epoch_reports
is created for each row of theepochs
table, even if there were no matches. This happens during normal operation.match_volume
,quote_volume
,low_rate
, andhigh_rate
.start_rate
andend_rate
fields are not based on book data as during normal operation where the book's midGap before/after matching is used. Insteadend_rate
, is set to the average of an epoch's high and low rates, whilestart_rate
is taken from the previous epoch with trade matches. As such, these are ballpark rates, not midGap book rates.book_buys*
andbook_sells*
fields (booked volume on either side within certain distance from midGap/market rate) are set to zero in the absence of a book snapshot to compute them. SeeInsertPartialEpochReport
.matches (epochidx, epochdur)
during the upgrade.Tested on a DB with 1,030,671 epochs containing 16,557 matches, the v2 upgrade completed in 76 seconds (~13.5k epochs/sec, 217 matches/sec):
Draft while I decide about
pgonline
tests.