From 94ab5e364eac2701f2672ae732f4fc8749b3bbf8 Mon Sep 17 00:00:00 2001
From: Georgy Moshkin <gmoshkin@picodata.io>
Date: Mon, 27 Nov 2023 13:00:26 +0300
Subject: [PATCH] refactor: make it explicit that Clusterwide storage is only
initialized once
---
src/cas.rs | 4 ++--
src/instance.rs | 16 ++++++++--------
src/storage.rs | 30 ++++++++++++++++++++++++------
src/sync.rs | 2 +-
src/traft/network.rs | 8 ++++----
5 files changed, 39 insertions(+), 21 deletions(-)
diff --git a/src/cas.rs b/src/cas.rs
index 997db280bc..0ae3c21bb8 100644
--- a/src/cas.rs
+++ b/src/cas.rs
@@ -757,7 +757,7 @@ mod tests {
#[::tarantool::test]
fn ddl() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
let t = |op: &Op, range: Range| -> std::result::Result<(), Error> {
let predicate = Predicate {
@@ -879,7 +879,7 @@ mod tests {
fn dml() {
let key = (12,).to_tuple_buffer().unwrap();
let tuple = (12, "twelve").to_tuple_buffer().unwrap();
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
let test = |op: &Dml, range: Range| {
let predicate = Predicate {
diff --git a/src/instance.rs b/src/instance.rs
index 4ddda4437b..0c855e3d46 100644
--- a/src/instance.rs
+++ b/src/instance.rs
@@ -205,7 +205,7 @@ mod tests {
#[::tarantool::test]
fn test_simple() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
setup_storage(&storage, vec![], 1);
let instance = build_instance(None, None, &FailureDomain::default(), &storage, DEFAULT_TIER).unwrap();
@@ -239,7 +239,7 @@ mod tests {
#[::tarantool::test]
fn test_override() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
setup_storage(&storage, vec![
Instance::new(Some(1), Some("i1"), Some("r1"), Grade::new(Online, 1), Grade::new(Online, 1), FailureDomain::default(), DEFAULT_TIER),
Instance::new(Some(2), Some("i2"), Some("r2-original"), Grade::new(Offline, 0), Grade::new(Offline, 0), FailureDomain::default(), DEFAULT_TIER),
@@ -291,7 +291,7 @@ mod tests {
#[::tarantool::test]
fn test_instance_id_collision() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
setup_storage(&storage, vec![
Instance::new(Some(1), Some("i1"), Some("r1"), Grade::new(Online, 1), Grade::new(Online, 1), FailureDomain::default(), DEFAULT_TIER),
Instance::new(Some(2), Some("i3"), Some("r3"), Grade::new(Online, 1), Grade::new(Online, 1), FailureDomain::default(), DEFAULT_TIER),
@@ -306,7 +306,7 @@ mod tests {
#[::tarantool::test]
fn test_replication_factor() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
setup_storage(&storage, vec![
Instance::new(Some(9), Some("i9"), Some("r9"), Grade::new(Online, 1), Grade::new(Online, 1), FailureDomain::default(), DEFAULT_TIER),
Instance::new(Some(10), Some("i10"), Some("r9"), Grade::new(Online, 1), Grade::new(Online, 1), FailureDomain::default(), DEFAULT_TIER),
@@ -348,7 +348,7 @@ mod tests {
#[::tarantool::test]
fn test_update_grade() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
let mut instance = Instance::new(Some(1), Some("i1"), Some("r1"), Grade::new(Online, 1), Grade::new(Online, 1), FailureDomain::default(), DEFAULT_TIER);
setup_storage(&storage, vec![instance.clone()], 1);
@@ -441,7 +441,7 @@ mod tests {
#[::tarantool::test]
fn failure_domain() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
setup_storage(&storage, vec![], 3);
let instance =
@@ -507,7 +507,7 @@ mod tests {
#[::tarantool::test]
fn reconfigure_failure_domain() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
setup_storage(&storage, vec![], 3);
// first instance
@@ -586,7 +586,7 @@ mod tests {
let second_tier = "compute";
let third_tier = "trash";
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
setup_storage(&storage, vec![], 1);
add_tier(&storage, first_tier, 3).unwrap();
diff --git a/src/storage.rs b/src/storage.rs
index ef26f7c4b7..f05459bbd2 100644
--- a/src/storage.rs
+++ b/src/storage.rs
@@ -123,8 +123,19 @@ macro_rules! define_clusterwide_tables {
}
impl $Clusterwide {
+ /// Initialize the clusterwide storage.
+ ///
+ /// This function is private because it should only be called once
+ /// per picodata instance on boot.
#[inline(always)]
- pub fn new() -> tarantool::Result<Self> {
+ fn initialize() -> tarantool::Result<Self> {
+ // SAFETY: safe as long as only called from tx thread.
+ static mut WAS_CALLED: bool = false;
+ unsafe {
+ assert!(!WAS_CALLED, "Clusterwide storage must only be initialized once");
+ WAS_CALLED = true;
+ }
+
Ok(Self {
$( $Clusterwide_field: $space_struct::new()?, )+
$( $Clusterwide_extra_field: Default::default(), )*
@@ -360,7 +371,7 @@ impl Clusterwide {
if !init {
return Err(Error::Uninitialized);
}
- STORAGE = Some(Self::new()?);
+ STORAGE = Some(Self::initialize()?);
}
Ok(STORAGE.as_ref().unwrap())
}
@@ -376,6 +387,13 @@ impl Clusterwide {
Self::try_get(false).expect("shouldn't be calling this until it's initialized")
}
+ /// Get an instance of clusterwide storage for use in unit tests.
+ ///
+ /// Should only be used in tests.
+ pub(crate) fn for_tests() -> Self {
+ Self::initialize().unwrap()
+ }
+
fn open_read_view(&self, entry_id: RaftEntryId) -> Result<SnapshotReadView> {
let mut space_indexes = Vec::with_capacity(32);
// ClusterwideTable::all_tables is guaranteed to iterate in order
@@ -3152,7 +3170,7 @@ mod tests {
use crate::instance::InstanceId;
use crate::failure_domain::FailureDomain;
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
let storage_peer_addresses = PeerAddresses::new().unwrap();
let space_peer_addresses = storage_peer_addresses.space.clone();
@@ -3261,7 +3279,7 @@ mod tests {
#[::tarantool::test]
fn clusterwide_space_index() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
storage
.space_by_name(ClusterwideTable::Address)
@@ -3292,7 +3310,7 @@ mod tests {
#[::tarantool::test]
fn snapshot_data() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
storage.for_each_space(|s| s.truncate()).unwrap();
let i = Instance {
@@ -3364,7 +3382,7 @@ mod tests {
#[::tarantool::test]
fn apply_snapshot_data() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
let mut data = SnapshotData {
schema_version: 0,
diff --git a/src/sync.rs b/src/sync.rs
index 9c2cf7753e..5e19fdd4c5 100644
--- a/src/sync.rs
+++ b/src/sync.rs
@@ -186,7 +186,7 @@ mod tests {
#[::tarantool::test]
async fn vclock_proc() {
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
// Connect to the current Tarantool instance
let pool = ConnectionPool::new(storage.clone(), Default::default());
let l = ::tarantool::lua_state();
diff --git a/src/traft/network.rs b/src/traft/network.rs
index c69fb0d108..c8cb14687e 100644
--- a/src/traft/network.rs
+++ b/src/traft/network.rs
@@ -628,7 +628,7 @@ mod tests {
)
.unwrap();
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
// Connect to the current Tarantool instance
let pool = ConnectionPool::new(storage.clone(), Default::default());
let listen: String = l.eval("return box.info.listen").unwrap();
@@ -673,7 +673,7 @@ mod tests {
}),
);
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
// Connect to the current Tarantool instance
let opts = WorkerOptions {
raft_msg_handler: "test_interact",
@@ -759,7 +759,7 @@ mod tests {
}),
);
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
// Connect to the current Tarantool instance
let opts = WorkerOptions {
raft_msg_handler: "test_interact",
@@ -837,7 +837,7 @@ mod tests {
}),
);
- let storage = Clusterwide::new().unwrap();
+ let storage = Clusterwide::for_tests();
// Connect to the current Tarantool instance
let opts = WorkerOptions {
raft_msg_handler: "test_interact",
--
GitLab