Skip to content

Commit bad893f

Browse files
fix(txn): preserve cache and transaction lifecycle state
1 parent f9e6cfa commit bad893f

8 files changed

Lines changed: 191 additions & 10 deletions

File tree

src/pager/cache.rs

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,10 @@ impl PageCache {
168168
}
169169

170170
fn evict_one(&mut self) -> Option<(FileKey, u64)> {
171-
// Walk the hand at most `capacity * 2` steps to bound worst case
172-
// (every entry either visited or unevictable triggers a wrap).
173-
let max_steps = self.capacity.saturating_mul(2).max(1);
171+
// Two passes are sufficient to clear visited bits and then evict.
172+
// Use the live list length rather than the configured capacity because
173+
// pinned or dirty pages can temporarily grow the cache past capacity.
174+
let max_steps = self.map.len().saturating_mul(2).max(1);
174175
let mut cur = self.hand.or(self.tail);
175176
for _ in 0..max_steps {
176177
let Some(idx) = cur else {
@@ -318,6 +319,33 @@ impl PageCache {
318319
self.dirty.remove(&key);
319320
}
320321
}
322+
323+
/// Drop every unpinned clean entry for `file`, leaving dirty entries
324+
/// available for a later flush and pinned entries available to readers.
325+
pub fn clear_clean_file(&mut self, file: FileKey) {
326+
let keys: Vec<(FileKey, u64)> = self
327+
.map
328+
.keys()
329+
.filter(|(f, _)| *f == file)
330+
.copied()
331+
.collect();
332+
for key in keys {
333+
if self.pins.get(&key).copied().unwrap_or(0) > 0 || self.dirty.contains(&key) {
334+
continue;
335+
}
336+
if let Some(idx) = self.map.remove(&key) {
337+
let (prev, next) = {
338+
let node = self.slab[idx].as_ref().expect("indexed node alive");
339+
(node.prev, node.next)
340+
};
341+
if self.hand == Some(idx) {
342+
self.hand = next;
343+
}
344+
self.unlink_node(idx, prev, next);
345+
self.free_node(idx);
346+
}
347+
}
348+
}
321349
}
322350

323351
#[cfg(test)]
@@ -393,6 +421,20 @@ mod tests {
393421
assert!(c.get((FileKey::Main, 1)).is_some());
394422
}
395423

424+
#[test]
425+
fn clear_clean_file_preserves_dirty_entries() {
426+
let mut c = PageCache::with_capacity(4);
427+
c.insert((FileKey::Main, 1), page(1));
428+
c.insert((FileKey::Main, 2), page(2));
429+
c.mark_dirty((FileKey::Main, 2));
430+
431+
c.clear_clean_file(FileKey::Main);
432+
433+
assert!(c.get((FileKey::Main, 1)).is_none());
434+
assert!(c.get((FileKey::Main, 2)).is_some());
435+
assert!(c.is_dirty((FileKey::Main, 2)));
436+
}
437+
396438
#[test]
397439
fn dirty_iter_is_sorted_ascending() {
398440
let mut c = PageCache::with_capacity(16);
@@ -421,4 +463,27 @@ mod tests {
421463
c.insert((FileKey::Main, 3), page(3));
422464
assert_eq!(c.slab.len(), 2, "slab reuses freed slot");
423465
}
466+
467+
#[test]
468+
fn overgrown_cache_still_finds_clean_victim() {
469+
let mut c = PageCache::with_capacity(2);
470+
471+
for id in 1u8..=4 {
472+
let key = (FileKey::Main, u64::from(id));
473+
c.insert(key, page(id));
474+
c.pin(key);
475+
}
476+
c.insert((FileKey::Main, 5), page(5));
477+
assert_eq!(c.len(), 5, "pinned pages may force temporary growth");
478+
479+
let evicted = c.insert((FileKey::Main, 6), page(6));
480+
assert_eq!(
481+
evicted,
482+
Some((FileKey::Main, 5)),
483+
"eviction must scan the full over-capacity list for a clean victim"
484+
);
485+
assert_eq!(c.len(), 5);
486+
assert!(c.get((FileKey::Main, 5)).is_none());
487+
assert!(c.get((FileKey::Main, 6)).is_some());
488+
}
424489
}

src/pager/core.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,15 @@ impl<V: Vfs> Pager<V> {
653653
}
654654
}
655655

