Skip to content

Commit

Permalink
Remove everything related to accounts_db_stats_recomputed_on_upgrade
Browse files Browse the repository at this point in the history
  • Loading branch information
jasonz-dfinity committed Jan 31, 2025
1 parent fe325f0 commit d5dc422
Show file tree
Hide file tree
Showing 4 changed files with 0 additions and 42 deletions.
1 change: 0 additions & 1 deletion rs/backend/nns-dapp.did
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ type Stats =
stable_memory_size_bytes: opt nat64;
wasm_memory_size_bytes: opt nat64;
migration_countdown: opt nat32;
accounts_db_stats_recomputed_on_upgrade: opt bool;
};

type PerformanceCount =
Expand Down
20 changes: 0 additions & 20 deletions rs/backend/src/accounts_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,25 +47,8 @@ pub struct AccountsStore {
accounts_db: schema::proxy::AccountsDbAsProxy,

accounts_db_stats: AccountsDbStats,
accounts_db_stats_recomputed_on_upgrade: IgnoreEq<Option<bool>>,
}

/// A wrapper around a value that returns true for `PartialEq` and `Eq` equality checks, regardless of the value.
///
/// This is intended to be used on incidental, volatile fields. A structure containing such a field will typically wish to disregard the field in any comparison.
#[derive(Default)]
struct IgnoreEq<T>(T)
where
T: Default;

impl<T: Default> PartialEq for IgnoreEq<T> {
fn eq(&self, _: &Self) -> bool {
true
}
}

impl<T: Default> Eq for IgnoreEq<T> {}

impl fmt::Debug for AccountsStore {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
Expand Down Expand Up @@ -684,7 +667,6 @@ impl AccountsStore {
stats.sub_accounts_count = self.accounts_db_stats.sub_accounts_count;
stats.hardware_wallet_accounts_count = self.accounts_db_stats.hardware_wallet_accounts_count;
stats.migration_countdown = Some(self.accounts_db.migration_countdown());
stats.accounts_db_stats_recomputed_on_upgrade = self.accounts_db_stats_recomputed_on_upgrade.0;
}

#[must_use]
Expand Down Expand Up @@ -792,7 +774,6 @@ impl StableState for AccountsStore {
Option<AccountsDbStats>,
) = Candid::from_bytes(bytes).map(|c| c.0)?;

let accounts_db_stats_recomputed_on_upgrade = IgnoreEq(Some(accounts_db_stats_maybe.is_none()));
let Some(accounts_db_stats) = accounts_db_stats_maybe else {
return Err("Accounts DB stats should be present since the stable structures migration.".to_string());
};
Expand All @@ -803,7 +784,6 @@ impl StableState for AccountsStore {
// State::from(Partitions) so it doesn't matter what we set here.
accounts_db: AccountsDbAsProxy::default(),
accounts_db_stats,
accounts_db_stats_recomputed_on_upgrade,
})
}
}
Expand Down
2 changes: 0 additions & 2 deletions rs/backend/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,6 @@ pub struct Stats {
pub stable_memory_size_bytes: Option<u64>,
pub wasm_memory_size_bytes: Option<u64>,
pub migration_countdown: Option<u32>, // When non-zero, a migration is in progress.
/// Whether account stats were recomputed on upgrade.
pub accounts_db_stats_recomputed_on_upgrade: Option<bool>,
}

/// Encodes the metrics into the format scraped by the monitoring system.
Expand Down
19 changes: 0 additions & 19 deletions scripts/nns-dapp/migration-test.canister
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,6 @@ get_upgrade_invariant_stats() {
dfx canister call nns-dapp get_stats | idl2json | jq '{accounts_count, sub_accounts_count}'
}

# Verifies that accounts were not recomputed on upgrade.
# Recomputing on upgrade is very expensive for stable memory.
assert_accounts_db_stats_are_not_recomputed_on_upgrade() {
local expected_value actual_value
expected_value="[false]"
actual_value="$(dfx canister call nns-dapp get_stats | idl2json | jq -c '.accounts_db_stats_recomputed_on_upgrade')"
[[ "$expected_value" == "$actual_value" ]] || {
echo "ERROR: Recompute stats on upgrade is not as expected."
echo "Expected: $expected_value"
echo "Actual: $actual_value"
exit 1
}
}

get_accounts_count() {
dfx canister call nns-dapp get_stats | idl2json | jq -r .accounts_count
}
Expand Down Expand Up @@ -152,11 +138,6 @@ upgrade_nnsdapp() {
exit 1
} >&2
fi

# Going forwards, this should always be false. Historically it was always true.
#
# TODO: Delete the code path for recomputing stats on upgrade.
assert_accounts_db_stats_are_not_recomputed_on_upgrade
}

# This file is intended to be sourced, but if called directly, offer some help
Expand Down

0 comments on commit d5dc422

Please sign in to comment.