Skip to content

Commit

Permalink
Refactor: Renaming "database flags" to "database config" + Minor clea…
Browse files Browse the repository at this point in the history
…nup of their handling (#860)

## Summary

This is a 100% Pure refactoring.
Refactoring opportunity noticed when working on
https://radixdlt.atlassian.net/browse/LZ-18.
The entire scope is captured in the PR's title.

## Testing

Regression tests pass.
  • Loading branch information
jakrawcz-rdx authored Mar 6, 2024
2 parents db1f204 + a280092 commit 972b60e
Show file tree
Hide file tree
Showing 22 changed files with 98 additions and 91 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,11 @@
import com.radixdlt.sbor.codec.StructCodec;

/** Database configuration options */
public record DatabaseFlags(
public record DatabaseConfig(
boolean enableLocalTransactionExecutionIndex, boolean enableAccountChangeIndex) {
public static void registerCodec(CodecMap codecMap) {
codecMap.register(
DatabaseFlags.class,
codecs -> StructCodec.fromRecordComponents(DatabaseFlags.class, codecs));
DatabaseConfig.class,
codecs -> StructCodec.fromRecordComponents(DatabaseConfig.class, codecs));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public record StateManagerConfig(
Option<RustMempoolConfig> mempoolConfigOpt,
Option<VertexLimitsConfig> vertexLimitsConfigOpt,
DatabaseBackendConfig databaseBackendConfig,
DatabaseFlags databaseFlags,
DatabaseConfig databaseConfig,
LoggingConfig loggingConfig,
StateHashTreeGcConfig stateHashTreeGcConfig,
LedgerProofsGcConfig ledgerProofsGcConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public static void registerCodecsWithCodecMap(CodecMap codecMap) {
LeaderRoundCounter.registerCodec(codecMap);
InvalidCommitRequestError.registerCodec(codecMap);
DatabaseBackendConfig.registerCodec(codecMap);
DatabaseFlags.registerCodec(codecMap);
DatabaseConfig.registerCodec(codecMap);
TransactionHeader.registerCodec(codecMap);
CoreApiServerConfig.registerCodec(codecMap);
CoreApiServerFlags.registerCodec(codecMap);
Expand Down
10 changes: 6 additions & 4 deletions core-rust/state-manager/src/state_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ use crate::{
mempool_manager::MempoolManager,
mempool_relay_dispatcher::MempoolRelayDispatcher,
priority_mempool::PriorityMempool,
store::{jmt_gc::StateHashTreeGc, DatabaseBackendConfig, DatabaseFlags, RawDbMetricsCollector},
store::{
jmt_gc::StateHashTreeGc, DatabaseBackendConfig, DatabaseConfig, RawDbMetricsCollector,
},
transaction::{CachedCommittabilityValidator, CommittabilityValidator, TransactionPreviewer},
ActualStateManagerDatabase, PendingTransactionResultCache, ProtocolUpdateResult, StateComputer,
StateManagerDatabase,
Expand All @@ -104,7 +106,7 @@ pub struct StateManagerConfig {
pub mempool_config: Option<MempoolConfig>,
pub vertex_limits_config: Option<VertexLimitsConfig>,
pub database_backend_config: DatabaseBackendConfig,
pub database_flags: DatabaseFlags,
pub database_config: DatabaseConfig,
pub logging_config: LoggingConfig,
pub state_hash_tree_gc_config: StateHashTreeGcConfig,
pub ledger_proofs_gc_config: LedgerProofsGcConfig,
Expand All @@ -127,7 +129,7 @@ impl StateManagerConfig {
database_backend_config: DatabaseBackendConfig {
rocks_db_path: rocks_db_path.into(),
},
database_flags: DatabaseFlags::default(),
database_config: DatabaseConfig::default(),
logging_config: LoggingConfig::default(),
state_hash_tree_gc_config: StateHashTreeGcConfig::default(),
ledger_proofs_gc_config: LedgerProofsGcConfig::default(),
Expand Down Expand Up @@ -165,7 +167,7 @@ impl StateManager {
let network = config.network_definition.clone();

let db_path = PathBuf::from(config.database_backend_config.rocks_db_path.clone());
let raw_db = match StateManagerDatabase::new(db_path, config.database_flags.clone(), &network) {
let raw_db = match StateManagerDatabase::new(db_path, config.database_config.clone(), &network) {
Ok(db) => db,
Err(error) => {
match error {
Expand Down
2 changes: 1 addition & 1 deletion core-rust/state-manager/src/store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ pub use rocks_db::{ActualStateManagerDatabase, StateManagerDatabase};
pub use rocks_db::{ReadableRocks, WriteableRocks};
use sbor::{Categorize, Decode, Encode};
use std::sync::Arc;
pub use traits::DatabaseFlags;
pub use traits::DatabaseConfig;

#[derive(Debug, Categorize, Encode, Decode, Clone)]
pub struct DatabaseBackendConfig {
Expand Down
46 changes: 26 additions & 20 deletions core-rust/state-manager/src/store/rocks_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -682,11 +682,11 @@ impl<'db> Snapshottable<'db> for StateManagerDatabase<DirectRocks> {

/// A RocksDB-backed persistence layer for state manager.
pub struct StateManagerDatabase<R> {
/// Database feature flags.
/// Database config.
///
/// These were passed during construction, validated and persisted. They are made available by
/// this field as a cache.
config: DatabaseFlags,
/// The config is passed during construction, validated, persisted, and effectively immutable
/// during the state manager's lifetime. This field only acts as a cache.
config: DatabaseConfig,

/// Underlying RocksDB instance.
rocks: R,
Expand All @@ -695,7 +695,7 @@ pub struct StateManagerDatabase<R> {
impl ActualStateManagerDatabase {
pub fn new(
root: PathBuf,
config: DatabaseFlags,
config: DatabaseConfig,
network: &NetworkDefinition,
) -> Result<Self, DatabaseConfigValidationError> {
let mut db_opts = Options::default();
Expand All @@ -710,18 +710,13 @@ impl ActualStateManagerDatabase {
let db = DB::open_cf_descriptors(&db_opts, root.as_path(), column_families).unwrap();

let state_manager_database = StateManagerDatabase {
config: config.clone(),
config,
rocks: DirectRocks { db },
};

let current_database_config = state_manager_database.read_flags_state();
config.validate(&current_database_config)?;
state_manager_database.write_flags(&config);

if state_manager_database.config.enable_account_change_index {
state_manager_database.catchup_account_change_index();
}
state_manager_database.validate_and_persist_new_config()?;

state_manager_database.catchup_account_change_index();
state_manager_database.restore_december_2023_lost_substates(network);

Ok(state_manager_database)
Expand Down Expand Up @@ -754,7 +749,7 @@ impl<R: ReadableRocks> StateManagerDatabase<R> {
.unwrap();

StateManagerDatabase {
config: DatabaseFlags {
config: DatabaseConfig {
enable_local_transaction_execution_index: false,
enable_account_change_index: false,
},
Expand Down Expand Up @@ -789,7 +784,7 @@ impl<R: SecondaryRocks> StateManagerDatabase<R> {
.unwrap();

StateManagerDatabase {
config: DatabaseFlags {
config: DatabaseConfig {
enable_local_transaction_execution_index: false,
enable_account_change_index: false,
},
Expand Down Expand Up @@ -817,7 +812,14 @@ impl<R: WriteableRocks> StateManagerDatabase<R> {
}

impl<R: WriteableRocks> StateManagerDatabase<R> {
fn read_flags_state(&self) -> DatabaseFlagsState {
fn validate_and_persist_new_config(&self) -> Result<(), DatabaseConfigValidationError> {
let stored_config_state = self.read_config_state();
self.config.validate(&stored_config_state)?;
self.write_config();
Ok(())
}

fn read_config_state(&self) -> DatabaseConfigState {
let db_context = self.open_read_context();
let extension_data_cf = db_context.cf(ExtensionsDataCf);
let account_change_index_enabled = extension_data_cf
Expand All @@ -826,22 +828,22 @@ impl<R: WriteableRocks> StateManagerDatabase<R> {
let local_transaction_execution_index_enabled = extension_data_cf
.get(&ExtensionsDataKey::LocalTransactionExecutionIndexEnabled)
.map(|bytes| scrypto_decode::<bool>(&bytes).unwrap());
DatabaseFlagsState {
DatabaseConfigState {
account_change_index_enabled,
local_transaction_execution_index_enabled,
}
}

fn write_flags(&self, database_config: &DatabaseFlags) {
fn write_config(&self) {
let db_context = self.open_rw_context();
let extension_data_cf = db_context.cf(ExtensionsDataCf);
extension_data_cf.put(
&ExtensionsDataKey::AccountChangeIndexEnabled,
&scrypto_encode(&database_config.enable_account_change_index).unwrap(),
&scrypto_encode(&self.config.enable_account_change_index).unwrap(),
);
extension_data_cf.put(
&ExtensionsDataKey::LocalTransactionExecutionIndexEnabled,
&scrypto_encode(&database_config.enable_local_transaction_execution_index).unwrap(),
&scrypto_encode(&self.config.enable_local_transaction_execution_index).unwrap(),
);
}
}
Expand Down Expand Up @@ -1788,6 +1790,10 @@ impl<R: WriteableRocks> AccountChangeIndexExtension for StateManagerDatabase<R>
}

fn catchup_account_change_index(&self) {
if !self.config.enable_account_change_index {
return; // Nothing to do
}

const MAX_TRANSACTION_BATCH: u64 = 16 * 1024;

info!("Account Change Index is enabled!");
Expand Down
15 changes: 7 additions & 8 deletions core-rust/state-manager/src/store/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,16 @@ pub enum DatabaseConfigValidationError {
LocalTransactionExecutionIndexChanged,
}

/// Database flags required for initialization built from
/// config file and environment variables.
/// Database flags required for initialization built from config file and environment variables.
#[derive(Debug, Categorize, Encode, Decode, Clone)]
pub struct DatabaseFlags {
pub struct DatabaseConfig {
pub enable_local_transaction_execution_index: bool,
pub enable_account_change_index: bool,
}

impl Default for DatabaseFlags {
impl Default for DatabaseConfig {
fn default() -> Self {
DatabaseFlags {
DatabaseConfig {
enable_local_transaction_execution_index: true,
enable_account_change_index: true,
}
Expand All @@ -104,15 +103,15 @@ impl Default for DatabaseFlags {
/// just being initialized (when all of the fields are None) but also
/// when new configurations are added - this is a cheap work around to
/// limit future needed ledger wipes until we have a better solution.
pub struct DatabaseFlagsState {
pub struct DatabaseConfigState {
pub local_transaction_execution_index_enabled: Option<bool>,
pub account_change_index_enabled: Option<bool>,
}

impl DatabaseFlags {
impl DatabaseConfig {
pub fn validate(
&self,
current_database_config: &DatabaseFlagsState,
current_database_config: &DatabaseConfigState,
) -> Result<(), DatabaseConfigValidationError> {
if !self.enable_local_transaction_execution_index && self.enable_account_change_index {
return Err(DatabaseConfigValidationError::AccountChangeIndexRequiresLocalTransactionExecutionIndex);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
import static org.assertj.core.api.Assertions.*;

import com.google.inject.Module;
import com.radixdlt.environment.DatabaseFlags;
import com.radixdlt.environment.DatabaseConfig;
import com.radixdlt.genesis.GenesisBuilder;
import com.radixdlt.genesis.GenesisConsensusManagerConfig;
import com.radixdlt.harness.deterministic.DeterministicTest;
Expand Down Expand Up @@ -220,7 +220,7 @@ private DeterministicTest createTest(
GenesisConsensusManagerConfig.Builder.testWithRoundsPerEpoch(
this.roundsPerEpoch)
.totalEmissionXrdPerEpoch(Decimal.ofNonNegative(0))),
new DatabaseFlags(true, false),
new DatabaseConfig(true, false),
StateComputerConfig.REV2ProposerConfig.transactionGenerator(
new REV2TransactionGenerator(), 1),
false,
Expand Down
6 changes: 3 additions & 3 deletions core/src/main/java/com/radixdlt/RadixNodeModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,8 @@ protected void configure() {
var enableLocalTransactionExecutionIndex =
properties.get("db.local_transaction_execution_index.enable", true);
var enableAccountChangeIndex = properties.get("db.account_change_index.enable", true);
var databaseFlags =
new DatabaseFlags(enableLocalTransactionExecutionIndex, enableAccountChangeIndex);
var databaseConfig =
new DatabaseConfig(enableLocalTransactionExecutionIndex, enableAccountChangeIndex);

install(new REv2LedgerInitializerModule(genesisProvider));

Expand Down Expand Up @@ -371,7 +371,7 @@ protected void configure() {
REv2StateManagerModule.create(
ProposalLimitsConfig.from(vertexLimitsConfig),
vertexLimitsConfig,
databaseFlags,
databaseConfig,
Option.some(mempoolConfig),
stateHashTreeGcConfig,
ledgerProofsGcConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public final class REv2StateManagerModule extends AbstractModule {

private final ProposalLimitsConfig proposalLimitsConfig;
private final Option<VertexLimitsConfig> vertexLimitsConfigOpt;
private final DatabaseFlags databaseFlags;
private final DatabaseConfig databaseConfig;
private final Option<RustMempoolConfig> mempoolConfig;
private final boolean debugLogging;
private final StateHashTreeGcConfig stateHashTreeGcConfig;
Expand All @@ -118,7 +118,7 @@ public final class REv2StateManagerModule extends AbstractModule {
private REv2StateManagerModule(
ProposalLimitsConfig proposalLimitsConfig,
Option<VertexLimitsConfig> vertexLimitsConfigOpt,
DatabaseFlags databaseFlags,
DatabaseConfig databaseConfig,
Option<RustMempoolConfig> mempoolConfig,
boolean debugLogging,
StateHashTreeGcConfig stateHashTreeGcConfig,
Expand All @@ -128,7 +128,7 @@ private REv2StateManagerModule(
boolean noFees) {
this.proposalLimitsConfig = proposalLimitsConfig;
this.vertexLimitsConfigOpt = vertexLimitsConfigOpt;
this.databaseFlags = databaseFlags;
this.databaseConfig = databaseConfig;
this.mempoolConfig = mempoolConfig;
this.debugLogging = debugLogging;
this.stateHashTreeGcConfig = stateHashTreeGcConfig;
Expand All @@ -141,7 +141,7 @@ private REv2StateManagerModule(
public static REv2StateManagerModule create(
ProposalLimitsConfig proposalLimitsConfig,
VertexLimitsConfig vertexLimitsConfig,
DatabaseFlags databaseFlags,
DatabaseConfig databaseConfig,
Option<RustMempoolConfig> mempoolConfig,
StateHashTreeGcConfig stateHashTreeGcConfig,
LedgerProofsGcConfig ledgerProofsGcConfig,
Expand All @@ -150,7 +150,7 @@ public static REv2StateManagerModule create(
return new REv2StateManagerModule(
proposalLimitsConfig,
Option.some(vertexLimitsConfig),
databaseFlags,
databaseConfig,
mempoolConfig,
false,
stateHashTreeGcConfig,
Expand All @@ -162,7 +162,7 @@ public static REv2StateManagerModule create(

public static REv2StateManagerModule createForTesting(
ProposalLimitsConfig proposalLimitsConfig,
DatabaseFlags databaseFlags,
DatabaseConfig databaseConfig,
Option<RustMempoolConfig> mempoolConfig,
boolean debugLogging,
StateHashTreeGcConfig stateHashTreeGcConfig,
Expand All @@ -173,7 +173,7 @@ public static REv2StateManagerModule createForTesting(
return new REv2StateManagerModule(
proposalLimitsConfig,
Option.none(),
databaseFlags,
databaseConfig,
mempoolConfig,
debugLogging,
stateHashTreeGcConfig,
Expand All @@ -188,7 +188,7 @@ public void configure() {
bind(StateComputerLedger.StateComputer.class).to(REv2StateComputer.class);
bind(REv2TransactionsAndProofReader.class).in(Scopes.SINGLETON);
bind(TransactionsAndProofReader.class).to(REv2TransactionsAndProofReader.class);
bind(DatabaseFlags.class).toInstance(databaseFlags);
bind(DatabaseConfig.class).toInstance(databaseConfig);
bind(LedgerSyncLimitsConfig.class).toInstance(ledgerSyncLimitsConfig);
bind(ProtocolConfig.class).toInstance(protocolConfig);
install(proposalLimitsConfig.asModule());
Expand All @@ -210,7 +210,7 @@ private NodeRustEnvironment stateManager(
FatalPanicHandler fatalPanicHandler,
Network network,
DatabaseBackendConfig databaseBackendConfig,
DatabaseFlags databaseFlags) {
DatabaseConfig databaseConfig) {
return new NodeRustEnvironment(
mempoolRelayDispatcher,
fatalPanicHandler,
Expand All @@ -219,7 +219,7 @@ private NodeRustEnvironment stateManager(
mempoolConfig,
vertexLimitsConfigOpt,
databaseBackendConfig,
databaseFlags,
databaseConfig,
getLoggingConfig(),
stateHashTreeGcConfig,
ledgerProofsGcConfig,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ public void configure() {
install(
REv2StateManagerModule.createForTesting(
ProposalLimitsConfig.testDefaults(),
rev2Config.databaseFlags(),
rev2Config.databaseConfig(),
Option.none(),
rev2Config.debugLogging(),
rev2Config.stateHashTreeGcConfig(),
Expand All @@ -435,7 +435,7 @@ public void configure() {
install(
REv2StateManagerModule.createForTesting(
mempool.proposalLimitsConfig(),
rev2Config.databaseFlags(),
rev2Config.databaseConfig(),
Option.some(mempool.mempoolConfig()),
rev2Config.debugLogging(),
rev2Config.stateHashTreeGcConfig(),
Expand Down
Loading

0 comments on commit 972b60e

Please sign in to comment.