Skip to content

Conversation

@jjyr
Copy link
Collaborator

@jjyr jjyr commented Sep 3, 2025

  1. Using indexmap struct to store graph channels, thus, we can support paginate in graph_channels.
  2. Remove Db access in graph_channels and graph_nodes Rpc
  3. Add total_count to result

Drawback:

Since our paginate based on return count, the query result may contains duplicates or missing for multiple pages. It is acceptable since graph_channels is mainly used for statistics, one way to workaround this problems is to set limit large enough to return all channels in once.

Also fixes #534


Updated the new commit use BTreeMap to store graph channels and nodes avoid the paginate drawback

.map(|node| JsonBytes::from_vec(node.cursor().to_bytes().into()))
.unwrap_or_default();
let channels = network_graph.get_channels_with_params(limit, after);
let last_cursor = JsonBytes::from_vec(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefer to use page and per_page as parameters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The param is still exposed as last_cursor for the external RPC, so even we change to network_graph.get_channels_with_params(per_page, page * per_page); the extern user feels no difference, and some unneccesary complexity is introduced: need to encode two (page, per_page) instead of one param (after) into last_cursor.

@chenyukang
Copy link
Collaborator

and to resolve #534,
please add a total_count field for nodes and channels graph rpc.

@chenyukang
Copy link
Collaborator

please do similar refactoring for graph_nodes rpc.

@jjyr
Copy link
Collaborator Author

jjyr commented Sep 3, 2025

and to resolve #534, please add a total_count field for nodes and channels graph rpc.

The tricky part is we need to track private_channels_count or public_channels_count, since graph_channels interface only return public channels, updated in the commits

@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 0% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 0.00%. Comparing base (8d98152) to head (e4a2170).
⚠️ Report is 8 commits behind head on develop.

Files with missing lines Patch % Lines
crates/fiber-lib/src/fiber/graph.rs 0.00% 28 Missing ⚠️
crates/fiber-lib/src/rpc/graph.rs 0.00% 25 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           develop    #879   +/-   ##
=======================================
  Coverage     0.00%   0.00%           
=======================================
  Files           61      61           
  Lines        38148   38144    -4     
=======================================
+ Misses       38148   38144    -4     
Flag Coverage Δ
unittests 0.00% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jjyr jjyr force-pushed the use-memory-graph-channels-to-return branch from 64f7d1d to b44759c Compare September 12, 2025 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Optimization]consider adding an API to query the channel synchronization progress.

3 participants