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

change(state): Write finalized blocks to the state in a separate thread, to avoid network and RPC hangs #5134

Merged
merged 47 commits into from
Sep 28, 2022
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
0d43b69
Add a new block commit task and channels, that don't do anything yet
teor2345 Sep 12, 2022
63486a1
Add last_block_hash_sent to the state service, to avoid database acce…
teor2345 Sep 13, 2022
c819fb2
Update last_block_hash_sent regardless of commit errors
teor2345 Sep 13, 2022
5c8af25
Rename a field to StateService.max_queued_finalized_height
teor2345 Sep 13, 2022
5ea30f7
Commit finalized blocks to the state in a separate task
teor2345 Sep 13, 2022
1ba6e5b
Check for panics in the block write task
teor2345 Sep 14, 2022
a34032d
Wait for the block commit task in tests, and check for errors
teor2345 Sep 14, 2022
2479be0
Always run a proptest that sleeps once
teor2345 Sep 14, 2022
725adec
Add extra debugging to state shutdowns
teor2345 Sep 14, 2022
9dd5e07
Work around a RocksDB shutdown bug
teor2345 Sep 14, 2022
bbd44b8
Close the finalized block channel when we're finished with it
teor2345 Sep 15, 2022
71f4a59
Only reset state queue once per error
teor2345 Sep 15, 2022
932d9b4
Update some TODOs
teor2345 Sep 15, 2022
d8c63d3
Add a module doc comment
teor2345 Sep 15, 2022
d051464
Drop channels and check for closed channels in the block commit task
teor2345 Sep 15, 2022
1d5455e
Close state channels and tasks on drop
teor2345 Sep 15, 2022
de6ff83
Remove some duplicate fields across StateService and ReadStateService
teor2345 Sep 15, 2022
406fcc2
Try tweaking the shutdown steps
teor2345 Sep 15, 2022
01a4a79
Update and clarify some comments
teor2345 Sep 16, 2022
863bb74
Clarify another comment
teor2345 Sep 16, 2022
6ede492
Don't try to cancel RocksDB background work on drop
teor2345 Sep 16, 2022
1657f28
Fix up some comments
teor2345 Sep 16, 2022
6a50d0e
Remove some duplicate code
teor2345 Sep 16, 2022
439c3dd
Remove redundant workarounds for shutdown issues
teor2345 Sep 16, 2022
e87dd1d
Remode a redundant channel close in the block commit task
teor2345 Sep 20, 2022
04be50b
Remove a mistaken `!force` shutdown condition
teor2345 Sep 20, 2022
cd9bd54
Remove duplicate force-shutdown code and explain it better
teor2345 Sep 20, 2022
622dd5c
Improve RPC error logging
teor2345 Sep 20, 2022
3e95876
Wait for chain tip updates in the RPC tests
teor2345 Sep 20, 2022
82f355a
Wait 2 seconds for chain tip updates before skipping them
teor2345 Sep 20, 2022
199df99
Remove an unnecessary block_in_place()
teor2345 Sep 20, 2022
f597518
Fix some test error messages that were changed by earlier fixes
teor2345 Sep 21, 2022
4710047
Expand some comments, fix typos
teor2345 Sep 21, 2022
0531cd2
Actually drop children of failed blocks
teor2345 Sep 21, 2022
ef35dea
Explain why we drop descendants of failed blocks
teor2345 Sep 21, 2022
61f33f6
Clarify a comment
teor2345 Sep 23, 2022
8029e74
Wait for chain tip updates in a failing test on macOS
teor2345 Sep 23, 2022
5af44a2
Clean duplicate finalized blocks when the non-finalized state activates
teor2345 Sep 23, 2022
b43c433
Send an error when receiving a duplicate finalized block
teor2345 Sep 23, 2022
6eaafc1
Update checkpoint block behaviour, document its consensus rule
teor2345 Sep 23, 2022
9be4996
Wait for chain tip changes in inbound_block_height_lookahead_limit test
teor2345 Sep 23, 2022
b66ef8f
Wait for the genesis block to commit in the fake peer set mempool tests
teor2345 Sep 26, 2022
83a7f2f
Disable unreliable mempool verification check in the send transaction…
teor2345 Sep 27, 2022
8e137ea
Appease rustfmt
teor2345 Sep 27, 2022
04a5808
Use clear_finalized_block_queue() everywhere that blocks are dropped
teor2345 Sep 27, 2022
96d3966
Document how Finalized and NonFinalized clones are different
teor2345 Sep 27, 2022
b150e3f
Use the same check as commit_finalized() for finalized block heights
teor2345 Sep 27, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions zebra-rpc/src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ where
hashes
.iter()
.map(|(tx_loc, tx_id)| {
// TODO: downgrade to debug, because there's nothing the user can do
// Check that the returned transactions are in chain order.
assert!(
*tx_loc > last_tx_location,
"Transactions were not in chain order:\n\
Expand Down Expand Up @@ -931,7 +931,7 @@ where
let satoshis = u64::from(utxo_data.3.value);

let output_location = *utxo_data.2;
// TODO: downgrade to debug, because there's nothing the user can do
// Check that the returned UTXOs are in chain order.
assert!(
output_location > last_output_location,
"UTXOs were not in chain order:\n\
Expand Down Expand Up @@ -1272,17 +1272,19 @@ impl GetRawTransaction {
/// Check if provided height range is valid for address indexes.
fn check_height_range(start: Height, end: Height, chain_height: Height) -> Result<()> {
if start == Height(0) || end == Height(0) {
return Err(Error::invalid_params(
"Start and end are expected to be greater than zero",
));
return Err(Error::invalid_params(format!(
"start {start:?} and end {end:?} must both be greater than zero"
)));
}
if end < start {
return Err(Error::invalid_params(
"End value is expected to be greater than or equal to start",
));
if start > end {
return Err(Error::invalid_params(format!(
"start {start:?} must be less than or equal to end {end:?}"
)));
}
if start > chain_height || end > chain_height {
return Err(Error::invalid_params("Start or end is outside chain range"));
return Err(Error::invalid_params(format!(
"start {start:?} and end {end:?} must both be less than or equal to the chain tip {chain_height:?}"
)));
}

Ok(())
Expand Down
6 changes: 3 additions & 3 deletions zebra-rpc/src/methods/tests/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ async fn rpc_getaddresstxids_invalid_arguments() {
.unwrap_err();
assert_eq!(
error.message,
"End value is expected to be greater than or equal to start".to_string()
"start Height(2) must be less than or equal to end Height(1)".to_string()
);

// call the method with start equal zero
Expand All @@ -411,7 +411,7 @@ async fn rpc_getaddresstxids_invalid_arguments() {
.unwrap_err();
assert_eq!(
error.message,
"Start and end are expected to be greater than zero".to_string()
"start Height(0) and end Height(1) must both be greater than zero".to_string()
);

// call the method outside the chain tip height
Expand All @@ -427,7 +427,7 @@ async fn rpc_getaddresstxids_invalid_arguments() {
.unwrap_err();
assert_eq!(
error.message,
"Start or end is outside chain range".to_string()
"start Height(1) and end Height(11) must both be less than or equal to the chain tip Height(10)".to_string()
);

mempool.expect_no_requests().await;
Expand Down
2 changes: 2 additions & 0 deletions zebra-state/src/arbitrary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ use crate::{

/// Mocks computation done during semantic validation
pub trait Prepare {
/// Runs block semantic validation computation, and returns the result.
/// Test-only method.
fn prepare(self) -> PreparedBlock;
}

Expand Down
5 changes: 3 additions & 2 deletions zebra-state/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
extern crate tracing;

#[cfg(any(test, feature = "proptest-impl"))]
mod arbitrary;
pub mod arbitrary;

mod config;
pub mod constants;
mod error;
Expand All @@ -39,7 +40,7 @@ pub use service::{

#[cfg(any(test, feature = "proptest-impl"))]
pub use service::{
arbitrary::populated_state,
arbitrary::{populated_state, CHAIN_TIP_UPDATE_WAIT_LIMIT},
chain_tip::{ChainTipBlock, ChainTipSender},
init_test, init_test_services,
};
Expand Down
34 changes: 29 additions & 5 deletions zebra-state/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,20 +381,44 @@ pub enum Request {
/// documentation for details.
CommitBlock(PreparedBlock),

/// Commit a finalized block to the state, skipping all validation.
/// Commit a checkpointed block to the state, skipping most block validation.
///
/// This is exposed for use in checkpointing, which produces finalized
/// blocks. It is the caller's responsibility to ensure that the block is
/// valid and final. This request can be made out-of-order; the state service
/// will queue it until its parent is ready.
/// semantically valid and final. This request can be made out-of-order;
/// the state service will queue it until its parent is ready.
///
/// Returns [`Response::Committed`] with the hash of the newly committed
/// block, or an error.
///
/// This request cannot be cancelled once submitted; dropping the response
/// future will have no effect on whether it is eventually processed.
/// Duplicate requests should not be made, because it is the caller's
/// responsibility to ensure that each block is valid and final.
/// Duplicate requests will replace the older duplicate, and return an error
/// in its response future.
///
/// # Note
///
/// Finalized and non-finalized blocks are an internal Zebra implementation detail.
/// There is no difference between these blocks on the network, or in Zebra's
/// network or syncer implementations.
///
/// # Consensus
///
/// Checkpointing is allowed under the Zcash "social consensus" rules.
/// Zebra checkpoints both settled network upgrades, and blocks past the rollback limit.
/// (By the time Zebra release is tagged, its final checkpoint is typically hours or days old.)
///
/// > A network upgrade is settled on a given network when there is a social consensus
/// > that it has activated with a given activation block hash. A full validator that
/// > potentially risks Mainnet funds or displays Mainnet transaction information to a user
/// > MUST do so only for a block chain that includes the activation block of the most
/// > recent settled network upgrade, with the corresponding activation block hash.
/// > ...
/// > A full validator MAY impose a limit on the number of blocks it will “roll back”
/// > when switching from one best valid block chain to another that is not a descendent.
/// > For `zcashd` and `zebra` this limit is 100 blocks.
///
/// <https://zips.z.cash/protocol/protocol.pdf#blockchain>
///
/// # Correctness
///
Expand Down
Loading