Skip to content

Commit

Permalink
Enable writeback cache by default (MystenLabs#19508)
Browse files Browse the repository at this point in the history
Writeback cache has been stable on all mysten fullnodes plus our testnet
validator for several months. Time to enable it by default everywhere
  • Loading branch information
mystenmark authored Oct 3, 2024
1 parent 50ddd15 commit 8761bf8
Show file tree
Hide file tree
Showing 14 changed files with 222 additions and 66 deletions.
13 changes: 11 additions & 2 deletions crates/sui-config/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,16 +206,25 @@ pub struct NodeConfig {
pub enable_db_write_stall: Option<bool>,
}

#[derive(Clone, Debug, Deserialize, Serialize, Default)]
#[derive(Clone, Debug, Deserialize, Serialize)]
#[serde(rename_all = "kebab-case")]
pub enum ExecutionCacheConfig {
#[default]
PassthroughCache,
WritebackCache {
/// Maximum number of entries in each cache. (There are several different caches).
/// If None, the default of 10000 is used.
max_cache_size: Option<usize>,
},
}

impl Default for ExecutionCacheConfig {
fn default() -> Self {
ExecutionCacheConfig::WritebackCache {
max_cache_size: None,
}
}
}

#[derive(Clone, Copy, Debug, Deserialize, Serialize)]
#[serde(rename_all = "lowercase")]
pub enum ServerType {
Expand Down
4 changes: 3 additions & 1 deletion crates/sui-core/src/authority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5034,7 +5034,7 @@ impl AuthorityState {
.protocol_config()
.simplified_unwrap_then_delete();
self.get_accumulator_store()
.iter_live_object_set(include_wrapped_object)
.iter_cached_live_object_set_for_testing(include_wrapped_object)
}

#[cfg(test)]
Expand All @@ -5053,6 +5053,8 @@ impl AuthorityState {
.bulk_insert_genesis_objects(objects)?;
self.get_object_cache_reader()
.force_reload_system_packages(&BuiltInFramework::all_package_ids());
self.get_reconfig_api()
.clear_state_end_of_epoch(&self.execution_lock_for_reconfiguration().await);
Ok(())
}
}
Expand Down
8 changes: 4 additions & 4 deletions crates/sui-core/src/authority/authority_test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ pub async fn publish_package_on_single_authority(
dep_original_addresses: impl IntoIterator<Item = (&'static str, ObjectID)>,
dep_ids: Vec<ObjectID>,
state: &Arc<AuthorityState>,
) -> SuiResult<(ObjectID, ObjectRef)> {
) -> SuiResult<(TransactionDigest, (ObjectID, ObjectRef))> {
let mut build_config = BuildConfig::new_for_testing();
for (addr_name, obj_id) in dep_original_addresses {
build_config
Expand Down Expand Up @@ -561,7 +561,7 @@ pub async fn publish_package_on_single_authority(
.find(|c| matches!(c.1, Owner::AddressOwner(..)))
.unwrap()
.0;
Ok((package_id, cap_object))
Ok((*effects.transaction_digest(), (package_id, cap_object)))
}

pub async fn upgrade_package_on_single_authority(
Expand All @@ -574,7 +574,7 @@ pub async fn upgrade_package_on_single_authority(
dep_original_addresses: impl IntoIterator<Item = (&'static str, ObjectID)>,
dep_id_mapping: impl IntoIterator<Item = (&'static str, ObjectID)>,
state: &Arc<AuthorityState>,
) -> SuiResult<ObjectID> {
) -> SuiResult<(TransactionDigest, ObjectID)> {
let package = build_test_modules_with_dep_addr(path, dep_original_addresses, dep_id_mapping);

let with_unpublished_deps = false;
Expand Down Expand Up @@ -606,5 +606,5 @@ pub async fn upgrade_package_on_single_authority(
.unwrap()
.0
.0;
Ok(package_id)
Ok((*effects.transaction_digest(), package_id))
}
6 changes: 6 additions & 0 deletions crates/sui-core/src/authority/test_authority_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,12 @@ impl<'a> TestAuthorityBuilder<'a> {
.await
.unwrap();

state
.get_cache_commit()
.commit_transaction_outputs(epoch_store.epoch(), &[*genesis.transaction().digest()])
.await
.unwrap();

// We want to insert these objects directly instead of relying on genesis because
// genesis process would set the previous transaction field for these objects, which would
// change their object digest. This makes it difficult to write tests that want to use
Expand Down
20 changes: 10 additions & 10 deletions crates/sui-core/src/execution_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl ExecutionCacheTraitPointers {
}
}

static ENABLE_WRITEBACK_CACHE_ENV_VAR: &str = "ENABLE_WRITEBACK_CACHE";
static DISABLE_WRITEBACK_CACHE_ENV_VAR: &str = "DISABLE_WRITEBACK_CACHE";

#[derive(Debug)]
pub enum ExecutionCacheConfigType {
Expand All @@ -130,12 +130,12 @@ pub fn choose_execution_cache(config: &ExecutionCacheConfig) -> ExecutionCacheCo
}
}

if std::env::var(ENABLE_WRITEBACK_CACHE_ENV_VAR).is_ok()
|| matches!(config, ExecutionCacheConfig::WritebackCache { .. })
if std::env::var(DISABLE_WRITEBACK_CACHE_ENV_VAR).is_ok()
|| matches!(config, ExecutionCacheConfig::PassthroughCache)
{
ExecutionCacheConfigType::WritebackCache
} else {
ExecutionCacheConfigType::PassthroughCache
} else {
ExecutionCacheConfigType::WritebackCache
}
}

Expand All @@ -158,13 +158,13 @@ pub fn build_execution_cache_from_env(
) -> ExecutionCacheTraitPointers {
let execution_cache_metrics = Arc::new(ExecutionCacheMetrics::new(prometheus_registry));

if std::env::var(ENABLE_WRITEBACK_CACHE_ENV_VAR).is_ok() {
if std::env::var(DISABLE_WRITEBACK_CACHE_ENV_VAR).is_ok() {
ExecutionCacheTraitPointers::new(
WritebackCache::new(store.clone(), execution_cache_metrics).into(),
PassthroughCache::new(store.clone(), execution_cache_metrics).into(),
)
} else {
ExecutionCacheTraitPointers::new(
PassthroughCache::new(store.clone(), execution_cache_metrics).into(),
WritebackCache::new(store.clone(), execution_cache_metrics).into(),
)
}
}
Expand Down Expand Up @@ -847,11 +847,11 @@ macro_rules! implement_passthrough_traits {

impl ExecutionCacheReconfigAPI for $implementor {
fn insert_genesis_object(&self, object: Object) -> SuiResult {
self.store.insert_genesis_object(object)
self.insert_genesis_object_impl(object)
}

fn bulk_insert_genesis_objects(&self, objects: &[Object]) -> SuiResult {
self.store.bulk_insert_genesis_objects(objects)
self.bulk_insert_genesis_objects_impl(objects)
}

fn revert_state_update(&self, digest: &TransactionDigest) -> SuiResult {
Expand Down
1 change: 0 additions & 1 deletion crates/sui-core/src/execution_cache/object_locks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ impl ObjectLocks {

let live_digest = live_object.digest();
if obj_ref.2 != live_digest {
debug!("object digest mismatch: {:?} vs {:?}", obj_ref, live_digest);
return Err(SuiError::UserInputError {
error: UserInputError::InvalidObjectDigest {
object_id: obj_ref.0,
Expand Down
8 changes: 8 additions & 0 deletions crates/sui-core/src/execution_cache/passthrough_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,14 @@ impl PassthroughCache {
})
.ok();
}

fn bulk_insert_genesis_objects_impl(&self, objects: &[Object]) -> SuiResult {
self.store.bulk_insert_genesis_objects(objects)
}

fn insert_genesis_object_impl(&self, object: Object) -> SuiResult {
self.store.insert_genesis_object(object)
}
}

impl ObjectCacheRead for PassthroughCache {
Expand Down
18 changes: 18 additions & 0 deletions crates/sui-core/src/execution_cache/writeback_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,16 +1146,34 @@ impl WritebackCache {
self.packages.invalidate(object_id);
}
self.cached.object_by_id_cache.invalidate(object_id);
self.cached.object_cache.invalidate(object_id);
}

for ObjectKey(object_id, _) in outputs.deleted.iter().chain(outputs.wrapped.iter()) {
self.cached.object_by_id_cache.invalidate(object_id);
self.cached.object_cache.invalidate(object_id);
}

// Note: individual object entries are removed when clear_state_end_of_epoch_impl is called
Ok(())
}

fn bulk_insert_genesis_objects_impl(&self, objects: &[Object]) -> SuiResult {
self.store.bulk_insert_genesis_objects(objects)?;
for obj in objects {
dbg!("invalidagted!", obj.id());
self.cached.object_cache.invalidate(&obj.id());
self.cached.object_by_id_cache.invalidate(&obj.id());
}
Ok(())
}

fn insert_genesis_object_impl(&self, object: Object) -> SuiResult {
self.cached.object_by_id_cache.invalidate(&object.id());
self.cached.object_cache.invalidate(&object.id());
self.store.insert_genesis_object(object)
}

pub fn clear_caches_and_assert_empty(&self) {
info!("clearing caches");
self.cached.clear_and_assert_empty();
Expand Down
6 changes: 4 additions & 2 deletions crates/sui-core/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,11 @@ pub async fn send_and_confirm_transaction(
.epoch_store_for_testing()
.protocol_config()
.simplified_unwrap_then_delete();
let mut state = state_acc.accumulate_live_object_set(include_wrapped_tombstone);
let mut state =
state_acc.accumulate_cached_live_object_set_for_testing(include_wrapped_tombstone);
let (result, _execution_error_opt) = authority.try_execute_for_test(&certificate).await?;
let state_after = state_acc.accumulate_live_object_set(include_wrapped_tombstone);
let state_after =
state_acc.accumulate_cached_live_object_set_for_testing(include_wrapped_tombstone);
let effects_acc = state_acc.accumulate_effects(
vec![result.inner().data().clone()],
epoch_store.protocol_config(),
Expand Down
Loading

0 comments on commit 8761bf8

Please sign in to comment.