Skip to content

Commit

Permalink
Catch IO exceptions when flushing WAL and log them (#3283)
Browse files Browse the repository at this point in the history
Unhandled exceptions in the Database destructor can't be easily handled in the
rust API, and even in the C++ API it may cause terminate to be called, which is not very
user-friendly and will bypass other error reporting mechanisms.
  • Loading branch information
benjaminwinger committed Apr 16, 2024
1 parent b24d41b commit 014c17a
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 5 deletions.
4 changes: 3 additions & 1 deletion src/common/file_system/local_file_system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,9 @@ void LocalFileSystem::syncFile(const FileInfo& fileInfo) const {
#if defined(_WIN32)
// Note that `FlushFileBuffers` returns 0 when fail, while `fsync` returns 0 when succeed.
if (FlushFileBuffers((HANDLE)localFileInfo.handle) == 0) {
throw IOException(stringFormat("Failed to sync file {}.", fileInfo.path));
auto error = GetLastError();
throw IOException(stringFormat("Failed to sync file {}. Error {}: {}", fileInfo.path, error,
std::system_category().message(error)));
}
#else
if (fsync(localFileInfo.fd) != 0) {
Expand Down
8 changes: 7 additions & 1 deletion src/storage/wal/wal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@

#include <fcntl.h>

#include "common/exception/io.h"
#include "common/file_system/file_info.h"
#include "common/file_system/virtual_file_system.h"
#include "common/serializer/buffered_file.h"
#include "common/serializer/deserializer.h"
#include "common/serializer/serializer.h"
#include "spdlog/spdlog.h"
#include "storage/storage_utils.h"

using namespace kuzu::common;
Expand All @@ -31,7 +33,11 @@ WAL::WAL(const std::string& directory, bool readOnly, BufferManager& bufferManag
}

WAL::~WAL() {
flushAllPages();
try {
flushAllPages();
} catch (IOException& e) {
spdlog::error("Failed to flush WAL: {}", e.what());
}
}

page_idx_t WAL::logPageUpdateRecord(DBFileID dbFileID, page_idx_t pageIdxInOriginalFile) {
Expand Down
5 changes: 2 additions & 3 deletions tools/rust_api/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,8 @@ mod tests {

#[test]
fn create_database_failure() {
let result: Error = Database::new("", SystemConfig::default())
.expect_err("An empty string should not be a valid database path!")
.into();
Database::new("", SystemConfig::default())
.expect_err("An empty string should not be a valid database path!");
}

#[test]
Expand Down

0 comments on commit 014c17a

Please sign in to comment.