Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gossip notifications / cluster node notifications to geyser #34618

Closed

Conversation

musitdev
Copy link

@musitdev musitdev commented Jan 2, 2024

Problem

See issue #32934

Summary of Changes

This PR is a POC to add cluster node's info to the Geyzer plugin notification.

The LegacyContactInfo received on GOSSIP network are forwarded with the Geyzer interface to a client subscriber.

The PR contains:

A GRPC Yellowstone plugin update has been done to test the process and see the notified data volume. You can see the PR here.

An example of client can be found here. It subscribes to the Cluster info node notification and call from time to time the RPC get_cluster_nodes to compare with the notified data.

Early results

The test has been done on Testnet cluster with the Solana version 1.17.5 and Yellowstone plugin 1.11.0+solana.1.17.5.

Comparison with RPC call

It takes a few second for the plugin to get the majority of the Testnet cluster nodes and after 30s there's less than 10 nodes in error when comparing with the RPC call (around 3000 nodes in the cluster).

After a few minutes, Geyzer and RPC has the same list of cluster's nodes. From time to time they can have some difference (a few nodes) from one part or the other depending on the notification time propagation of update or removal.

Data volume

I use Tcpdump to monitor the packet send between the validator grpc server and the client to build the get_cluster_nodes RPC call nodes list.

This graphic is an example of network activity and bandwidth use. The volume of data transmitted is between 0,5 Mbits/s and 3 Mbits/s.

I've done some monitoring of the network connection between the grpc server and the client for the cluster node info notification.

This is the graphic of the data transferred between the validator grpc server and the client to build the get_cluster_nodes RPC call nodes list.

Network metrics

@musitdev musitdev changed the title Add gossip notifications / cluster node notifications to geyserAdd cluster info on geyzer 1.17.5 Add gossip notifications / cluster node notifications to geyser Jan 2, 2024
@willhickey
Copy link
Contributor

v1.17 is nearly at the end of its stabilization and only accepting PRs for bug fixes. Please consider opening this PR against master instead.

@musitdev
Copy link
Author

musitdev commented Jan 3, 2024

v1.17 is nearly at the end of its stabilization and only accepting PRs for bug fixes. Please consider opening this PR against master instead.

I've used the 1.17 to have the grpc geyzer plugin compatible in version. I've created this PR first to do a test to validate the concept. It wasn't intended to be merged.

If it seems to you interesting, I can move my change to the current master and try to adapt the grpc plugin to work with it.

Copy link
Contributor

@lijunwangs lijunwangs left a comment

Choose a reason for hiding this comment

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

Please move the PR to against the master branch.

}
};
//notify geyzer interface
self.notify_clusterinfo_update(self.table.get(&gossip_label));
Copy link
Contributor

Choose a reason for hiding this comment

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

What about for the upsert case?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry, I didn't understand. It's clusterinfo you prefer cluster_info?

use crate::crds_value::CrdsData;

impl Crds {
/// Notified when an account is updated at runtime, due to transaction activities
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct these copy-pasted comments please

Copy link
Author

Choose a reason for hiding this comment

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

Corrected. I change the comments.

@@ -533,6 +550,8 @@ impl Crds {
self.shards.remove(index, &value);
match value.value.data {
CrdsData::LegacyContactInfo(_) => {
//notify geyzer interface
Copy link
Contributor

Choose a reason for hiding this comment

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

nits: --> geyser

Copy link
Author

Choose a reason for hiding this comment

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

Corrected

@@ -2,6 +2,8 @@
/// the GeyserPlugin trait to work with the runtime.
/// In addition, the dynamic library must export a "C" function _create_plugin which
/// creates the implementation of the plugin.
use solana_sdk::pubkey::Pubkey;
Copy link
Contributor

Choose a reason for hiding this comment

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

Follow our coding style, put them in "use {...}";

Copy link
Author

Choose a reason for hiding this comment

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

Corrected


/// Called when a cluster info is removed on gossip network.
#[allow(unused_variables)]
fn notify_clusterinfo_remove(&self, pubkey: &Pubkey) -> Result<()> {
Copy link
Contributor

Choose a reason for hiding this comment

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

When is this called? notify_clusterinfo_remove?

Copy link
Author

Choose a reason for hiding this comment

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

You can find the call here:

self.notify_clusterinfo_remove(&value.value.pubkey());

@musitdev
Copy link
Author

musitdev commented Jan 9, 2024

I've created a new PR to start from master. I've issues with master rebase so I apply the commit to master branch directly and use a new branch.
The new PR is #34713

@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jan 24, 2024
@github-actions github-actions bot closed this Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need:merge-assist stale [bot only] Added to stale content; results in auto-close after a week.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants