-
Notifications
You must be signed in to change notification settings - Fork 264
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
feat: speed up rollback command #620
Changes from 6 commits
dc90e0b
a9d704d
325405d
76c0d19
1493ae5
e0cfa42
36f3727
2e9db23
255d416
e3f445f
06baf1f
7e88c5c
9b36a27
d64ee05
1f7dd20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -616,19 +616,31 @@ func (tree *MutableTree) LoadVersion(targetVersion int64) (int64, error) { | |
// LoadVersionForOverwriting attempts to load a tree at a previously committed | ||
// version, or the latest version below it. Any versions greater than targetVersion will be deleted. | ||
func (tree *MutableTree) LoadVersionForOverwriting(targetVersion int64) (int64, error) { | ||
latestVersion, err := tree.LoadVersion(targetVersion) | ||
return tree.LoadVersionForOverwritingWithMode(targetVersion, false) | ||
} | ||
|
||
func (tree *MutableTree) LoadVersionForOverwritingWithMode(targetVersion int64, fastMode bool) (int64, error) { | ||
var latestVersion int64 | ||
var err error | ||
if !fastMode { | ||
latestVersion, err = tree.LoadVersion(targetVersion) | ||
} else { | ||
latestVersion, err = tree.LazyLoadVersion(targetVersion) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a param name clash:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually this flag is mainly related with rollback, should we rename to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, but in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sense, then |
||
} | ||
if err != nil { | ||
return latestVersion, err | ||
} | ||
|
||
if err = tree.ndb.DeleteVersionsFrom(targetVersion + 1); err != nil { | ||
if err = tree.ndb.DeleteVersionsFrom(targetVersion+1, fastMode); err != nil { | ||
return latestVersion, err | ||
} | ||
|
||
if !tree.skipFastStorageUpgrade { | ||
if err := tree.enableFastStorageAndCommitLocked(); err != nil { | ||
return latestVersion, err | ||
} | ||
} else if err = tree.ndb.Commit(); err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @yihuang seems we still need commit if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, so it don't commit previously? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yup, I wonder if need fix as a separate bug There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see, the current rollback cmd maybe don't work at all if the fast node is disabled 😂 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Closing this PR, let's limit the options. |
||
return latestVersion, err | ||
} | ||
|
||
tree.ndb.resetLatestVersion(latestVersion) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -420,7 +420,7 @@ func (ndb *nodeDB) DeleteVersion(version int64, checkLatestVersion bool) error { | |
} | ||
|
||
// DeleteVersionsFrom permanently deletes all tree versions from the given version upwards. | ||
func (ndb *nodeDB) DeleteVersionsFrom(version int64) error { | ||
func (ndb *nodeDB) DeleteVersionsFrom(version int64, fastMode bool) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto, also update doc string |
||
latest, err := ndb.getLatestVersion() | ||
if err != nil { | ||
return err | ||
|
@@ -444,34 +444,53 @@ func (ndb *nodeDB) DeleteVersionsFrom(version int64) error { | |
|
||
// First, delete all active nodes in the current (latest) version whose node version is after | ||
// the given version. | ||
err = ndb.deleteNodesFrom(version, root) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
// Next, delete orphans: | ||
// - Delete orphan entries *and referred nodes* with fromVersion >= version | ||
// - Delete orphan entries with toVersion >= version-1 (since orphans at latest are not orphans) | ||
err = ndb.traverseOrphans(func(key, hash []byte) error { | ||
var fromVersion, toVersion int64 | ||
orphanKeyFormat.Scan(key, &toVersion, &fromVersion) | ||
|
||
if fromVersion >= version { | ||
if err = ndb.batch.Delete(key); err != nil { | ||
return err | ||
} | ||
if err = ndb.batch.Delete(ndb.nodeKey(hash)); err != nil { | ||
return err | ||
if !fastMode { | ||
err = ndb.deleteNodesFrom(version, root) | ||
if err != nil { | ||
return err | ||
} | ||
// Next, delete orphans: | ||
// - Delete orphan entries *and referred nodes* with fromVersion >= version | ||
// - Delete orphan entries with toVersion >= version-1 (since orphans at latest are not orphans) | ||
err = ndb.traverseOrphans(func(key, hash []byte) error { | ||
var fromVersion, toVersion int64 | ||
orphanKeyFormat.Scan(key, &toVersion, &fromVersion) | ||
|
||
if fromVersion >= version { | ||
if err = ndb.batch.Delete(key); err != nil { | ||
return err | ||
} | ||
if err = ndb.batch.Delete(ndb.nodeKey(hash)); err != nil { | ||
return err | ||
} | ||
ndb.nodeCache.Remove(hash) | ||
} else if toVersion >= version-1 { | ||
if err = ndb.batch.Delete(key); err != nil { | ||
return err | ||
} | ||
} | ||
ndb.nodeCache.Remove(hash) | ||
} else if toVersion >= version-1 { | ||
if err = ndb.batch.Delete(key); err != nil { | ||
return err | ||
return nil | ||
}) | ||
} else { | ||
err = ndb.traverseOrphansVersion(version-1, func(key, hash []byte) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we change a version here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks, let's add a comment in the code. |
||
var fromVersion, toVersion int64 | ||
orphanKeyFormat.Scan(key, &toVersion, &fromVersion) | ||
if fromVersion >= version { | ||
if err = ndb.batch.Delete(key); err != nil { | ||
return err | ||
} | ||
if err = ndb.batch.Delete(ndb.nodeKey(hash)); err != nil { | ||
return err | ||
} | ||
ndb.nodeCache.Remove(hash) | ||
} else if toVersion >= version-1 { | ||
if err = ndb.batch.Delete(key); err != nil { | ||
return err | ||
} | ||
} | ||
} | ||
return nil | ||
}) | ||
|
||
return nil | ||
}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The inner function is the same in both cases. Let's avoid copy-paste. |
||
} | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -489,24 +508,29 @@ func (ndb *nodeDB) DeleteVersionsFrom(version int64) error { | |
} | ||
|
||
// Delete fast node entries | ||
err = ndb.traverseFastNodes(func(keyWithPrefix, v []byte) error { | ||
key := keyWithPrefix[1:] | ||
fastNode, err := fastnode.DeserializeNode(key, v) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
if version <= fastNode.GetVersionLastUpdatedAt() { | ||
if err = ndb.batch.Delete(keyWithPrefix); err != nil { | ||
// Delete step will be skipped with enable fastMode | ||
// with the assumption that the rollback happens offline | ||
// since fast nodes will be reinforced when next start up | ||
if !fastMode { | ||
err = ndb.traverseFastNodes(func(keyWithPrefix, v []byte) error { | ||
key := keyWithPrefix[1:] | ||
fastNode, err := fastnode.DeserializeNode(key, v) | ||
if err != nil { | ||
return err | ||
} | ||
ndb.fastNodeCache.Remove(key) | ||
} | ||
return nil | ||
}) | ||
|
||
if err != nil { | ||
return err | ||
if version <= fastNode.GetVersionLastUpdatedAt() { | ||
if err = ndb.batch.Delete(keyWithPrefix); err != nil { | ||
return err | ||
} | ||
ndb.fastNodeCache.Remove(key) | ||
} | ||
return nil | ||
}) | ||
|
||
if err != nil { | ||
return err | ||
} | ||
} | ||
|
||
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.
let's add a doc string explaining the new parameter.