Skip to content

Commit

Permalink
remove partitioned reward test code
Browse files Browse the repository at this point in the history
  • Loading branch information
HaoranYi committed Dec 5, 2024
1 parent ad8aaab commit 4d7ce6a
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 329 deletions.
15 changes: 8 additions & 7 deletions accounts-db/src/accounts_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ use {
cache_hash_data::{CacheHashData, DeletionPolicy as CacheHashDeletionPolicy},
contains::Contains,
epoch_accounts_hash::EpochAccountsHashManager,
partitioned_rewards::{PartitionedEpochRewardsConfig, TestPartitionedEpochRewards},
partitioned_rewards::{
PartitionedEpochRewardsConfig, DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG,
},
read_only_accounts_cache::ReadOnlyAccountsCache,
sorted_storages::SortedStorages,
storable_accounts::{StorableAccounts, StorableAccountsBySlot},
Expand Down Expand Up @@ -506,7 +508,7 @@ pub const ACCOUNTS_DB_CONFIG_FOR_TESTING: AccountsDbConfig = AccountsDbConfig {
skip_initial_hash_calc: false,
exhaustively_verify_refcounts: false,
create_ancient_storage: CreateAncientStorage::Pack,
test_partitioned_epoch_rewards: TestPartitionedEpochRewards::CompareResults,
partitioned_epoch_rewards_config: DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG,
test_skip_rewrites_but_include_in_bank_hash: false,
storage_access: StorageAccess::Mmap,
scan_filter_for_shrinking: ScanFilter::OnlyAbnormalWithVerify,
Expand All @@ -532,7 +534,7 @@ pub const ACCOUNTS_DB_CONFIG_FOR_BENCHMARKS: AccountsDbConfig = AccountsDbConfig
skip_initial_hash_calc: false,
exhaustively_verify_refcounts: false,
create_ancient_storage: CreateAncientStorage::Pack,
test_partitioned_epoch_rewards: TestPartitionedEpochRewards::None,
partitioned_epoch_rewards_config: DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG,
test_skip_rewrites_but_include_in_bank_hash: false,
storage_access: StorageAccess::Mmap,
scan_filter_for_shrinking: ScanFilter::OnlyAbnormalWithVerify,
Expand Down Expand Up @@ -661,7 +663,7 @@ pub struct AccountsDbConfig {
pub exhaustively_verify_refcounts: bool,
/// how to create ancient storages
pub create_ancient_storage: CreateAncientStorage,
pub test_partitioned_epoch_rewards: TestPartitionedEpochRewards,
pub partitioned_epoch_rewards_config: PartitionedEpochRewardsConfig,
pub storage_access: StorageAccess,
pub scan_filter_for_shrinking: ScanFilter,
pub enable_experimental_accumulator_hash: bool,
Expand Down Expand Up @@ -1969,9 +1971,8 @@ impl AccountsDb {
accounts_hash_cache_path
});

let test_partitioned_epoch_rewards = accounts_db_config.test_partitioned_epoch_rewards;
let partitioned_epoch_rewards_config: PartitionedEpochRewardsConfig =
PartitionedEpochRewardsConfig::new(test_partitioned_epoch_rewards);
PartitionedEpochRewardsConfig::default();

let read_cache_size = accounts_db_config.read_cache_limit_bytes.unwrap_or((
Self::DEFAULT_MAX_READ_ONLY_CACHE_DATA_SIZE_LO,
Expand Down Expand Up @@ -2035,7 +2036,7 @@ impl AccountsDb {
Self::READ_ONLY_CACHE_MS_TO_SKIP_LRU_UPDATE,
),
write_cache_limit_bytes: accounts_db_config.write_cache_limit_bytes,
partitioned_epoch_rewards_config,
partitioned_epoch_rewards_config: accounts_db_config.partitioned_epoch_rewards_config,
exhaustively_verify_refcounts: accounts_db_config.exhaustively_verify_refcounts,
test_skip_rewrites_but_include_in_bank_hash: accounts_db_config
.test_skip_rewrites_but_include_in_bank_hash,
Expand Down
93 changes: 17 additions & 76 deletions accounts-db/src/partitioned_rewards.rs
Original file line number Diff line number Diff line change
@@ -1,98 +1,39 @@
//! Code related to partitioned rewards distribution
#[derive(Debug)]
/// # stake accounts to store in one block during partitioned reward interval
/// Target to store 64 rewards per entry/tick in a block. A block has a minimum of 64
/// entries/tick. This gives 4096 total rewards to store in one block.
/// This constant affects consensus.
const MAX_PARTITIONED_REWARDS_PER_BLOCK: u64 = 4096;

#[derive(Debug, Clone, Copy)]
/// Configuration options for partitioned epoch rewards.
/// This struct allows various forms of testing, especially prior to feature activation.
pub struct PartitionedEpochRewardsConfig {
/// number of stake accounts to store in one block during partitioned reward interval
/// normally, this is a number tuned for reasonable performance, such as 4096 accounts/block
/// if force_one_slot_partitioned_rewards, this will usually be u64::MAX so that all stake accounts are written in the first block
pub stake_account_stores_per_block: u64,
/// if true, end of epoch bank rewards will force using partitioned rewards distribution.
/// see `set_test_enable_partitioned_rewards`
pub test_enable_partitioned_rewards: bool,
/// if true, end of epoch non-partitioned bank rewards will test the partitioned rewards distribution vote and stake accounts
/// This has a significant performance impact on the first slot in each new epoch.
pub test_compare_partitioned_epoch_rewards: bool,
}

/// Convenient constant for default partitioned epoch rewards configuration
/// used for benchmarks and tests.
pub const DEFAULT_PARTITIONED_EPOCH_REWARDS_CONFIG: PartitionedEpochRewardsConfig =
PartitionedEpochRewardsConfig {
stake_account_stores_per_block: MAX_PARTITIONED_REWARDS_PER_BLOCK,
};

impl Default for PartitionedEpochRewardsConfig {
fn default() -> Self {
Self {
// # stake accounts to store in one block during partitioned reward interval
// Target to store 64 rewards per entry/tick in a block. A block has a minimum of 64
// entries/tick. This gives 4096 total rewards to store in one block.
// This constant affects consensus.
stake_account_stores_per_block: 4096,
test_enable_partitioned_rewards: false,
test_compare_partitioned_epoch_rewards: false,
stake_account_stores_per_block: MAX_PARTITIONED_REWARDS_PER_BLOCK,
}
}
}

#[derive(Debug, Default, Clone, Copy)]
pub enum TestPartitionedEpochRewards {
#[default]
None,
CompareResults,
ForcePartitionedEpochRewardsInOneBlock,
PartitionedEpochRewardsConfigRewardBlocks {
stake_account_stores_per_block: u64,
},
}

impl PartitionedEpochRewardsConfig {
pub fn new(test: TestPartitionedEpochRewards) -> Self {
match test {
TestPartitionedEpochRewards::None => Self::default(),
TestPartitionedEpochRewards::CompareResults => {
Self::set_test_compare_partitioned_epoch_rewards()
}
TestPartitionedEpochRewards::ForcePartitionedEpochRewardsInOneBlock => {
Self::set_test_enable_partitioned_rewards()
}
TestPartitionedEpochRewards::PartitionedEpochRewardsConfigRewardBlocks {
stake_account_stores_per_block,
} => {
Self::set_test_enable_partitioned_rewards_with_custom_number_of_stake_accounts_per_block(
stake_account_stores_per_block
)
}
}
}

/// All rewards will be distributed in the first block in the epoch, matching
/// consensus for the non-partitioned rewards, but running all the partitioned rewards
/// code.
fn set_test_enable_partitioned_rewards() -> Self {
Self {
stake_account_stores_per_block: u64::MAX,
test_enable_partitioned_rewards: true,
// irrelevant if we are not running old code path
test_compare_partitioned_epoch_rewards: false,
}
}

/// All rewards will be distributed in the first block in the epoch as normal.
/// Then, the partitioned rewards code will calculate expected results and compare to
/// the old code path's results.
fn set_test_compare_partitioned_epoch_rewards() -> Self {
Self {
test_compare_partitioned_epoch_rewards: true,
..PartitionedEpochRewardsConfig::default()
}
}

/// A method that configures how many reward reward calculation blocks and how many stake
/// accounts to store per reward block.
fn set_test_enable_partitioned_rewards_with_custom_number_of_stake_accounts_per_block(
stake_account_stores_per_block: u64,
) -> Self {
/// Only for tests and benchmarks
pub fn new_for_test(stake_account_stores_per_block: u64) -> Self {
Self {
stake_account_stores_per_block,
test_enable_partitioned_rewards: true,
// irrelevant if we are not running old code path
test_compare_partitioned_epoch_rewards: false,
}
}
}
11 changes: 0 additions & 11 deletions ledger-tool/src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use {
accounts_db::{AccountsDb, AccountsDbConfig, CreateAncientStorage},
accounts_file::StorageAccess,
accounts_index::{AccountsIndexConfig, IndexLimitMb, ScanFilter},
partitioned_rewards::TestPartitionedEpochRewards,
utils::create_and_canonicalize_directories,
},
solana_clap_utils::{
Expand Down Expand Up @@ -301,15 +300,6 @@ pub fn get_accounts_db_config(
..AccountsIndexConfig::default()
};

let test_partitioned_epoch_rewards =
if arg_matches.is_present("partitioned_epoch_rewards_compare_calculation") {
TestPartitionedEpochRewards::CompareResults
} else if arg_matches.is_present("partitioned_epoch_rewards_force_enable_single_slot") {
TestPartitionedEpochRewards::ForcePartitionedEpochRewardsInOneBlock
} else {
TestPartitionedEpochRewards::None
};

let accounts_hash_cache_path = arg_matches
.value_of("accounts_hash_cache_path")
.map(Into::into)
Expand Down Expand Up @@ -388,7 +378,6 @@ pub fn get_accounts_db_config(
.ok(),
exhaustively_verify_refcounts: arg_matches.is_present("accounts_db_verify_refcounts"),
skip_initial_hash_calc: arg_matches.is_present("accounts_db_skip_initial_hash_calculation"),
test_partitioned_epoch_rewards,
test_skip_rewrites_but_include_in_bank_hash: arg_matches
.is_present("accounts_db_test_skip_rewrites"),
create_ancient_storage,
Expand Down
23 changes: 0 additions & 23 deletions ledger-tool/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1136,29 +1136,6 @@ fn main() {
tasks and assert.",
),
)
.arg(
Arg::with_name("partitioned_epoch_rewards_compare_calculation")
.long("partitioned-epoch-rewards-compare-calculation")
.takes_value(false)
.help(
"Do normal epoch rewards distribution, but also calculate rewards \
using the partitioned rewards code path and compare the resulting \
vote and stake accounts",
)
.hidden(hidden_unless_forced()),
)
.arg(
Arg::with_name("partitioned_epoch_rewards_force_enable_single_slot")
.long("partitioned-epoch-rewards-force-enable-single-slot")
.takes_value(false)
.help(
"Force the partitioned rewards distribution, but distribute all \
rewards in the first slot in the epoch. This should match consensus \
with the normal rewards distribution.",
)
.conflicts_with("partitioned_epoch_rewards_compare_calculation")
.hidden(hidden_unless_forced()),
)
.arg(
Arg::with_name("print_accounts_stats")
.long("print-accounts-stats")
Expand Down
20 changes: 1 addition & 19 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2673,21 +2673,6 @@ impl Bank {
metrics,
);

// this checking of an unactivated feature can be enabled in tests or with a validator by passing `--partitioned-epoch-rewards-compare-calculation`
if self
.partitioned_epoch_rewards_config()
.test_compare_partitioned_epoch_rewards
{
// immutable `&self` to avoid side effects
(self as &Bank).compare_with_partitioned_rewards(
&stake_rewards,
&vote_account_rewards,
rewarded_epoch,
thread_pool,
null_tracer(),
);
}

self.store_stake_accounts(thread_pool, &stake_rewards, metrics);
let vote_rewards = self.store_vote_accounts(vote_account_rewards, metrics);
self.update_reward_history(stake_rewards, vote_rewards);
Expand Down Expand Up @@ -5658,16 +5643,13 @@ impl Bank {
let measure_total = Measure::start("");

let slot = self.slot();
let ignore = (!self.is_partitioned_rewards_feature_enabled()
&& self.force_partition_rewards_in_first_block_of_epoch())
.then_some(sysvar::epoch_rewards::id());
let (accounts_delta_hash, accounts_delta_hash_us) = measure_us!({
self.rc
.accounts
.accounts_db
.calculate_accounts_delta_hash_internal(
slot,
ignore,
None,
self.skipped_rewrites.lock().unwrap().clone(),
)
});
Expand Down
7 changes: 1 addition & 6 deletions runtime/src/bank/partitioned_epoch_rewards/calculation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,7 @@ impl Bank {

let slot = self.slot();
let distribution_starting_block_height =
// For live-cluster testing pre-activation
if self.force_partition_rewards_in_first_block_of_epoch() {
self.block_height()
} else {
self.block_height() + REWARD_CALCULATION_NUM_BLOCKS
};
self.block_height() + REWARD_CALCULATION_NUM_BLOCKS;

let num_partitions = stake_rewards_by_partition.len() as u64;

Expand Down
Loading

0 comments on commit 4d7ce6a

Please sign in to comment.