graph/db: remove various kvdb parameters from exported methods
#9695
Conversation
This file contains hidden or 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 Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
Later on we will create an interface for the persisted graph data. We want this interface to be as small and as neat as possible. In preparation for this, we remove this unused `Wipe` method.
ellemouton
force-pushed
the
graph-clean-up
branch
from
a0cf5f1 to
2a02689
Compare
bhandras
approved these changes
Apr 10, 2025
| return nil | ||
| }) | ||
| require.NoError(t, err) | ||
| require.Empty(t, expectedSrcChans) |
guggero
approved these changes
Apr 10, 2025
| *models.ChannelEdgePolicy) error) error { | ||
|
|
||
| return nodeTraversal(nil, nodePub[:], c.db, cb) | ||
| return nodeTraversal(nil, nodePub[:], c.db, func(_ kvdb.RTx, |
There was a problem hiding this comment.
I assume the callback here will be the target of another refactor, probably not as easy to remove yet.
There was a problem hiding this comment.
the call-back for nodeTraversal? I think we can just leave that as is no? we wont use that for SQL stuff.
maybe i'm misunderstanding
ellemouton
force-pushed
the
graph-clean-up
branch
from
2a02689 to
2697e55
Compare
In preparation for creating a clean interface for the graph store, we want to hide anything that is DB specific from the exposed methods on the interface. Currently the `ForEachNodeChannel` and the `FetchOtherNode` methods of the `KVStore` expose a `kvdb.RTx` parameter which is bbolt specific. There is only one call-site of `ForEachNodeChannel` actually makes use of the passed `kvdb.RTx` parameter, and that is in the `establishPersistentConnections` method of the `server` which then passes the tx parameter to `FetchOtherNode`. So to clean-up the interface such that the `kvdb.RTx` is no longer exposed: we instead create one new method called `ForEachSourceNodeChannel` which can be used to replace the above mentioned call-site. So as of this commit, all the remaining call-site of `ForEachNodeChannel` pass in a nil param for `kvdb.RTx` - meaning we can remove the parameter in a future commit.
Unexport the KVStore `FetchOtherNode` and `ForEachNodeChannelTx` methods so that fewer exposed methods are leaking implementation details.
Replace all calls to bbolt specific methods on the KVStore to instead use exported methods on the KVStore that are more db-agnostic.
ellemouton
force-pushed
the
graph-clean-up
branch
from
2697e55 to
219ea13
Compare
Since we have not removed all call-sites that make use of this parameter, we can remove it. This helps hide DB-specific details from the interface we will introduce for the graph store.
ellemouton
force-pushed
the
graph-clean-up
branch
from
219ea13 to
f6bdb87
Compare
|
the lint and check commits checks both failing due to "out of memory" error. |
ellemouton
merged commit 225a694
into
lightningnetwork:elle-graph
32 of 35 checks passed
60 tasks
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.
In this PR, we continue to clean-up some of the exported methods on the
KVStoresuch that they dont expose details about the underlying DB (such askvdb.RTx). This is a step towards us creating an interface that can be implemented by a native SQL graph backend.Part of #9795