-
Notifications
You must be signed in to change notification settings - Fork 92
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
client/db: active and archived match buckets #1263
Conversation
1ebfedb
to
b67901d
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.
Works well. Some of the code looks really good to.
I didn't realized the UI was updated until just now. Looks good.
@@ -4673,7 +4673,7 @@ func (c *Core) dbTrackers(dc *dexConnection) (map[order.OrderID]*trackedTrade, e | |||
for _, dbMatch := range dbMatches { | |||
// Only trade matches are added to the matches map. Detect and skip | |||
// cancel order matches, which have an empty Address field. | |||
if dbMatch.Address == "" { | |||
if dbMatch.Address == "" { // only correct for maker's cancel match |
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.
This is a sorta redundant check anyway since MatchesForOrder
a few lines up should screen them out, but if it fails to do so, then this check is effective for screening out this trade order's cancel match, which was a problem in the past.
byteEpoch = uint16Bytes(uint16(order.OrderStatusEpoch)) | ||
byteBooked = uint16Bytes(uint16(order.OrderStatusBooked)) |
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.
Were used before the orders bucket was split.
2fb45ae
to
0ea934e
Compare
// TODO: consider changing upgradefunc to accept a bbolt.DB so it may create its | ||
// own transactions. The individual upgrades have to happen in separate | ||
// transactions because they get too big for a single transaction. Plus we | ||
// should consider switching certain upgrades from ForEach to a more cumbersome | ||
// Cursor()-based iteration that will facilitate partitioning updates into | ||
// smaller batches of buckets. |
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.
👍
matchesBucket := []byte("matches") | ||
return reloadMatchProofs(dbtx, skipCancels, matchesBucket) |
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.
archivedMatchesBucket
? Or would that be confusing?
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.
Just trying to make these older upgrades more robust to future changes to package level variables by defining the bucket/key names as they are known to be at that particular version.
oldMatchesBucket := []byte("matches") | ||
newActiveMatchesBucket := []byte("activeMatches") |
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.
Not using activeMatchesBucket
and archivedMatchesBucket
because this is better semantics?
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.
Missed this comment, sorry. Same consideration as other comment - a v7+ upgrade might change the package-level variables, but this v6 upgrade will always need to work with a specific key as it was in v5
d5fc628
to
57c65da
Compare
client/db: add v3upgrade to set match retired field for existing matches merge (*trackedTrade).matchIsActive and db.isRetiredMatch There is now one single function with the heuristics for determining a match's activeness, used by both trackedTrade and db.
57c65da
to
7909ec7
Compare
This redefines the v5 client db upgrade to introduce a new active matches bucket instead of creating a Retired flag on each match. The db and upgrade code is modeled after the active/archived orders buckets changes. client/db.MatchIsActive is updated to detect cancel matches as inactive. This is detected as MatchComplete and len(proof.Auth.InitSig) == 0 since cancel matches begin as complete and have no init/redeem data set in the Auth field. The Address field is only empty for the maker/trade match with a cancel, but for the taker/cancel's match with the trade it has to this point contained the canceled maker order's address. This also updates client/core to clear the Address field for both sides of a cancel match. Removing this useless data from a MetaMatch will save a bit of storage space, but previous cancel matches are not updated by the v6 upgrade. Shut down the db tests properly. upgrade logger, log matches upgrade outcome
e2b65e7
to
7a35720
Compare
This builds on #957. @itswisdomagain's commits will be merged without squashing into this. I've added his v5 test db generator app as well, just with an added cancel order match.
This redefines the v6 client db upgrade to introduce a new active matches bucket instead of creating a
Retired
flag on each match.The db and upgrade code is modeled after the active/archived orders buckets changes in #1071, whereby existing matches remain in what is to be the archived bucket, and only active ones are moved into the new active matches bucket.
client/db.MatchIsActive
is updated to detect cancel matches as inactive. This is detected asMatchComplete
andlen(proof.Auth.InitSig) == 0
since cancel matches begin as complete and have no init/redeem data set in theAuth
field.The
Address
field is only empty for the maker/trade match with a cancel, but for the taker/cancel's match with the trade it has to this point contained the canceled maker order's address.This also updates client/core to clear the
Address
field for both sides of a cancel match because it is pointless to store. Removing this useless data from aMetaMatch
will save a bit of storage space, but previous cancel matches are not updated by the v6 upgrade. Similarly, the server's swapper is updated to not send the maker address with a cancelmsgjson.Match
.Shut down the db tests properly.
Maybe 0.3