Skip to content

Commit 9d1f833

Browse files
AlexStocksOmX
andcommitted
test(storage): keep cache regressions mergeable with main
Move the block-cache regression module away from the lifecycle-test attribute changed by current main, and carry the same Clippy allowance on the existing lifecycle module. This removes the textual conflict without changing cache behavior. Constraint: Limit this follow-up to the current-main conflict in src/storage/src/redis.rs. Confidence: High; the exact synthetic merge with main passed cache tests, snapshot roundtrip tests, formatting, and workspace Clippy. Scope-risk: Test-module placement and an attribute already present on main only. Tested: exact main merge-tree; merged block-cache tests 3/3; merged snapshot roundtrip 12/12; merged make lint; cargo fmt --check; staged diff check. Co-authored-by: OmX <omx@oh-my-codex.dev> Signed-off-by: Xin.Zh <alexstocks@foxmail.com>
1 parent a131a04 commit 9d1f833

1 file changed

Lines changed: 104 additions & 103 deletions

File tree

src/storage/src/redis.rs

Lines changed: 104 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,109 +1029,7 @@ macro_rules! get_db_and_cfs {
10291029
}
10301030

10311031
#[cfg(test)]
1032-
mod block_cache_tests {
1033-
use std::sync::Arc;
1034-
1035-
use kstd::lock_mgr::LockMgr;
1036-
1037-
use super::Redis;
1038-
use crate::{BgTaskHandler, StorageOptions, safe_cleanup_test_db, unique_test_db_path};
1039-
1040-
fn new_redis(options: Arc<StorageOptions>, index: i32) -> Redis {
1041-
let (bg_task_handler, _) = BgTaskHandler::new();
1042-
Redis::new(
1043-
options,
1044-
index,
1045-
Arc::new(bg_task_handler),
1046-
Arc::new(LockMgr::new(64)),
1047-
)
1048-
}
1049-
1050-
#[test]
1051-
fn disabled_sharing_creates_one_distinct_cache_per_redis_instance() {
1052-
let options = Arc::new(StorageOptions {
1053-
block_cache_size: 4 * 1024 * 1024,
1054-
share_block_cache: false,
1055-
block_cache: None,
1056-
..StorageOptions::default()
1057-
});
1058-
1059-
let first = new_redis(Arc::clone(&options), 0);
1060-
let second = new_redis(options, 1);
1061-
let first_cache = first
1062-
.block_cache
1063-
.as_ref()
1064-
.expect("the first Redis instance should own a block cache");
1065-
let second_cache = second
1066-
.block_cache
1067-
.as_ref()
1068-
.expect("the second Redis instance should own a block cache");
1069-
1070-
assert!(
1071-
!Arc::ptr_eq(first_cache, second_cache),
1072-
"different Redis instances must not share their per-instance caches"
1073-
);
1074-
}
1075-
1076-
#[test]
1077-
fn default_options_share_one_cache_across_redis_instances() {
1078-
let options = Arc::new(StorageOptions::default());
1079-
let shared_cache = options
1080-
.block_cache
1081-
.as_ref()
1082-
.expect("default options should configure a shared block cache")
1083-
.clone();
1084-
1085-
let first = new_redis(Arc::clone(&options), 0);
1086-
let second = new_redis(options, 1);
1087-
let first_cache = first
1088-
.block_cache
1089-
.as_ref()
1090-
.expect("the first Redis instance should use the shared block cache");
1091-
let second_cache = second
1092-
.block_cache
1093-
.as_ref()
1094-
.expect("the second Redis instance should use the shared block cache");
1095-
1096-
assert!(Arc::ptr_eq(first_cache, &shared_cache));
1097-
assert!(Arc::ptr_eq(second_cache, &shared_cache));
1098-
}
1099-
1100-
#[test]
1101-
fn zero_block_cache_size_disables_rocksdb_internal_cache_for_every_cf() {
1102-
let path = unique_test_db_path();
1103-
safe_cleanup_test_db(&path);
1104-
let options = Arc::new(StorageOptions {
1105-
block_cache_size: 0,
1106-
share_block_cache: true,
1107-
block_cache: None,
1108-
..StorageOptions::default()
1109-
});
1110-
let mut redis = new_redis(options, 0);
1111-
redis
1112-
.open(path.to_str().expect("test DB path should be valid UTF-8"))
1113-
.expect("Redis should open with block cache disabled");
1114-
let db = redis.db.as_ref().expect("Redis should own RocksDB");
1115-
1116-
for cf_name in &redis.handles {
1117-
let cf = db
1118-
.cf_handle(cf_name)
1119-
.expect("every configured CF should be open");
1120-
let capacity = db
1121-
.property_int_value_cf(&cf, "rocksdb.block-cache-capacity")
1122-
.expect("block cache capacity property should be readable");
1123-
assert_eq!(
1124-
capacity, None,
1125-
"{cf_name} must not receive RocksDB's implicit internal block cache"
1126-
);
1127-
}
1128-
1129-
drop(redis);
1130-
safe_cleanup_test_db(&path);
1131-
}
1132-
}
1133-
1134-
#[cfg(test)]
1032+
#[allow(clippy::unwrap_used)]
11351033
mod lifecycle_tests {
11361034
use std::sync::{Arc, mpsc};
11371035
use std::time::Duration;
@@ -1420,3 +1318,106 @@ mod type_check_state_tests {
14201318
assert_eq!(state, TypeCheckState::Match);
14211319
}
14221320
}
1321+
1322+
#[cfg(test)]
1323+
mod block_cache_tests {
1324+
use std::sync::Arc;
1325+
1326+
use kstd::lock_mgr::LockMgr;
1327+
1328+
use super::Redis;
1329+
use crate::{BgTaskHandler, StorageOptions, safe_cleanup_test_db, unique_test_db_path};
1330+
1331+
fn new_redis(options: Arc<StorageOptions>, index: i32) -> Redis {
1332+
let (bg_task_handler, _) = BgTaskHandler::new();
1333+
Redis::new(
1334+
options,
1335+
index,
1336+
Arc::new(bg_task_handler),
1337+
Arc::new(LockMgr::new(64)),
1338+
)
1339+
}
1340+
1341+
#[test]
1342+
fn disabled_sharing_creates_one_distinct_cache_per_redis_instance() {
1343+
let options = Arc::new(StorageOptions {
1344+
block_cache_size: 4 * 1024 * 1024,
1345+
share_block_cache: false,
1346+
block_cache: None,
1347+
..StorageOptions::default()
1348+
});
1349+
1350+
let first = new_redis(Arc::clone(&options), 0);
1351+
let second = new_redis(options, 1);
1352+
let first_cache = first
1353+
.block_cache
1354+
.as_ref()
1355+
.expect("the first Redis instance should own a block cache");
1356+
let second_cache = second
1357+
.block_cache
1358+
.as_ref()
1359+
.expect("the second Redis instance should own a block cache");
1360+
1361+
assert!(
1362+
!Arc::ptr_eq(first_cache, second_cache),
1363+
"different Redis instances must not share their per-instance caches"
1364+
);
1365+
}
1366+
1367+
#[test]
1368+
fn default_options_share_one_cache_across_redis_instances() {
1369+
let options = Arc::new(StorageOptions::default());
1370+
let shared_cache = options
1371+
.block_cache
1372+
.as_ref()
1373+
.expect("default options should configure a shared block cache")
1374+
.clone();
1375+
1376+
let first = new_redis(Arc::clone(&options), 0);
1377+
let second = new_redis(options, 1);
1378+
let first_cache = first
1379+
.block_cache
1380+
.as_ref()
1381+
.expect("the first Redis instance should use the shared block cache");
1382+
let second_cache = second
1383+
.block_cache
1384+
.as_ref()
1385+
.expect("the second Redis instance should use the shared block cache");
1386+
1387+
assert!(Arc::ptr_eq(first_cache, &shared_cache));
1388+
assert!(Arc::ptr_eq(second_cache, &shared_cache));
1389+
}
1390+
1391+
#[test]
1392+
fn zero_block_cache_size_disables_rocksdb_internal_cache_for_every_cf() {
1393+
let path = unique_test_db_path();
1394+
safe_cleanup_test_db(&path);
1395+
let options = Arc::new(StorageOptions {
1396+
block_cache_size: 0,
1397+
share_block_cache: true,
1398+
block_cache: None,
1399+
..StorageOptions::default()
1400+
});
1401+
let mut redis = new_redis(options, 0);
1402+
redis
1403+
.open(path.to_str().expect("test DB path should be valid UTF-8"))
1404+
.expect("Redis should open with block cache disabled");
1405+
let db = redis.db.as_ref().expect("Redis should own RocksDB");
1406+
1407+
for cf_name in &redis.handles {
1408+
let cf = db
1409+
.cf_handle(cf_name)
1410+
.expect("every configured CF should be open");
1411+
let capacity = db
1412+
.property_int_value_cf(&cf, "rocksdb.block-cache-capacity")
1413+
.expect("block cache capacity property should be readable");
1414+
assert_eq!(
1415+
capacity, None,
1416+
"{cf_name} must not receive RocksDB's implicit internal block cache"
1417+
);
1418+
}
1419+
1420+
drop(redis);
1421+
safe_cleanup_test_db(&path);
1422+
}
1423+
}

0 commit comments

Comments
 (0)