channeldb: avoid creating empty buckets #1745
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Needs a rebase! |
Roasbeef
previously approved these changes
Dec 4, 2018
halseth
force-pushed
the
channeldb-avoid-create-buckets
branch
3 times, most recently
from
befdf47
to
82bd43a
Compare
|
Rebased! |
cfromknecht
reviewed
Dec 4, 2018
halseth
force-pushed
the
channeldb-avoid-create-buckets
branch
from
82bd43a
to
9f7daea
Compare
wpaulino
reviewed
Dec 6, 2018
| return err | ||
| nodes := tx.Bucket(nodeBucket) | ||
| if nodes == nil { | ||
| return ErrSourceNodeNotSet |
There was a problem hiding this comment.
Same answer as above :)
| nodes, err := tx.CreateBucketIfNotExists(nodeBucket) | ||
| if err != nil { | ||
| return err | ||
| nodes := tx.Bucket(nodeBucket) |
There was a problem hiding this comment.
Looks like the buckets above also don't need to be created.
There was a problem hiding this comment.
True, but it will change the behavior of the method slightly, which cascades into tests etc.. Tried to keep behavior changes minimal for now :)
Avoids creating a bucket unneccessarily.
It would return ErrEdgeNotFound when edge not found in bucket anyway.
Instead return ErrEdgeNotFound if buckets don't exist. This would be returned anyway, when the chanIndex is checked for the channel point in question.
Instead return ErrGraphNodeNotFound directly. If the node bucket was created it would be empty, and the call delChannelByEdge -> fetchChanEdgePolicies -> fetchChanEdgePolicy -> deserializeChanEdgePolicy -> fetchLightningNode would return this error anyway.
instead return ErEdgeNotFound, which would be returned anyway when querying the edgeIndex for the channel.
We are deleting what's in the bucket, so if it doesn't exist, just return early with a non-error.
Lets us avoid passing the tx from pruneGraphNodes
Instead return ErrSourceNodeNotSet directly. This would be returned from the call pruneGraphNodes -> sourceNode anyway.
halseth
force-pushed
the
channeldb-avoid-create-buckets
branch
from
9f7daea
to
70aeda1
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR removes several cases of the call to
CreateBucketIfNotExistswithin the channel database, from calls where we were not adding anything to the buckets, but rather querying or deleting. This ensures we are not creating empty buckets unnecessarily, and makes it easier to keep track of where the different buckets are initially created.Note that this change shouldn't alter behaviour of the DB, and aims to return the same errors as would have been returned even when the buckets were created.
TODO: