Skip to content

chore: fix journal mocking #2356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: 04-13-chore_reord_wal_to_be_stateful
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions packages/common/sqlite-vfs-fdb/src/vfs/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ pub struct FdbFileExt {
pub shm_map_count: u32,
/// WAL manager for WAL operations
pub wal_manager: WalManager,
/// Whether the journal file exists (only used for journal files)
pub journal_exists: bool,
}

/// The I/O methods for FdbFile
Expand Down Expand Up @@ -209,12 +211,17 @@ pub unsafe extern "C" fn fdb_file_read(
wal::read_wal_file(file, file_path, buf, count, offset, ext, vfs)
}
SqliteFileType::Journal => {
// For journal files, we just log operations without actual implementation
// This is sufficient because in WAL mode the journal file is only used temporarily
// during database initialization, then deleted once WAL mode is fully established
tracing::info!("Journal read operation on: {}, offset={}, count={}", file_path, offset, count);
// Return SQLITE_OK with zeroed buffer which is already done above
SQLITE_OK
// Journal files are mocked and SQLite should never read from them
// In WAL mode, journal files are only created temporarily during initialization
tracing::error!(
"UNEXPECTED journal file read attempt: {}, offset={}, count={}. This indicates a problem in WAL mode setup.",
file_path, offset, count
);

// Always return an error for journal reads - SQLite should not be reading from journal files
// when we're forcing WAL mode
metrics::record_vfs_error("journal_read_attempt");
return SQLITE_IOERR;
}
SqliteFileType::Database => {
db::read_database_file(file, file_path, buf, count, offset, ext, vfs)
Expand Down Expand Up @@ -314,6 +321,11 @@ pub unsafe extern "C" fn fdb_file_write(
preview_size, &buf_data[..preview_size]);
}

// Mark the journal file as existing now
let ext_mut = fdb_file.ext.assume_init_mut();
ext_mut.journal_exists = true;
tracing::info!("Journal file marked as existing: {}", file_path);

// Return success - the operation is tracked in memory but not persisted
// This is sufficient for initialization since the journal is temporary
SQLITE_OK
Expand Down
74 changes: 47 additions & 27 deletions packages/common/sqlite-vfs-fdb/src/vfs/general.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,28 +214,32 @@ impl FdbVfs {

match file_type_result {
Ok(file_type) => {
// For journal files, we pretend they always exist if requested
// This allows SQLite initialization to proceed normally in WAL mode
if file_type == super::file::SqliteFileType::Journal {
tracing::info!(
"Journal file exists check for {}: true (mocked response for WAL initialization)",
path
);
return Ok(true);
match file_type {
super::file::SqliteFileType::Journal => {
// For journal files, we don't really check the FDB metadata
// Instead, we should use the journal_exists flag from any open journal file
// For now, we'll just return false since we don't track open files at the VFS level
tracing::info!(
"Journal file exists check for {}: returning false (journals simulated)",
path
);
return Ok(false);
},
_ => {
// For database and WAL files, check actual metadata
let metadata_result = self.get_metadata(path)?;
let exists = metadata_result.is_some();

tracing::info!(
"File exists check for {} (type: {:?}): {}",
path,
file_type,
exists
);

Ok(exists)
}
}

// For database and WAL files, check actual metadata
let metadata_result = self.get_metadata(path)?;
let exists = metadata_result.is_some();

tracing::info!(
"File exists check for {} (type: {:?}): {}",
path,
file_type,
exists
);

Ok(exists)
},
Err(err) => {
// Log the error and return false for invalid file types
Expand Down Expand Up @@ -296,14 +300,15 @@ pub unsafe extern "C" fn vfs_open(
}
};

// For journal files, we always say they exist and create them on demand
// Check if this is a journal file
let is_journal = file_type == SqliteFileType::Journal;

// Check if file exists (except for journals which we handle specially)
// Check if file exists
let file_exists = if is_journal {
// Journal files are always treated as "exists" or "can create"
tracing::info!("Journal file auto-handled: {}", path_str);
true
// For journal files, we report as not existing initially
// The journal file will be created on demand and tracked in memory
tracing::info!("Journal file existence check for open: {}", path_str);
false
} else {
// For non-journal files, check normally
match (*vfs_instance).get_metadata(path_str) {
Expand Down Expand Up @@ -422,6 +427,8 @@ pub unsafe extern "C" fn vfs_open(
shm_map_count: 0,
// Initialize WAL manager - will be used for WAL operations
wal_manager: WalManager::new((*vfs_instance).db.clone(), (*vfs_instance).keyspace.clone()),
// Initialize journal_exists to false by default
journal_exists: false,
};
fdb_file.ext = MaybeUninit::new(ext);

Expand Down Expand Up @@ -468,7 +475,20 @@ pub unsafe extern "C" fn vfs_delete(
return SQLITE_IOERR;
}

// Delete the file
// Check if it's a journal file
let is_journal = match super::file::SqliteFileType::from_path(path_str) {
Ok(file_type) => file_type == super::file::SqliteFileType::Journal,
Err(_) => false
};

// For journal files, we handle deletion specially
if is_journal {
tracing::info!("Journal file deletion requested: {}", path_str);
// We'll just acknowledge deletion without doing anything
return SQLITE_OK;
}

// For other files, delete normally
match (*vfs_instance).delete_file(path_str) {
Ok(_) => {
tracing::debug!("Successfully deleted file: {}", path_str);
Expand Down
Loading