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

Fullnode rpc to exit with unsafe config #3082

Merged

Conversation

aeyakovenko
Copy link
Member

@aeyakovenko aeyakovenko commented Mar 4, 2019

Problem

Leader failure fault tolerance for an in-process LocalCluster requires the node to expose an exit mechanism via an RPC. This mechanism is only available with the JsonRpcConfig::Unsafe fullnode option.

Summary of Changes

  • Config option to guard availability of the exit rpc
  • test to check that default is safe
  • test to verify nodes exit and fails to exit with the safe config
  • pass an exit signal from the fullnode through to TPU and TVU

Fixes #

@codecov
Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #3082 into master will decrease coverage by 0.3%.
The diff coverage is 70%.

@@           Coverage Diff            @@
##           master   #3082     +/-   ##
========================================
- Coverage    78.5%   78.1%   -0.4%     
========================================
  Files         136     137      +1     
  Lines       19944   20134    +190     
========================================
+ Hits        15660   15744     +84     
- Misses       4284    4390    +106

@garious
Copy link
Contributor

garious commented Mar 4, 2019

Can you find a better term than "unsafe"? Rust uses that term for the user to communicate to the compiler "your memory-safety rules are too conservative, back off, I got this".

@aeyakovenko
Copy link
Member Author

@garious, debug seems to benign. A super safe way to do this would be to have 1 config item per “unsafe” test. So I could call this DebugRpcFullnodeExit

@garious
Copy link
Contributor

garious commented Mar 4, 2019

sure or "TestOnly"

@mvines
Copy link
Contributor

mvines commented Mar 4, 2019

I'd rather the flag be very specific, DebugEnableRpcFullnodeExit, or something. This avoids creating a generic home for people to later sneak in other unrelated flags.

But this change overall is great, once it lands I'll expose it as a flag to the fullnode program since I'd like to use it in the bash integration tests and network deployment scripts

Copy link
Contributor

@garious garious left a comment

Choose a reason for hiding this comment

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

Just the naming nit

@aeyakovenko aeyakovenko added the automerge Merge this Pull Request automatically once CI passes label Mar 4, 2019
@solana-grimes solana-grimes merged commit 3a4018c into solana-labs:master Mar 4, 2019
bw-solana pushed a commit to bw-solana/solana that referenced this pull request Oct 30, 2024
* extract solana-nonce crate

* activate solana-nonce/serde in solana-program

* update lock file

* post-rebase fixes

* add docsrs metadata

Co-authored-by: Jon C <me@jonc.dev>

* add doc_auto_cfg

* reorganise modules

---------

Co-authored-by: Jon C <me@jonc.dev>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants