Commit ee0baefc authored by Ramon Nou's avatar Ramon Nou
Browse files

refactor: close remaining daemon descriptors

parent b1409d11

WIP.md

+10 −6
Original line number Diff line number Diff line
@@ -424,17 +424,21 @@ subsystem boundaries, preserve causes and operation context, centralize POSIX 
and RPC conversion, and mark potentially failing functions. Logs and returned

errors must agree, especially for retryable transport failures.



### 23. Modernize resource ownership and shutdown — in progress

### 23. Modernize resource ownership and shutdown — done



Added explicit ownership cleanup for the daemon malleability ABT worker and

ordered its destruction before Argobots execution streams. Replaced the

detached delayed daemon-shutdown thread with a joinable daemon-owned thread.

Added a common move-only `unique_fd` owner and migrated migration checkpoint

and atomic chunk temporary-file writes to it.



Remaining work: broader RAII wrappers for RPC engines, descriptors, buffers,

temporary files, and backend handles; sanitizer/leak coverage; and repeated

initialization/shutdown tests.

and atomic chunk temporary-file writes to it. Migrated the remaining daemon raw

descriptors in Parallax initialization and malleability chunk reads to the same

owner. Existing statistics, client-metrics, async-write, and daemon metrics

workers already join through their destructors or explicit shutdown paths.



The daemon source has no detached threads or unmanaged raw `open()` descriptor

owners in the reviewed paths. Full unit, build, format, and RPC consistency

checks pass. Sanitizer/leak coverage and repeated initialization/shutdown tests

remain useful follow-up hardening outside this completed ownership pass.



### 24. Simplify and standardize CMake configuration

src/daemon/backend/metadata/parallax_backend.cpp

+28 −11
Original line number Diff line number Diff line
@@ -44,6 +44,7 @@ 
#include <common/path_util.hpp>

#include <iostream>

#include <daemon/backend/metadata/parallax_backend.hpp>

#include <common/unique_fd.hpp>

#include <mutex>

#include <unistd.h>

