graph: move graph cache out of CRUD layer #9544
Draft
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 (
|
Pull reviewers statsStats of the last 30 days for lnd:
|
This was referenced Feb 24, 2025
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. We introduce the new ChannelGraph and rename the old one in the same commit so that all existing call-sites don't need to change at all :)
Since we have renamed a file housing some very old code, the linter has now run on all this code for the first time. So we gotta do some clean-up work here to make it happy.
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. Since the ChannelGraph is currently just a pass-through for any of the KVStore methods, all that needs to be done for calls to go via the ChannelGraph instead directly to the KVStore is for the ChannelGraph to go and implement those methods.
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.
This commit moves the cache writes for PruneGraphNodes and PruneGraph from the KVStore to the ChannelGraph.
Here, we move the business logic in FilterKnownChanIDs from the CRUD layer to the ChannelGraph layer. We also add a test for the logic.
From the KVStore to the ChannelGraph.
To the ChannelGraph.
ellemouton
force-pushed
the
elle-graphCacheBase
branch
from
5eb96dc to
3ce4799
Compare
|
^ this push was a pure rebase on master |
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 will be moved out of draft once all sub-PRs are merged into
elle-graphCacheBase.See this draft pr for the final goal here.
Note: depending on when this is ready, might need to move the release notes comment to 0.20
part of #9494