graph: move graph cache out of CRUD layer & move topology change subscription #9529
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
|
Important Review skippedAuto reviews are limited to specific labels. 🏷️ Labels to auto review (1)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ellemouton
changed the title
ellemouton
force-pushed
the
graph7
branch
3 times, most recently
from
dc553c3 to
5e7084c
Compare
ellemouton
changed the title
Rename it to kv_store.go so that we can re-use the graph.go file name later on. We will use it to house the _new_ ChannelGraph when the existing ChannelGraph is renamed to more clearly reflect its responsibilities as the CRUD layer.
In this commit, we rename the existing ChannelGraph struct to KVStore to better reflect its responsibilities as the CRUD layer. We then introduce a new ChannelGraph struct which will eventually be the layer above the CRUD layer in which we will handle cacheing and topology subscriptions. For now, however, it houses only the KVStore. This means that all calls to the KVStore will now go through this layer of indirection first. This will allow us to slowly move the graph Cache management out of the KVStore and into the new ChannelGraph layer.
Namespace these options so that we can introduce separate options for the new ChannelGraph.
And use this struct to pass NewChannelGraph anything it needs to be able to init the KVStore that it houses. This will allow us to add ChannelGraph specific options.
In this commit, we let the ChannelGraph be responsible for populating the graphCache and then passing it to the KVStore. This is a first step in moving the graphCache completely out of the KVStore layer.
This commit moves the graph cache checks for FetchNodeFeatures, ForEachNodeDirectedChannel, GraphSession and ForEachNodeCached from the KVStore to the ChannelGraph.
Here, we move the graph cache writes for AddLightningNode, DeleteLightningNode, AddChannelEdge and MarkEdgeLive to the ChannelGraph. Since these are writes, the cache is only updated if the DB write is successful.
And update cache outside the method rather. This will make it easier to completely move the cache write out to the ChannelGraph layer.
Here we move the cache writes for DisconnectBlockAtHeight and DeleteChannelEdges to the ChannelGraph.
In preparation for moving the cache write completely out of KVStore, we move the cache write up one layer.
Here, we move the business logic in FilterKnownChanIDs from the CRUD layer to the ChannelGraph layer. We also add a test for the logic.
To the ChannelGraph.
We do this in preparation for moving channel cache population logic out of the constructor and into the Start method. We also will later on (when topology subscription is moved to the ChannelGraph), have a goroutine that will need to be kicked off and stopped.
…ction In this commit, we move the graph cache population logic out of the ChannelGraph constructor and into its Start method instead.
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 demonstrates the end goal of some micro PRs that will be put up towards this graph abstraction project . This is so that reviewers can get an overview of the final end goal while not needing to commit to reviewing the whole thing at once.
This is one big refactor. Only 1 logical change is made in 1 commit and it is very minor (2 DB transactions used instead of 1).
This does 2 things:
Please see #9494 for context on why we are doing the above 🙏
So this is just for a concept ACK of these 2 goals by the reviewers. Once ACK'd, I'll start churning the smaller PRs towards these 2 goals 🫡