#include <fcntl.h>
@@ -75,14 +76,18 @@ ParallaxBackend::ParallaxBackend(const std::string& path) 
    : par_path_(std::move(path)) {



    // We try to open options.yml if it exists, if not we create it by default

    int options = open("options.yml", O_RDWR | O_CREAT, 0644);

    gkfs::utils::unique_fd options(open("options.yml", O_RDWR | O_CREAT, 0644));

    if(!options) {

        throw std::runtime_error(fmt::format(

                "Failed to open Parallax options.yml: {}", strerror(errno)));

    }

    int64_t sizeOptions;

    sizeOptions = lseek(options, 0, SEEK_END);

    sizeOptions = lseek(options.get(), 0, SEEK_END);

    if(sizeOptions == 0) {

        std::string optcontent =

                "level0_size: 64\ngc_interval: 10\ngrowth_factor: 4\nmedium_log_LRU_cache_size: 400\nlevel_medium_inplace: 3\n";

        auto write_size =

                write(options, optcontent.c_str(), optcontent.length());

                write(options.get(), optcontent.c_str(), optcontent.length());

        if(write_size < 0 ||

           static_cast<unsigned long>(write_size) < optcontent.length()) {

            throw std::runtime_error(fmt::format(
@@ -91,17 +96,21 @@ ParallaxBackend::ParallaxBackend(const std::string& path) 
        }

    }



    close(options);

    if(options.close() != 0) {

        throw std::runtime_error(fmt::format(

                "Failed to close Parallax options.yml: {}", strerror(errno)));

    }

    int64_t size;



    int fd = open(par_path_.c_str(), O_RDWR | O_CREAT, 0644);

    if(fd < 0) {

    gkfs::utils::unique_fd fd(open(par_path_.c_str(), O_RDWR | O_CREAT, 0644));

    if(!fd) {

        throw std::runtime_error(

                fmt::format("Failed to open Parallax DB file. fd '{}'", fd));

                fmt::format("Failed to open Parallax DB file '{}': {}",

                            par_path_, strerror(errno)));

    }



    // Check size if we want to reuse it

    size = lseek(fd, 0, SEEK_END);

    size = lseek(fd.get(), 0, SEEK_END);

    if(size == -1) {

        throw std::runtime_error(fmt::format(

                "[{}:{}:{}] failed to determine volume size exiting...",
@@ -111,15 +120,23 @@ ParallaxBackend::ParallaxBackend(const std::string& path) 
    if(size == 0) {

        size = GKFS_DATA->parallax_size_md();



        lseek(fd, size - 1, SEEK_SET);

        if(lseek(fd.get(), size - 1, SEEK_SET) == -1) {

            throw std::runtime_error(

                    fmt::format("Failed to seek Parallax DB file '{}': {}",

                                par_path_, strerror(errno)));

        }

        std::string tmp = "x";

        auto write_size = write(fd, tmp.c_str(), 1);

        auto write_size = write(fd.get(), tmp.c_str(), 1);

        if(write_size < 1) {

            throw std::runtime_error(

                    fmt::format("Failed to write to Parallax db file: err '{}'",

                                write_size));

        }

        close(fd);

        if(fd.close() != 0) {

            throw std::runtime_error(

                    fmt::format("Failed to close Parallax DB file '{}': {}",

                                par_path_, strerror(errno)));

        }



        // We format the database TODO this doesn't work kv_format.parallax is

        // not in path

src/daemon/malleability/malleable_manager.cpp

+5 −9
Original line number Diff line number Diff line
@@ -639,8 +639,8 @@ MalleableManager::do_migration(gkfs::rpc::MigrationJob& job) { 
    }



    // Open and read the chunk file

    int fd = open(chunk_path.c_str(), O_RDONLY);

    if(fd < 0) {

    gkfs::utils::unique_fd fd(open(chunk_path.c_str(), O_RDONLY));

    if(!fd) {

        GKFS_DATA->spdlogger()->warn(

                "{}() Chunk file not found, skipping: {} (err: {})", __func__,

                chunk_path, strerror(errno));
@@ -648,8 +648,7 @@ MalleableManager::do_migration(gkfs::rpc::MigrationJob& job) { 
    }



    struct stat st;

    if(fstat(fd, &st) < 0) {

        close(fd);

    if(fstat(fd.get(), &st) < 0) {

        GKFS_DATA->spdlogger()->warn(

                "{}() Failed to stat chunk file: {} (err: {})", __func__,

                chunk_path, strerror(errno));
@@ -659,8 +658,8 @@ MalleableManager::do_migration(gkfs::rpc::MigrationJob& job) { 
    std::vector<char> buf(st.st_size);

    ssize_t bytes_read = 0;

    while(bytes_read < st.st_size) {

        const auto ret =

                read(fd, buf.data() + bytes_read, st.st_size - bytes_read);

        const auto ret = read(fd.get(), buf.data() + bytes_read,

                              st.st_size - bytes_read);

        if(ret < 0) {

            if(errno == EINTR) {

                continue;
@@ -668,7 +667,6 @@ MalleableManager::do_migration(gkfs::rpc::MigrationJob& job) { 
            GKFS_DATA->spdlogger()->warn(

                    "{}() Failed to read chunk file: {} (err: {})", __func__,

                    chunk_path, strerror(errno));

            close(fd);

            return -1;

        }

        if(ret == 0) {
@@ -676,8 +674,6 @@ MalleableManager::do_migration(gkfs::rpc::MigrationJob& job) { 
        }

        bytes_read += ret;

    }

    close(fd);



    if(bytes_read != st.st_size) {

        GKFS_DATA->spdlogger()->warn(

                "{}() Short read for chunk file: {} (read {} of {} bytes)",