Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Plumb Entry notification into plugin manager#30910

Merged
CriesofCarrots merged 1 commit intosolana-labs:masterfrom
CriesofCarrots:geyser-manager-entry
Mar 28, 2023
Merged

Plumb Entry notification into plugin manager#30910
CriesofCarrots merged 1 commit intosolana-labs:masterfrom
CriesofCarrots:geyser-manager-entry

Conversation

@CriesofCarrots
Copy link
Copy Markdown
Contributor

Problem

#30872 added geyser interfaces for receiving/handling Entry notifications, but the validator plugin manager isn't yet able to actually send these notifications.

Summary of Changes

Add plumbing toward Entry notifications. This adds the geyser-plugin-manager pieces needed to construct and send the notifications, but still does not actually send them.

Comment thread rpc/src/lib.rs
@@ -1,5 +1,6 @@
#![allow(clippy::integer_arithmetic)]
mod cluster_tpu_info;
pub mod entry_notifier_interface;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I put this in solana-rpc because I thought we could piggyback off the TransactionStatusService (with a new TransactionStatusMessage variant) to call notify_entry()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Might move this in a follow-up, depending on where the Entry notification service ends up.

return;
}

let entry_info = Self::build_replica_entry_info(slot, index, entry);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While a lot of this file is copied from transaction_notifier.rs, I moved this line down, as there's no reason to build the entry info if plugins.is_empty(). This is probably inconsequential in terms of perf, but we could make this change in the other notifiers.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 27, 2023

Codecov Report

Merging #30910 (2d2f930) into master (24be850) will decrease coverage by 0.1%.
The diff coverage is 0.0%.

@@            Coverage Diff            @@
##           master   #30910     +/-   ##
=========================================
- Coverage    81.5%    81.5%   -0.1%     
=========================================
  Files         726      727      +1     
  Lines      204914   204955     +41     
=========================================
+ Hits       167065   167072      +7     
- Misses      37849    37883     +34     

@CriesofCarrots CriesofCarrots marked this pull request as draft March 27, 2023 22:57
Copy link
Copy Markdown
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.

Looks good!

@CriesofCarrots CriesofCarrots marked this pull request as ready for review March 28, 2023 18:54
@CriesofCarrots CriesofCarrots merged commit 9323015 into solana-labs:master Mar 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants