From ced5aea6e498facd497c1552c0b4ef3f280d8ab7 Mon Sep 17 00:00:00 2001 From: Ramon Nou Date: Wed, 11 Mar 2026 18:57:16 +0100 Subject: [PATCH 1/2] regression fix --- include/client/open_file_map.hpp | 2 - src/client/gkfs_metadata.cpp | 18 ++++---- src/client/intercept.cpp | 20 ++++----- src/client/open_file_map.cpp | 15 ------- src/client/rpc/forward_metadata.cpp | 43 +++++++++++++------ .../backend/metadata/rocksdb_backend.cpp | 29 ++----------- 6 files changed, 52 insertions(+), 75 deletions(-) diff --git a/include/client/open_file_map.hpp b/include/client/open_file_map.hpp index 54a2fdb20..89aa24cea 100644 --- a/include/client/open_file_map.hpp +++ b/include/client/open_file_map.hpp @@ -200,8 +200,6 @@ public: int get_fd_idx(); - std::vector - get_range(unsigned int first, unsigned int last); bool empty() const; diff --git a/src/client/gkfs_metadata.cpp b/src/client/gkfs_metadata.cpp index 426d0f397..c7e522918 100644 --- a/src/client/gkfs_metadata.cpp +++ b/src/client/gkfs_metadata.cpp @@ -162,7 +162,7 @@ generate_lock_file(const std::string& path, bool increase) { if(md) { if(md->size() == 1 || md->size() == 0) { LOG(DEBUG, "Deleting Lock file {}", lock_path); - gkfs::rpc::forward_remove(lock_path, false, 0); + gkfs::rpc::forward_remove(lock_path, false, 0, md->size()); } else { gkfs::rpc::forward_decr_size(lock_path, md.value().size() - 1, 0); @@ -502,8 +502,8 @@ gkfs_remove(const std::string& path) { return -1; } } - auto err = gkfs::rpc::forward_remove(new_path, false, - CTX->get_replicas()); + auto err = gkfs::rpc::forward_remove( + new_path, false, CTX->get_replicas(), md->size()); if(err) { errno = err; return -1; @@ -517,7 +517,8 @@ gkfs_remove(const std::string& path) { if(gkfs::config::proxy::fwd_remove && CTX->use_proxy()) { err = gkfs::rpc::forward_remove_proxy(path, false); } else { - err = gkfs::rpc::forward_remove(path, false, CTX->get_replicas()); + err = gkfs::rpc::forward_remove(path, false, CTX->get_replicas(), + md->size()); } if(err) { errno = err; @@ -601,8 +602,8 @@ gkfs_rename(const string& old_path, const string& new_path) { auto is_dir = false; if(S_ISDIR(md_old->mode())) is_dir = true; - err = gkfs::rpc::forward_remove(old_path, is_dir, - CTX->get_replicas()); + err = gkfs::rpc::forward_remove( + old_path, is_dir, CTX->get_replicas(), md_old->size()); if(err) { errno = err; return -1; @@ -636,7 +637,8 @@ gkfs_rename(const string& old_path, const string& new_path) { if(S_ISDIR(md_old->mode())) is_dir = true; // Remove intermediate file - gkfs::rpc::forward_remove(old_path, is_dir, CTX->get_replicas()); + gkfs::rpc::forward_remove(old_path, is_dir, CTX->get_replicas(), + md_old->size()); } int err = 0; if(!S_ISLNK(md_old->mode())) { @@ -1365,7 +1367,7 @@ gkfs_rmdir(const std::string& path) { if(gkfs::config::proxy::fwd_remove && CTX->use_proxy()) { err = gkfs::rpc::forward_remove_proxy(path, true); } else { - err = gkfs::rpc::forward_remove(path, true, CTX->get_replicas()); + err = gkfs::rpc::forward_remove(path, true, CTX->get_replicas(), 0); } if(err) { errno = err; diff --git a/src/client/intercept.cpp b/src/client/intercept.cpp index 249212212..8e839e6c9 100644 --- a/src/client/intercept.cpp +++ b/src/client/intercept.cpp @@ -593,18 +593,16 @@ hook(long syscall_number, long arg0, long arg1, long arg2, long arg3, long arg4, auto last = static_cast(arg1); auto flags = static_cast(arg2); - // Close any GekkoFS virtual fds in the range ourselves. - // Iterate only over our tracked fds to avoid touching native fds. - auto to_close = CTX->file_map()->get_range(first, last); - for(auto fd : to_close) { - gkfs::syscall::gkfs_close(fd); + auto fds = get_open_fds(); + for(auto fd : fds) { + if(fd < first || fd > last) + continue; + if(CTX->file_map()->exist(fd)) { + gkfs::syscall::gkfs_close(fd); + } else + close(fd); } - - // Forward the range to the kernel for all native fds. - // The kernel handles O_CLOEXEC semantics and won't touch our - // internal fds (which live in the upper fd range). - *result = syscall_no_intercept_wrapper(SYS_close_range, first, last, - flags); + *result = 0; break; } #endif // SYS_close_range diff --git a/src/client/open_file_map.cpp b/src/client/open_file_map.cpp index 71a32d113..29a3178ba 100644 --- a/src/client/open_file_map.cpp +++ b/src/client/open_file_map.cpp @@ -341,21 +341,6 @@ OpenFileMap::get_fd_idx() { return fd_idx; } -std::vector -OpenFileMap::get_range(unsigned int first, unsigned int last) { - std::vector result; - for(auto& shard : shards_) { - lock_guard lock(shard.mutex); - auto it = shard.files.lower_bound(static_cast(first)); - while(it != shard.files.end() && - static_cast(it->first) <= last) { - result.push_back(it->first); - ++it; - } - } - std::sort(result.begin(), result.end()); - return result; -} bool OpenFileMap::empty() const { diff --git a/src/client/rpc/forward_metadata.cpp b/src/client/rpc/forward_metadata.cpp index 8f0cc4c5c..c453d3a02 100644 --- a/src/client/rpc/forward_metadata.cpp +++ b/src/client/rpc/forward_metadata.cpp @@ -115,7 +115,8 @@ forward_stat(const std::string& path, string& attr, string& inline_data, } int -forward_remove(const std::string& path, bool rm_dir, int8_t num_copies) { +forward_remove(const std::string& path, bool rm_dir, int8_t num_copies, + int64_t size) { int err = 0; // Step 1: Remove metadata from the responsible server(s) @@ -145,23 +146,37 @@ forward_remove(const std::string& path, bool rm_dir, int8_t num_copies) { // clean up whatever chunks it holds for this file. // NOTE: When using the proxy, the proxy handles this broadcast itself, so // we skip it here (CTX->use_proxy() is checked by the caller). - // We send remove_data unconditionally for non-directory removes. // Daemons handle the no-op case safely (empty chunk dir == nothing to do). if(!rm_dir) { - auto rpc_remove_data = - CTX->rpc_engine()->define(gkfs::rpc::tag::remove_data); + if(size > 0) { + auto rpc_remove_data = + CTX->rpc_engine()->define(gkfs::rpc::tag::remove_data); + + gkfs::rpc::rpc_rm_node_in_t rm_in; + rm_in.path = path; + rm_in.rm_dir = + true; // tells daemon to remove the whole chunk directory + + auto chunk_size = gkfs::config::rpc::chunksize; + auto num_chunks = size / chunk_size; + if(size % chunk_size != 0) { + num_chunks++; + } - gkfs::rpc::rpc_rm_node_in_t rm_in; - rm_in.path = path; - rm_in.rm_dir = true; // tells daemon to remove the whole chunk directory + std::unordered_set host_ids; + for(uint64_t chnk_id = 0; chnk_id < num_chunks; chnk_id++) { + host_ids.insert( + CTX->distributor()->locate_data(path, chnk_id, 0)); + } - for(std::size_t i = 0; i < CTX->hosts().size(); i++) { - try { - rpc_remove_data.on(CTX->hosts().at(i)).async(rm_in); - } catch(const std::exception& e) { - LOG(WARNING, - "{}() Failed to send remove_data RPC to host {}: {}", - __func__, i, e.what()); + for(auto host_id : host_ids) { + try { + rpc_remove_data.on(CTX->hosts().at(host_id)).async(rm_in); + } catch(const std::exception& e) { + LOG(WARNING, + "{}() Failed to send remove_data RPC to host {}: {}", + __func__, host_id, e.what()); + } } } } diff --git a/src/daemon/backend/metadata/rocksdb_backend.cpp b/src/daemon/backend/metadata/rocksdb_backend.cpp index abf326a8a..8649ba275 100644 --- a/src/daemon/backend/metadata/rocksdb_backend.cpp +++ b/src/daemon/backend/metadata/rocksdb_backend.cpp @@ -457,7 +457,8 @@ RocksDBBackend::get_dirents_extended_impl(const std::string& dir, assert(!name.empty()); // Filter out inline data keys - if(name.size() >= 7 && name.substr(name.size() - 7) == "#inline") { + if(name.size() >= 7 && + name.compare(name.size() - 7, 7, "#inline") == 0) { continue; } @@ -529,7 +530,8 @@ RocksDBBackend::get_all_dirents_extended_impl(const std::string& dir, assert(!name.empty()); // Filter out inline data keys - if(name.size() >= 7 && name.substr(name.size() - 7) == "#inline") { + if(name.size() >= 7 && + name.compare(name.size() - 7, 7, "#inline") == 0) { continue; } @@ -723,30 +725,7 @@ RocksDBBackend::db_size_impl() const { */ void RocksDBBackend::optimize_database_impl() { - constexpr size_t kib = 1024; - constexpr size_t mib = 1024 * kib; - options_.max_successive_merges = 128; - options_.level_compaction_dynamic_level_bytes = true; - options_.max_background_jobs = - std::max(4, static_cast(std::thread::hardware_concurrency())); - options_.bytes_per_sync = 1 * mib; - - // Metadata workloads are point-lookups and small updates heavy. - options_.write_buffer_size = 64 * mib; - options_.max_write_buffer_number = 4; - options_.min_write_buffer_number_to_merge = 2; - options_.target_file_size_base = 64 * mib; - options_.max_bytes_for_level_base = 256 * mib; - options_.optimize_filters_for_hits = true; - - rdb::BlockBasedTableOptions table_options; - table_options.block_size = 16 * kib; - table_options.cache_index_and_filter_blocks = true; - table_options.pin_l0_filter_and_index_blocks_in_cache = true; - table_options.block_cache = rdb::NewLRUCache(128 * mib); - table_options.filter_policy.reset(rdb::NewBloomFilterPolicy(10, false)); - options_.table_factory.reset(rdb::NewBlockBasedTableFactory(table_options)); } -- GitLab From af1773248d9a49c45391fd87403c9f8a32749bf6 Mon Sep 17 00:00:00 2001 From: Ramon Nou Date: Wed, 11 Mar 2026 19:04:21 +0100 Subject: [PATCH 2/2] fix --- include/client/rpc/forward_metadata.hpp | 3 ++- src/client/gkfs_metadata.cpp | 8 ++++---- src/client/rpc/forward_metadata.cpp | 5 ++++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/include/client/rpc/forward_metadata.hpp b/include/client/rpc/forward_metadata.hpp index 52f728969..7242da6d7 100644 --- a/include/client/rpc/forward_metadata.hpp +++ b/include/client/rpc/forward_metadata.hpp @@ -77,7 +77,8 @@ forward_rename(const std::string& oldpath, const std::string& newpath, const gkfs::metadata::Metadata& md); int -forward_remove(const std::string& path, bool rm_dir, const int8_t num_copies); +forward_remove(const std::string& path, bool rm_dir, const int8_t num_copies, + int64_t size); int forward_decr_size(const std::string& path, size_t length, const int copy); diff --git a/src/client/gkfs_metadata.cpp b/src/client/gkfs_metadata.cpp index c7e522918..7fbf13d46 100644 --- a/src/client/gkfs_metadata.cpp +++ b/src/client/gkfs_metadata.cpp @@ -471,12 +471,12 @@ gkfs_libcremove(const std::string& path) { */ int gkfs_remove(const std::string& path) { + auto md = gkfs::utils::get_metadata(path); + if(!md) { + return -1; + } if(gkfs::config::metadata::rename_support) { - auto md = gkfs::utils::get_metadata(path); - if(!md) { - return -1; - } if(S_ISDIR(md->mode())) { LOG(ERROR, "Cannot remove directory '{}'", path); diff --git a/src/client/rpc/forward_metadata.cpp b/src/client/rpc/forward_metadata.cpp index c453d3a02..44ef5cafc 100644 --- a/src/client/rpc/forward_metadata.cpp +++ b/src/client/rpc/forward_metadata.cpp @@ -148,7 +148,10 @@ forward_remove(const std::string& path, bool rm_dir, int8_t num_copies, // we skip it here (CTX->use_proxy() is checked by the caller). // Daemons handle the no-op case safely (empty chunk dir == nothing to do). if(!rm_dir) { - if(size > 0) { + bool is_inline = gkfs::config::metadata::use_inline_data && + size <= gkfs::config::metadata::inline_data_size; + + if(size > 0 && !is_inline) { auto rpc_remove_data = CTX->rpc_engine()->define(gkfs::rpc::tag::remove_data); -- GitLab