Skip to content

Commit 5f1a431

Browse files
presempathy-awbfarhan-syah
authored andcommitted
fix(txn): preserve cache and transaction lifecycle state
1 parent 74b1b37 commit 5f1a431

8 files changed

Lines changed: 190 additions & 8 deletions

File tree

src/pager/cache.rs

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,10 @@ impl PageCache {
153153
}
154154

155155
fn evict_one(&mut self) -> Option<(FileKey, u64)> {
156-
// Walk the hand at most `capacity * 2` steps to bound worst case
157-
// (every entry either visited or unevictable triggers a wrap).
158-
let max_steps = self.capacity.saturating_mul(2).max(1);
156+
// Two passes are sufficient to clear visited bits and then evict.
157+
// Use the live list length rather than the configured capacity because
158+
// pinned or dirty pages can temporarily grow the cache past capacity.
159+
let max_steps = self.map.len().saturating_mul(2).max(1);
159160
let mut cur = self.hand.or(self.tail);
160161
for _ in 0..max_steps {
161162
let Some(idx) = cur else {
@@ -293,6 +294,33 @@ impl PageCache {
293294
self.dirty.remove(&key);
294295
}
295296
}
297+
298+
/// Drop every unpinned clean entry for `file`, leaving dirty entries
299+
/// available for a later flush and pinned entries available to readers.
300+
pub fn clear_clean_file(&mut self, file: FileKey) {
301+
let keys: Vec<(FileKey, u64)> = self
302+
.map
303+
.keys()
304+
.filter(|(f, _)| *f == file)
305+
.copied()
306+
.collect();
307+
for key in keys {
308+
if self.pins.get(&key).copied().unwrap_or(0) > 0 || self.dirty.contains(&key) {
309+
continue;
310+
}
311+
if let Some(idx) = self.map.remove(&key) {
312+
let (prev, next) = {
313+
let node = self.slab[idx].as_ref().expect("indexed node alive");
314+
(node.prev, node.next)
315+
};
316+
if self.hand == Some(idx) {
317+
self.hand = next;
318+
}
319+
self.unlink_node(idx, prev, next);
320+
self.free_node(idx);
321+
}
322+
}
323+
}
296324
}
297325

298326
#[cfg(test)]
@@ -368,6 +396,20 @@ mod tests {
368396
assert!(c.get((FileKey::Main, 1)).is_some());
369397
}
370398

399+
#[test]
400+
fn clear_clean_file_preserves_dirty_entries() {
401+
let mut c = PageCache::with_capacity(4);
402+
c.insert((FileKey::Main, 1), page(1));
403+
c.insert((FileKey::Main, 2), page(2));
404+
c.mark_dirty((FileKey::Main, 2));
405+
406+
c.clear_clean_file(FileKey::Main);
407+
408+
assert!(c.get((FileKey::Main, 1)).is_none());
409+
assert!(c.get((FileKey::Main, 2)).is_some());
410+
assert!(c.is_dirty((FileKey::Main, 2)));
411+
}
412+
371413
#[test]
372414
fn dirty_iter_is_sorted_ascending() {
373415
let mut c = PageCache::with_capacity(16);
@@ -396,4 +438,27 @@ mod tests {
396438
c.insert((FileKey::Main, 3), page(3));
397439
assert_eq!(c.slab.len(), 2, "slab reuses freed slot");
398440
}
441+
442+
#[test]
443+
fn overgrown_cache_still_finds_clean_victim() {
444+
let mut c = PageCache::with_capacity(2);
445+
446+
for id in 1u8..=4 {
447+
let key = (FileKey::Main, u64::from(id));
448+
c.insert(key, page(id));
449+
c.pin(key);
450+
}
451+
c.insert((FileKey::Main, 5), page(5));
452+
assert_eq!(c.len(), 5, "pinned pages may force temporary growth");
453+
454+
let evicted = c.insert((FileKey::Main, 6), page(6));
455+
assert_eq!(
456+
evicted,
457+
Some((FileKey::Main, 5)),
458+
"eviction must scan the full over-capacity list for a clean victim"
459+
);
460+
assert_eq!(c.len(), 5);
461+
assert!(c.get((FileKey::Main, 5)).is_none());
462+
assert!(c.get((FileKey::Main, 6)).is_some());
463+
}
399464
}

src/pager/core.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -870,6 +870,15 @@ impl<V: Vfs> Pager<V> {
870870
}
871871
}
872872

873+
/// Evict unpinned clean main.db pages so later reads fetch and authenticate
874+
/// durable bytes without discarding in-flight writes.
875+
pub fn evict_clean_main_pages(&self, _realm_id: crate::RealmId) {
876+
self.inner
877+
.buffer_pool
878+
.lock()
879+
.clear_clean_file(FileKey::Main);
880+
}
881+
873882
fn write_page(
874883
&self,
875884
file: FileKey,

src/txn/db/catalog.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ impl<V: Vfs + Clone> Db<V> {
151151
counter_anchor,
152152
commit_id: state.latest_commit_id,
153153
catalog_root: catalog_root_bytes,
154-
commit_history_root_page_id: 0,
155-
commit_history_root_version: 0,
154+
commit_history_root_page_id: state.commit_history_root_page_id,
155+
commit_history_root_version: state.commit_history_root_version,
156156
free_list_root_page_id: state.free_list_root_page_id,
157157
next_page_id: new_next,
158158
})?;

src/txn/db/misc.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,15 +40,16 @@ impl<V: Vfs + Clone> Db<V> {
4040
.applies_incremental_snapshots()
4141
}
4242

43-
/// Drop every cached page of `realm` so the next read goes to storage.
43+
/// Drop unpinned clean cached pages of `realm` so the next read goes to
44+
/// storage, leaving dirty and pinned entries untouched.
4445
///
4546
/// Compiled only for the crate's own tests: nothing an embedder does is
4647
/// supposed to depend on whether a page is warm, and publishing the lever
4748
/// invites exactly that dependency. A test that needs a cold read from
4849
/// outside the crate reopens the handle instead.
4950
#[cfg(test)]
5051
pub(crate) fn evict_main_pages(&self, realm: crate::RealmId) {
51-
self.pager.discard_dirty_main(realm);
52+
self.pager.evict_clean_main_pages(realm);
5253
}
5354

5455
/// Current size of `main.db` in bytes, read from the file rather than from

src/txn/write/txn.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,14 @@ impl<'db, V: Vfs + Clone> WriteTxn<'db, V> {
183183
// Assign a txn_seq starting from 1: fetch_add returns the old value (0
184184
// for the first call), so we add 1 to produce 1-based ids.
185185
let txn_seq = db.txn_seq.fetch_add(1, Ordering::Relaxed) + 1;
186+
187+
// Drop cannot await VFS removal. Sweep the only scratch file a dropped
188+
// predecessor could have left before this transaction can allocate its
189+
// own path. Abort and commit already remove their files eagerly.
190+
if txn_seq > 1 {
191+
let _ = db.vfs.remove(&format!("tmp/scratch-{}", txn_seq - 1)).await;
192+
}
193+
186194
Ok(Self {
187195
db,
188196
guard,
@@ -407,6 +415,9 @@ impl<V: Vfs + Clone> Drop for WriteTxn<'_, V> {
407415
fn drop(&mut self) {
408416
if !self.committed_or_aborted {
409417
self.db.pager.discard_dirty_main(self.db.realm_id);
418+
self.db
419+
.spill_bytes_in_use
420+
.store(0, std::sync::atomic::Ordering::Relaxed);
410421
}
411422
}
412423
}

tests/commit_history.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::time::Duration;
22

33
use pagedb::vfs::memory::MemVfs;
4-
use pagedb::{CommitId, Db, OpenOptions, PagedbError, RealmId, RetainPolicy};
4+
use pagedb::{CommitId, Db, OpenOptions, PagedbError, RealmId, RealmQuotas, RetainPolicy};
55

66
const PAGE: usize = 4096;
77
const KEK: [u8; 32] = [7u8; 32];
@@ -140,6 +140,38 @@ async fn history_persists_across_reopen() {
140140
.unwrap_or_else(|e| panic!("expected commit {cid3:?} to survive reopen, got {e:?}"));
141141
}
142142

143+
#[tokio::test(flavor = "current_thread")]
144+
async fn quota_update_preserves_history_across_reopen() {
145+
let vfs = MemVfs::new();
146+
let opts = OpenOptions::default().with_commit_history_retain(RetainPolicy::Unbounded);
147+
148+
let first_commit = {
149+
let db = Db::open_internal_with_options(vfs.clone(), KEK, PAGE, REALM, opts.clone())
150+
.await
151+
.unwrap();
152+
let ids = write_n(&db, 2).await;
153+
db.set_realm_quotas(REALM, RealmQuotas::default())
154+
.await
155+
.unwrap();
156+
db.begin_read_at(ids[0])
157+
.await
158+
.expect("quota publication must preserve live retained history");
159+
ids[0]
160+
};
161+
162+
let reopened = Db::open_existing_with_options(vfs, KEK, PAGE, REALM, opts)
163+
.await
164+
.unwrap();
165+
let historical = reopened
166+
.begin_read_at(first_commit)
167+
.await
168+
.expect("quota publication must preserve retained history after reopen");
169+
assert_eq!(
170+
historical.get(b"k").await.unwrap().as_deref(),
171+
Some(0_u64.to_le_bytes().as_slice())
172+
);
173+
}
174+
143175
#[tokio::test(flavor = "current_thread")]
144176
async fn retain_policy_age_prunes_old() {
145177
// 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
@@ -105,6 +105,38 @@ async fn spill_cleanup_on_abort() {
105105
assert!(res.is_err(), "tmp file should be cleaned up after abort");
106106
}
107107

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