feat: speed up rollback command #620
Conversation
* skip deleteNodesFrom * lazyLoad latest version
| if !fastMode { | ||
| latestVersion, err = tree.LoadVersion(targetVersion) | ||
| } else { | ||
| latestVersion, err = tree.LazyLoadVersion(targetVersion) |
There was a problem hiding this comment.
There is a param name clash:
- seams that we want to use
fastModewhen we don't want to usefastCache - so let's rename
fastModeparameter to something different, eglazy boolornoFastCache bool.
There was a problem hiding this comment.
Actually this flag is mainly related with rollback, should we rename to FastRollback or OfflineRollback.
There was a problem hiding this comment.
OK, but in the DeleteVersionsFrom it's related to the fast cache
There was a problem hiding this comment.
Make sense, then OfflineRollback might better since assumption is based on offline and re-index on restart.
mutable_tree.go
Outdated
| return tree.LoadVersionForOverwritingWithMode(targetVersion, false) | ||
| } | ||
|
|
||
| func (tree *MutableTree) LoadVersionForOverwritingWithMode(targetVersion int64, fastMode bool) (int64, error) { |
There was a problem hiding this comment.
let's add a doc string explaining the new parameter.
nodedb.go
Outdated
| @@ -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.
ditto, also update doc string
nodedb.go
Outdated
| }) | ||
|
|
||
| return nil | ||
| }) |
There was a problem hiding this comment.
The inner function is the same in both cases. Let's avoid copy-paste.
nodedb.go
Outdated
| return nil | ||
| }) | ||
| } else { | ||
| err = ndb.traverseOrphansVersion(version-1, func(key, hash []byte) error { |
There was a problem hiding this comment.
why do we change a version here?
There was a problem hiding this comment.
since toVersion in orphan records is current version-1
There was a problem hiding this comment.
thanks, let's add a comment in the code.
| if !fastMode { | ||
| latestVersion, err = tree.LoadVersion(targetVersion) | ||
| } else { | ||
| latestVersion, err = tree.LazyLoadVersion(targetVersion) |
There was a problem hiding this comment.
OK, but in the DeleteVersionsFrom it's related to the fast cache
|
I did a cleaner version: #636 |
| if err := tree.enableFastStorageAndCommitLocked(); err != nil { | ||
| return latestVersion, err | ||
| } | ||
| } else if err = tree.ndb.Commit(); err != nil { |
There was a problem hiding this comment.
@yihuang seems we still need commit if skipFastStorageUpgrade?
There was a problem hiding this comment.
I see, so it don't commit previously?
There was a problem hiding this comment.
yup, I wonder if need fix as a separate bug
There was a problem hiding this comment.
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.
Closing this PR, let's limit the options.
Update