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

Add Incremental Snapshot support to RPC #19559

Merged
merged 11 commits into from
Sep 2, 2021

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Sep 1, 2021

Problem

There's no way to get incremental snapshot information from RPC.

Summary of Changes

  • Add new RPC method, getHighestSnapshotSlot that returns a SnapshotSlotInfo, which contains both the highest full snapshot slot, and the highest incremental snapshot slot based on the full snapshot.
  • Deprecate old RPC method, getSnapshotSlot
  • Update API docs

Fixes #19579

@mvines
Copy link
Contributor

mvines commented Sep 2, 2021

Is there a reason to have three methods here? It seems like just getSnapshotSlots would be sufficient?

Aside: I don't love the name getSnapshotSlots. To me that implies it'll return multiple slots that snapshots are available for, rather than a full/incremental snapshot slot pair

@brooksprumo
Copy link
Contributor Author

@mvines

Is there a reason to have three methods here? It seems like just getSnapshotSlots would be sufficient?

Good point. I this will likely come down to desired usage. I can see two use cases:

  1. A node already has a full snapshot, and wants to get the highest incremental snapshot for their full snapshot. This would only be useful as long as the slot hasn't crossed a full snapshot interval.
  2. A node just wants to get the latest and is OK downloading a new full snapshot if necessary.

Use-case 1 may not be all that valuable though. If a node A has a full snapshot at slot 800,000, and node B has a full snapshot at slot 810,000, then node A won't be able to use an incremental snapshot from node B anyway. So getting both the highest full snapshot and highest incremental snapshot would be the right way to go.

OK, I'm on board with just having a single RPC function!

Aside: I don't love the name getSnapshotSlots. To me that implies it'll return multiple slots that snapshots are available for, rather than a full/incremental snapshot slot pair

Yeah, the name sucks. How about one of these:

  • getSnapshotInfo
  • getSnapshotSlot2
  • getSnapshotSlotPair

I'm in favor of getSnapshotInfo.

@mvines
Copy link
Contributor

mvines commented Sep 2, 2021

How about: getHighestSnapshotSlot(full: Number, incremental: Number | undefined)

@mvines
Copy link
Contributor

mvines commented Sep 2, 2021

Oh, also lets turn the response from getHighestSnapshotSlot into a struct with these fields. If getSnapshotSlot returned an object instead of a simple number, we'd be able to extend it in a backwards-compatible way and not be stuck with creating a new method!

client/src/rpc_response.rs Outdated Show resolved Hide resolved
@brooksprumo brooksprumo marked this pull request as ready for review September 2, 2021 18:20
@codecov
Copy link

codecov bot commented Sep 2, 2021

Codecov Report

Merging #19559 (0e458ed) into master (82a6bbe) will increase coverage by 0.0%.
The diff coverage is 37.5%.

@@           Coverage Diff           @@
##           master   #19559   +/-   ##
=======================================
  Coverage    82.6%    82.7%           
=======================================
  Files         461      461           
  Lines      131212   131249   +37     
=======================================
+ Hits       108483   108557   +74     
+ Misses      22729    22692   -37     

@brooksprumo brooksprumo merged commit 8ac94b2 into solana-labs:master Sep 2, 2021
@brooksprumo brooksprumo deleted the iss-rpc-3 branch September 2, 2021 20:25
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Sep 2, 2021
#### Problem

There's no way to get incremental snapshot information from RPC.

#### Summary of Changes

- Add new RPC method, `getHighestSnapshotSlot` that returns a `SnapshotSlotInfo`, which contains both the highest full snapshot slot, and the highest incremental snapshot slot _based on_ the full snapshot.
- Deprecate old RPC method, `getSnapshotSlot`
- Update API docs

Fixes solana-labs#19579
jeffwashington pushed a commit to jeffwashington/solana that referenced this pull request Sep 3, 2021
#### Problem

There's no way to get incremental snapshot information from RPC.

#### Summary of Changes

- Add new RPC method, `getHighestSnapshotSlot` that returns a `SnapshotSlotInfo`, which contains both the highest full snapshot slot, and the highest incremental snapshot slot _based on_ the full snapshot.
- Deprecate old RPC method, `getSnapshotSlot`
- Update API docs

Fixes solana-labs#19579
frits-metalogix added a commit to identity-com/solana that referenced this pull request Nov 24, 2021
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.

Make RPC aware of incremental snapshots
2 participants