656+
/// Evict unpinned clean main.db pages so later reads fetch and authenticate
657+
/// durable bytes without discarding in-flight writes.
658+
pub fn evict_clean_main_pages(&self, _realm_id: crate::RealmId) {
659+
self.inner
660+
.buffer_pool
661+
.lock()
662+
.clear_clean_file(FileKey::Main);
663+
}
664+
656665
fn write_page(
657666
&self,
658667
file: FileKey,

src/txn/db/catalog.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,8 +146,8 @@ impl<V: Vfs + Clone> Db<V> {
146146
counter_anchor,
147147
commit_id: state.latest_commit_id,
148148
catalog_root: catalog_root_bytes,
149-
commit_history_root_page_id: 0,
150-
commit_history_root_version: 0,
149+
commit_history_root_page_id: state.commit_history_root_page_id,
150+
commit_history_root_version: state.commit_history_root_version,
151151
free_list_root_page_id: state.free_list_root_page_id,
152152
next_page_id: new_next,
153153
})?;

src/txn/db/misc.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ impl<V: Vfs + Clone> Db<V> {
5858
self.snapshot.read().next_page_id
5959
}
6060

61-
/// Evict all clean and dirty pages for the main realm from the buffer
62-
/// pool. Intended for integration tests that corrupt pages on disk and
63-
/// want subsequent reads to see the disk contents rather than cached data.
61+
/// Evict unpinned clean pages for the main realm from the buffer pool.
62+
/// Intended for integration tests that corrupt durable pages and want
63+
/// subsequent reads to see disk contents without dropping in-flight writes.
6464
pub fn evict_main_pages(&self, realm: crate::RealmId) {
65-
self.pager.discard_dirty_main(realm);
65+
self.pager.evict_clean_main_pages(realm);
6666
}
6767

6868
/// Return the current size of `main.db` in bytes. Useful for tests that

