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

Add GetSlotLeaders RPC method #16057

Merged
merged 1 commit into from
Mar 23, 2021
Merged

Add GetSlotLeaders RPC method #16057

merged 1 commit into from
Mar 23, 2021

Conversation

jstarry
Copy link
Contributor

@jstarry jstarry commented Mar 22, 2021 •
edited
Loading

Problem

No way to get the upcoming leaders without fetching the entire leader schedule. It's also a pain for clients to get upcoming leaders across epoch boundaries.

Summary of Changes

  • Add new getSlotLeaders method which is similar to getSlotLeader but fetches leaders for a given slot range
  • Use leader schedule cache to avoid expensive leader schedule computation and reduce memory overhead

Fixes #16048

@jstarry
Copy link
Contributor Author

jstarry commented Mar 22, 2021

@mvines since the leader schedule response returns epoch slot indices, I decided not to support cross-epoch responses. I don't think it's too awful for the client to handle when they are near an epoch boundary themselves.

@mvines
Copy link
Contributor

mvines commented Mar 22, 2021

@mvines since the leader schedule response returns epoch slot indices, I decided not to support cross-epoch responses. I don't think it's too awful for the client to handle when they are near an epoch boundary themselves.

Yeah, ok. I foresee a small stream of epoch-boundary bugs due to this but I see how the current interface makes doing better troublesome

sakridge
sakridge reviewed Mar 22, 2021
View reviewed changes
core/src/rpc.rs Outdated Show resolved Hide resolved
@jstarry jstarry force-pushed the leader-schedule-limit branch from 246575e to 335ce9d Compare March 23, 2021 15:59
@jstarry jstarry changed the title rpc: add limit option to getLeaderSchedule Add GetSlotLeaders RPC method Mar 23, 2021
@jstarry jstarry mentioned this pull request Mar 23, 2021
Closed
mvines
mvines approved these changes Mar 23, 2021
View reviewed changes
Copy link
Contributor

@mvines mvines left a comment

Choose a reason for hiding this comment

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

nice!

@jstarry jstarry added the automerge Merge this Pull Request automatically once CI passes label Mar 23, 2021
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Mar 23, 2021
@mergify
Copy link
Contributor

mergify bot commented Mar 23, 2021

automerge label removed due to a CI failure

@jstarry jstarry added the automerge Merge this Pull Request automatically once CI passes label Mar 23, 2021
@codecov
Copy link

codecov bot commented Mar 23, 2021

Codecov Report

Merging #16057 (335ce9d) into master (2c4ff00) will decrease coverage by 0.0%.
The diff coverage is 78.4%.

@@            Coverage Diff            @@
##           master   #16057     +/-   ##
=========================================
- Coverage    79.9%    79.9%   -0.1%     
=========================================
  Files         409      409             
  Lines      107698   107774     +76     
=========================================
+ Hits        86119    86162     +43     
- Misses      21579    21612     +33

@mergify mergify bot merged commit e7fd7d4 into solana-labs:master Mar 23, 2021
@mvines
Copy link
Contributor

mvines commented Mar 23, 2021

(adding v1.5 and v1.6, I don't want to wait for v1.7 to use this)

mergify bot pushed a commit that referenced this pull request Mar 23, 2021
@jstarry
rpc: add getSlotLeaders method (#16057)
2e2044a 
(cherry picked from commit e7fd7d4)

# Conflicts:
#	core/src/rpc.rs
#	core/src/validator.rs
@mergify mergify bot mentioned this pull request Mar 23, 2021
Merged
mergify bot pushed a commit that referenced this pull request Mar 23, 2021
@jstarry
rpc: add getSlotLeaders method (#16057)
b6c3ae3 
(cherry picked from commit e7fd7d4)
@mergify mergify bot mentioned this pull request Mar 23, 2021
Merged
mergify bot added a commit that referenced this pull request Mar 23, 2021
@mergify @jstarry
rpc: add getSlotLeaders method (#16057) (#16079)
c3c4991 
(cherry picked from commit e7fd7d4)

Co-authored-by: Justin Starry <justin@solana.com>
mvines pushed a commit that referenced this pull request Mar 23, 2021
@jstarry @mvines
rpc: add getSlotLeaders method (#16057)
c137335 
(cherry picked from commit e7fd7d4)
mvines pushed a commit that referenced this pull request Mar 23, 2021
@jstarry @mvines
rpc: add getSlotLeaders method (#16057)
3847caf 
(cherry picked from commit e7fd7d4)
mvines pushed a commit that referenced this pull request Mar 23, 2021
@jstarry @mvines
rpc: add getSlotLeaders method (#16057)
96943bd 
(cherry picked from commit e7fd7d4)
@jstarry
Copy link
Contributor Author

jstarry commented Mar 24, 2021

(adding v1.5 and v1.6, I don't want to wait for v1.7 to use this)

Slipped my mind, thanks

@jstarry jstarry deleted the leader-schedule-limit branch March 24, 2021 02:31
mergify bot added a commit that referenced this pull request Mar 24, 2021
@mergify @jstarry
rpc: add getSlotLeaders method (#16057) (#16078)
3317a14 
(cherry picked from commit e7fd7d4)

Co-authored-by: Justin Starry <justin@solana.com>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sakridge sakridge sakridge left review comments

@mvines mvines mvines approved these changes

Assignees
No one assigned
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rpc: no way to limit the response size of getLeaderSchedule
3 participants
@jstarry @mvines @sakridge