Skip to content

Commit

Permalink
Add a simple suggestion in ValidatorOverloadedRetryAfter (#16277)
Browse files Browse the repository at this point in the history
## Description 

With some other nit changes:
- add `s` in `retry_after_secs`
- create a const for calculate the seed used in should_reject_tx, and
change interval to 30s (60s may be too long).

## Test Plan 

Existing tests. No logic change in this PR.

---
If your changes are not user-facing and do not break anything, you can
skip the following section. Otherwise, please briefly describe what has
changed under the Release Notes section.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
halfprice authored Feb 16, 2024
1 parent 2a4a9c7 commit 5322a2c
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 19 deletions.
43 changes: 26 additions & 17 deletions crates/sui-core/src/overload_monitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const STEADY_OVERLOAD_REDUCTION_PERCENTAGE: u32 = 10;
const EXECUTION_RATE_RATIO_FOR_COMPARISON: f64 = 0.9;
const ADDITIONAL_LOAD_SHEDDING: f64 = 0.1;

// The update interval of the random seed used to determine whether a txn should be rejected.
const SEED_UPDATE_DURATION_SECS: u64 = 30;

// Monitors the overload signals in `authority_state` periodically, and updates its `overload_info`
// when the signals indicates overload.
pub async fn overload_monitor(
Expand Down Expand Up @@ -215,11 +218,11 @@ fn check_overload_signals(
fn should_reject_tx(
load_shedding_percentage: u32,
tx_digest: TransactionDigest,
minutes_since_epoch: u64,
temporal_seed: u64,
) -> bool {
// TODO: we also need to add a secret salt (e.g. first consensus commit in the current epoch),
// to prevent gaming the system.
let mut hasher = XxHash64::with_seed(minutes_since_epoch);
let mut hasher = XxHash64::with_seed(temporal_seed);
hasher.write(tx_digest.inner());
let value = hasher.finish();
value % 100 < load_shedding_percentage as u64
Expand All @@ -230,17 +233,23 @@ pub fn overload_monitor_accept_tx(
load_shedding_percentage: u32,
tx_digest: TransactionDigest,
) -> SuiResult {
// Using the minutes_since_epoch as the hash seed to allow rejected transaction's
// retry to have a chance to go through in the future.
let minutes_since_epoch = SystemTime::now()
// Derive a random seed from the epoch time for transaction selection. Changing the seed every
// `SEED_UPDATE_DURATION_SECS` interval allows rejected transaction's retry to have a chance
// to go through in the future.
// Also, using the epoch time instead of randomly generating a seed allows that all validators
// makes the same decision.
let temporal_seed = SystemTime::now()
.duration_since(UNIX_EPOCH)
.expect("Sui did not exist prior to 1970")
.as_secs()
/ 60;

if should_reject_tx(load_shedding_percentage, tx_digest, minutes_since_epoch) {
// TODO: complete the suggestion for client retry deadline.
fp_bail!(SuiError::ValidatorOverloadedRetryAfter { retry_after_sec: 0 });
/ SEED_UPDATE_DURATION_SECS;

if should_reject_tx(load_shedding_percentage, tx_digest, temporal_seed) {
// TODO: using `SEED_UPDATE_DURATION_SECS` is a safe suggestion that the time based seed
// is definitely different by then. However, a shorter suggestion may be available.
fp_bail!(SuiError::ValidatorOverloadedRetryAfter {
retry_after_secs: SEED_UPDATE_DURATION_SECS
});
}
Ok(())
}
Expand Down Expand Up @@ -712,31 +721,31 @@ mod tests {
async fn test_txn_rejection_over_time() {
let start_time = Instant::now();
let mut digest = TransactionDigest::random();
let mut minutes_since_epoch = 28455473;
let mut temporal_seed = 1708108277 / SEED_UPDATE_DURATION_SECS;
let load_shedding_percentage = 50;

// Find a rejected transaction with 50% rejection rate.
while !should_reject_tx(load_shedding_percentage, digest, minutes_since_epoch)
while !should_reject_tx(load_shedding_percentage, digest, temporal_seed)
&& start_time.elapsed() < Duration::from_secs(30)
{
digest = TransactionDigest::random();
}

// It should always be rejected in the current minute.
// It should always be rejected using the current temporal_seed.
for _ in 0..100 {
assert!(should_reject_tx(
load_shedding_percentage,
digest,
minutes_since_epoch
temporal_seed
));
}

// It will be accepted in the future.
minutes_since_epoch += 1;
while should_reject_tx(load_shedding_percentage, digest, minutes_since_epoch)
temporal_seed += 1;
while should_reject_tx(load_shedding_percentage, digest, temporal_seed)
&& start_time.elapsed() < Duration::from_secs(30)
{
minutes_since_epoch += 1;
temporal_seed += 1;
}

// Make sure that the tests can finish within 30 seconds.
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,8 @@ pub enum SuiError {
#[error("Storage error: {0}")]
Storage(String),

#[error("Validator cannot handle the request at the moment. Please retry after at least {retry_after_sec}.")]
ValidatorOverloadedRetryAfter { retry_after_sec: u64 },
#[error("Validator cannot handle the request at the moment. Please retry after at least {retry_after_secs} seconds.")]
ValidatorOverloadedRetryAfter { retry_after_secs: u64 },
}

#[repr(u64)]
Expand Down

0 comments on commit 5322a2c

Please sign in to comment.