src/txn/write/txn.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,14 @@ impl<'db, V: Vfs + Clone> WriteTxn<'db, V> {
154154
// Assign a txn_seq starting from 1: fetch_add returns the old value (0
155155
// for the first call), so we add 1 to produce 1-based ids.
156156
let txn_seq = db.txn_seq.fetch_add(1, Ordering::Relaxed) + 1;
157+
158+
// Drop cannot await VFS removal. Sweep the only scratch file a dropped
159+
// predecessor could have left before this transaction can allocate its
160+
// own path. Abort and commit already remove their files eagerly.
161+
if txn_seq > 1 {
162+
let _ = db.vfs.remove(&format!("tmp/scratch-{}", txn_seq - 1)).await;
163+
}
164+
157165
Ok(Self {
158166
db,
159167
guard,
@@ -377,6 +385,9 @@ impl<V: Vfs + Clone> Drop for WriteTxn<'_, V> {
377385
fn drop(&mut self) {
378386
if !self.committed_or_aborted {
379387
self.db.pager.discard_dirty_main(self.db.realm_id);
388+
self.db
389+
.spill_bytes_in_use
390+
.store(0, std::sync::atomic::Ordering::Relaxed);
380391
}
381392
}
382393
}

tests/commit_history.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::time::Duration;
33
use pagedb::errors::PagedbError;
44
use pagedb::options::{OpenOptions, RetainPolicy};
55
use pagedb::vfs::memory::MemVfs;
6-
use pagedb::{CommitId, Db, RealmId};
6+
use pagedb::{CommitId, Db, RealmId, RealmQuotas};
77

88
const PAGE: usize = 4096;
99
const KEK: [u8; 32] = [7u8; 32];
@@ -144,6 +144,38 @@ async fn history_persists_across_reopen() {
144144
.unwrap_or_else(|e| panic!("expected commit {cid3:?} to survive reopen, got {e:?}"));
145145
}
146146

147+
#[tokio::test(flavor = "current_thread")]
148+
async fn quota_update_preserves_history_across_reopen() {
149+
let vfs = MemVfs::new();
150+
let opts = OpenOptions::default().with_commit_history_retain(RetainPolicy::Unbounded);
151+
152+
let first_commit = {
153+
let db = Db::open_internal_with_options(vfs.clone(), KEK, PAGE, REALM, opts.clone())
154+
.await
155+
.unwrap();
156+
let ids = write_n(&db, 2).await;
157+
db.set_realm_quotas(REALM, RealmQuotas::default())
158+
.await
159+
.unwrap();
160+
db.begin_read_at(ids[0])
161+
.await
162+
.expect("quota publication must preserve live retained history");
163+
ids[0]
164+
};
165+
166+
let reopened = Db::open_existing_with_options(vfs, KEK, PAGE, REALM, opts)
167+
.await
168+
.unwrap();
169+
let historical = reopened
170+
.begin_read_at(first_commit)
171+
.await
172+
.expect("quota publication must preserve retained history after reopen");
173+
assert_eq!(
174+
historical.get(b"k").await.unwrap().as_deref(),
175+
Some(0_u64.to_le_bytes().as_slice())
176+
);
177+
}
178+
147179
#[tokio::test(flavor = "current_thread")]
148180
async fn retain_policy_age_prunes_old() {
149181
// Age(0) means threshold = now - 0 = now, so every entry with

tests/spill_basic.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,38 @@ async fn spill_cleanup_on_abort() {
108108
assert!(res.is_err(), "tmp file should be cleaned up after abort");
109109
}
110110

111+
#[tokio::test(flavor = "current_thread")]
112+
async fn spill_drop_resets_stats_and_next_write_sweeps_stale_tmp() {
113+
let vfs = MemVfs::new();
114+
let opts = OpenOptions::default().with_scratch_bytes(1024 * 1024);
115+
let db = Db::open_internal_with_options(vfs.clone(), [9u8; 32], PAGE, REALM, opts)
116+
.await
117+
.unwrap();
118+
{
119+
let mut w = db.begin_write().await.unwrap();
120+
let mut s = w.spill_scope();
121+
s.append(b"drop-cleanup").await.unwrap();
122+
drop(s);
123+
drop(w);
124+
}
125+
126+
assert_eq!(
127+
db.stats().await.unwrap().spill_bytes_in_use,
128+
0,
129+
"dropping an uncommitted transaction must clear observable spill accounting"
130+
);
131+
132+
{
133+
let w = db.begin_write().await.unwrap();
134+
let res = vfs.open("tmp/scratch-1", OpenMode::Read).await;
135+
assert!(
136+
res.is_err(),
137+
"the next write transaction must sweep stale spill tmp files from dropped transactions"
138+
);
139+
w.abort().await;
140+
}
141+
}
142+
111143
#[tokio::test(flavor = "current_thread")]
112144
async fn spill_aead_protects_payload() {
113145
// Confirm bytes on disk are NOT plaintext.

tests/stats_basic.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,38 @@ async fn stats_reports_buffer_pool() {
117117
);
118118
}
119119

120+
#[tokio::test]
121+
async fn evict_main_pages_forces_next_read_to_miss_cache() {
122+
let db = fresh_db().await;
123+
{
124+
let mut txn = db.begin_write().await.unwrap();
125+
txn.put(b"hello", b"world").await.unwrap();
126+
txn.commit().await.unwrap();
127+
}
128+
129+
{
130+
let reader = db.begin_read().await.unwrap();
131+
assert_eq!(
132+
reader.get(b"hello").await.unwrap().as_deref(),
133+
Some(b"world".as_slice())
134+
);
135+
}
136+
let before = db.stats().await.unwrap();
137+
138+
db.evict_main_pages(realm());
139+
140+
let reader = db.begin_read().await.unwrap();
141+
assert_eq!(
142+
reader.get(b"hello").await.unwrap().as_deref(),
143+
Some(b"world".as_slice())
144+
);
145+
let after = db.stats().await.unwrap();
146+
assert!(
147+
after.buffer_pool_misses > before.buffer_pool_misses,
148+
"eviction must force a VFS-backed cache miss: before={before:?}, after={after:?}"
149+
);
150+
}
151+
120152
#[tokio::test]
121153
async fn stats_reports_mode() {
122154
let vfs = MemVfs::new();

0 commit comments

Comments
 (0)