Mercurial > lbo > hg > leveldb-rs
changeset 593:53a459b14a17
Merge pull request #21 from cfzjywxk/fix_drop
skipmap: implement drop for the skip map to avoid stackoverflow caused by drop
author | Lewin Bormann <lbo@spheniscida.de> |
---|---|
date | Sun, 15 Jan 2023 09:25:34 +0100 |
parents | 3ac181023b24 (current diff) ab55279aa1a6 (diff) |
children | d819c7d21878 |
files | |
diffstat | 5 files changed, 176 insertions(+), 37 deletions(-) [+] |
line wrap: on
line diff
--- a/src/db_impl.rs Fri Jan 06 23:31:44 2023 +0100 +++ b/src/db_impl.rs Sun Jan 15 09:25:34 2023 +0100 @@ -188,14 +188,15 @@ for file in &filenames { match parse_file_name(&file) { Ok((num, typ)) => { - expected.remove(&num); - if typ == FileType::Log - && (num >= self.vset.borrow().log_num || num == self.vset.borrow().prev_log_num) - { - log_files.push(num); + expected.remove(&num); + if typ == FileType::Log + && (num >= self.vset.borrow().log_num + || num == self.vset.borrow().prev_log_num) + { + log_files.push(num); + } } - }, - Err(e) => return Err(e.annotate(format!("While parsing {:?}", file))) + Err(e) => return Err(e.annotate(format!("While parsing {:?}", file))), } } if !expected.is_empty() { @@ -1168,7 +1169,9 @@ log!(l, "and another {}", 1); let mut s = String::new(); - let mut r = e.open_sequential_file(&Path::new("abc").join("LOG")).unwrap(); + let mut r = e + .open_sequential_file(&Path::new("abc").join("LOG")) + .unwrap(); r.read_to_string(&mut s).unwrap(); assert_eq!("something else\nand another 1\n", &s); } @@ -1209,9 +1212,15 @@ ); assert!(env.exists(&Path::new("otherdb").join("CURRENT")).unwrap()); // Database is initialized and initial manifest reused. - assert!(!env.exists(&Path::new("otherdb").join("MANIFEST-000001")).unwrap()); - assert!(env.exists(&Path::new("otherdb").join("MANIFEST-000002")).unwrap()); - assert!(env.exists(&Path::new("otherdb").join("000003.log")).unwrap()); + assert!(!env + .exists(&Path::new("otherdb").join("MANIFEST-000001")) + .unwrap()); + assert!(env + .exists(&Path::new("otherdb").join("MANIFEST-000002")) + .unwrap()); + assert!(env + .exists(&Path::new("otherdb").join("000003.log")) + .unwrap()); } { @@ -1225,7 +1234,9 @@ ); assert!(env.exists(&Path::new("db").join("CURRENT")).unwrap()); // Database is initialized and initial manifest reused. - assert!(env.exists(&Path::new("db").join("MANIFEST-000001")).unwrap()); + assert!(env + .exists(&Path::new("db").join("MANIFEST-000001")) + .unwrap()); assert!(env.exists(&Path::new("db").join("LOCK")).unwrap()); assert!(env.exists(&Path::new("db").join("000003.log")).unwrap()); @@ -1248,9 +1259,13 @@ env.children(&Path::new("db").join("")).unwrap() ); // Obsolete manifest is deleted. - assert!(!env.exists(&Path::new("db").join("MANIFEST-000001")).unwrap()); + assert!(!env + .exists(&Path::new("db").join("MANIFEST-000001")) + .unwrap()); // New manifest is created. - assert!(env.exists(&Path::new("db").join("MANIFEST-000002")).unwrap()); + assert!(env + .exists(&Path::new("db").join("MANIFEST-000002")) + .unwrap()); // Obsolete log file is deleted. assert!(!env.exists(&Path::new("db").join("000003.log")).unwrap()); // New L0 table has been added. @@ -1287,9 +1302,15 @@ "children after: {:?}", env.children(Path::new("db")).unwrap() ); - assert!(!env.exists(&Path::new("db").join("MANIFEST-000001")).unwrap()); - assert!(env.exists(&Path::new("db").join("MANIFEST-000002")).unwrap()); - assert!(!env.exists(&Path::new("db").join("MANIFEST-000005")).unwrap()); + assert!(!env + .exists(&Path::new("db").join("MANIFEST-000001")) + .unwrap()); + assert!(env + .exists(&Path::new("db").join("MANIFEST-000002")) + .unwrap()); + assert!(!env + .exists(&Path::new("db").join("MANIFEST-000005")) + .unwrap()); assert!(env.exists(&Path::new("db").join("000004.log")).unwrap()); // 000004 should be reused, no new log file should be created. assert!(!env.exists(&Path::new("db").join("000006.log")).unwrap()); @@ -1321,10 +1342,30 @@ env.children(&Path::new("db").join("")).unwrap() ); - assert_eq!(250, opt.env.size_of(&Path::new("db").join("000007.ldb")).unwrap()); - assert_eq!(200, opt.env.size_of(&Path::new("db").join("000008.ldb")).unwrap()); - assert_eq!(200, opt.env.size_of(&Path::new("db").join("000009.ldb")).unwrap()); - assert_eq!(435, opt.env.size_of(&Path::new("db").join("000015.ldb")).unwrap()); + assert_eq!( + 250, + opt.env + .size_of(&Path::new("db").join("000007.ldb")) + .unwrap() + ); + assert_eq!( + 200, + opt.env + .size_of(&Path::new("db").join("000008.ldb")) + .unwrap() + ); + assert_eq!( + 200, + opt.env + .size_of(&Path::new("db").join("000009.ldb")) + .unwrap() + ); + assert_eq!( + 435, + opt.env + .size_of(&Path::new("db").join("000015.ldb")) + .unwrap() + ); assert!(!opt.env.exists(&Path::new("db").join("000001.ldb")).unwrap()); assert!(!opt.env.exists(&Path::new("db").join("000002.ldb")).unwrap()); @@ -1357,11 +1398,36 @@ env.children(Path::new("db")).unwrap() ); - assert_eq!(250, opt.env.size_of(&Path::new("db").join("000007.ldb")).unwrap()); - assert_eq!(200, opt.env.size_of(&Path::new("db").join("000008.ldb")).unwrap()); - assert_eq!(200, opt.env.size_of(&Path::new("db").join("000009.ldb")).unwrap()); - assert_eq!(182, opt.env.size_of(&Path::new("db").join("000014.ldb")).unwrap()); - assert_eq!(435, opt.env.size_of(&Path::new("db").join("000017.ldb")).unwrap()); + assert_eq!( + 250, + opt.env + .size_of(&Path::new("db").join("000007.ldb")) + .unwrap() + ); + assert_eq!( + 200, + opt.env + .size_of(&Path::new("db").join("000008.ldb")) + .unwrap() + ); + assert_eq!( + 200, + opt.env + .size_of(&Path::new("db").join("000009.ldb")) + .unwrap() + ); + assert_eq!( + 182, + opt.env + .size_of(&Path::new("db").join("000014.ldb")) + .unwrap() + ); + assert_eq!( + 435, + opt.env + .size_of(&Path::new("db").join("000017.ldb")) + .unwrap() + ); assert!(!opt.env.exists(&Path::new("db").join("000001.ldb")).unwrap()); assert!(!opt.env.exists(&Path::new("db").join("000002.ldb")).unwrap()); @@ -1559,7 +1625,11 @@ "children after: {:?}", db.opt.env.children(Path::new("db")).unwrap() ); - assert!(db.opt.env.exists(&Path::new("db").join("000004.ldb")).unwrap()); + assert!(db + .opt + .env + .exists(&Path::new("db").join("000004.ldb")) + .unwrap()); { let v = db.current(); @@ -1589,9 +1659,23 @@ // Trigger memtable compaction. db.make_room_for_write(true).unwrap(); assert_eq!(0, db.mem.len()); - assert!(db.opt.env.exists(&Path::new("db").join("000002.log")).unwrap()); - assert!(db.opt.env.exists(&Path::new("db").join("000003.ldb")).unwrap()); - assert_eq!(351, db.opt.env.size_of(&Path::new("db").join("000003.ldb")).unwrap()); + assert!(db + .opt + .env + .exists(&Path::new("db").join("000002.log")) + .unwrap()); + assert!(db + .opt + .env + .exists(&Path::new("db").join("000003.ldb")) + .unwrap()); + assert_eq!( + 351, + db.opt + .env + .size_of(&Path::new("db").join("000003.ldb")) + .unwrap() + ); assert_eq!( 7, LdbIteratorIter::wrap(&mut db.cache.borrow_mut().get_table(3).unwrap().iter()).count() @@ -1607,9 +1691,23 @@ db.maybe_do_compaction().unwrap(); - assert!(!db.opt.env.exists(&Path::new("db").join("000003.ldb")).unwrap()); - assert!(db.opt.env.exists(&Path::new("db").join("000013.ldb")).unwrap()); - assert_eq!(345, db.opt.env.size_of(&Path::new("db").join("000013.ldb")).unwrap()); + assert!(!db + .opt + .env + .exists(&Path::new("db").join("000003.ldb")) + .unwrap()); + assert!(db + .opt + .env + .exists(&Path::new("db").join("000013.ldb")) + .unwrap()); + assert_eq!( + 345, + db.opt + .env + .size_of(&Path::new("db").join("000013.ldb")) + .unwrap() + ); // New current version. let v = db.current(); @@ -1633,7 +1731,12 @@ assert!(opt.env.exists(&Path::new("db").join("000006.ldb")).unwrap()); assert!(!opt.env.exists(&Path::new("db").join("000010.ldb")).unwrap()); - assert_eq!(218, opt.env.size_of(&Path::new("db").join("000006.ldb")).unwrap()); + assert_eq!( + 218, + opt.env + .size_of(&Path::new("db").join("000006.ldb")) + .unwrap() + ); let v = db.current(); assert_eq!(1, v.borrow().files[2].len()); @@ -1705,4 +1808,23 @@ assert_eq!(None, db.get_at(&ss, b"xx2").unwrap()); } } + + #[test] + fn test_drop_memtable() { + let mut db = DB::open("db", options::for_test()).unwrap(); + let mut cnt = 0; + let mut buf: Vec<u8> = Vec::with_capacity(8); + let entries_per_batch = 1; + let max_num = 100000; + while cnt < max_num { + let mut write_batch = WriteBatch::new(); + for i in 0..entries_per_batch { + buf.clear(); + buf.extend_from_slice(format!("{}-{}", cnt, i).as_bytes()); + write_batch.put(buf.as_slice(), buf.as_slice()); + } + db.write(write_batch, false).unwrap(); + cnt += entries_per_batch; + } + } }
--- a/src/error.rs Fri Jan 06 23:31:44 2023 +0100 +++ b/src/error.rs Sun Jan 15 09:25:34 2023 +0100 @@ -115,7 +115,7 @@ #[cfg(test)] mod tests { - use super::{StatusCode, Status}; + use super::{Status, StatusCode}; #[test] fn test_status_to_string() { let s = Status::new(StatusCode::InvalidData, "Invalid data!");
--- a/src/mem_env.rs Fri Jan 06 23:31:44 2023 +0100 +++ b/src/mem_env.rs Sun Jan 15 09:25:34 2023 +0100 @@ -635,7 +635,11 @@ #[test] fn test_memenv_all() { let me = MemEnv::new(); - let (p1, p2, p3) = (Path::new("\\a\\b"), Path::new("\\a\\c"), Path::new("\\a\\d")); + let (p1, p2, p3) = ( + Path::new("\\a\\b"), + Path::new("\\a\\c"), + Path::new("\\a\\d"), + ); let nonexist = Path::new("\\x\\y"); me.open_writable_file(p2).unwrap(); me.open_appendable_file(p3).unwrap();
--- a/src/skipmap.rs Fri Jan 06 23:31:44 2023 +0100 +++ b/src/skipmap.rs Sun Jan 15 09:25:34 2023 +0100 @@ -33,6 +33,16 @@ cmp: Rc<Box<dyn Cmp>>, } +impl Drop for InnerSkipMap { + // Avoid possible stack overflow caused by dropping many nodes. + fn drop(&mut self) { + let mut next_node = self.head.next.take(); + while let Some(mut boxed_node) = next_node { + next_node = boxed_node.next.take(); + } + } +} + pub struct SkipMap { map: Rc<RefCell<InnerSkipMap>>, }
--- a/src/version_set.rs Fri Jan 06 23:31:44 2023 +0100 +++ b/src/version_set.rs Sun Jan 15 09:25:34 2023 +0100 @@ -1125,7 +1125,10 @@ vs.log_and_apply(ve).unwrap(); assert!(opt.env.exists(&Path::new("db").join("CURRENT")).unwrap()); - assert!(opt.env.exists(&Path::new("db").join("MANIFEST-000019")).unwrap()); + assert!(opt + .env + .exists(&Path::new("db").join("MANIFEST-000019")) + .unwrap()); // next_file_num and last_seq are untouched by log_and_apply assert_eq!(21, vs.new_file_number()); assert_eq!(22, vs.next_file_num);