Commit 003c5231 authored by Ramon Nou's avatar Ramon Nou
Browse files

fix(rename,metadata): support atomic rename with inline data and fix spurious host failure tracking 

- In forward_rename(), create newpath preserving inline data via create_write_inline if present.
- In RocksDBBackend::put_no_exist_impl(), reclaim obsolete rename stubs (blocks == -1) and inline data entries so replaced files can be recreated.
- In Metadata::inline_data(), return const reference to avoid dangling references on temporaries.
- In forward_stat, forward_read_inline, and forward_get_metadentry_size, do not treat POSIX filesystem errors like ENOENT as server host failures.
- Guard replica host failure tracking when replicas are not configured.
parent 3f1bf940

include/common/metadata.hpp

+1 −1
Changes for include/common/metadata.hpp: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -182,7 +182,7 @@ public: 
    rename_path(const std::string& rename_path);



    /** @brief Return or set inline file data. */

    std::string

    const std::string&

    inline_data() const;

    void

    inline_data(const std::string& data);

src/client/gkfs_metadata.cpp

+2 −9
Changes for src/client/gkfs_metadata.cpp: 2 added lines, 9 removed lines.
Original line number Diff line number Diff line
@@ -606,7 +606,7 @@ gkfs_access(const std::string& path, const int mask, bool follow_links) { 


int

gkfs_rename(const string& old_path, const string& new_path) {

    auto md_old = gkfs::utils::get_metadata(old_path, false);

    auto md_old = gkfs::utils::get_metadata(old_path, false, true);

    std::string original_path = old_path;

    // if the file is not found, or it is a renamed one cancel.

    if(!md_old || md_old.value().blocks() == -1) {
@@ -659,11 +659,9 @@ gkfs_rename(const string& old_path, const string& new_path) { 
        // original file. (There will be only one level)

        original_path = md_old.value().target_path();

        if(!S_ISLNK(md_old->mode())) {

            md_old = gkfs::utils::get_metadata(original_path, false);

            md_old = gkfs::utils::get_metadata(original_path, false, true);



            if(!md_old) {

                fprintf(stderr, "GKFS_RENAME: md_old missing intermediate %d\n",

                        errno);

                return -1;

            }

        }
@@ -679,19 +677,14 @@ gkfs_rename(const string& old_path, const string& new_path) { 
    if(!S_ISLNK(md_old->mode())) {

        err = gkfs::rpc::forward_rename(original_path, new_path,

                                        md_old.value());

        fprintf(stderr, "GKFS_RENAME: forward_rename err=%d\n", err);

    } else {

        // Was a link so do a forward symlink to regenerate it

        err = gkfs_mk_symlink(new_path, original_path);

        fprintf(stderr, "GKFS_RENAME: gkfs_mk_symlink err=%d\n", err);

    }

    if(err) {

        errno = err;

        fprintf(stderr, "GKFS_RENAME: forward failed setting errno=%d\n",

                errno);

        return -1;

    }

    fprintf(stderr, "GKFS_RENAME: success returning 0\n");

    return 0;

}

src/client/preload_context.cpp

+9 −0
Changes for src/client/preload_context.cpp: 9 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -845,16 +845,25 @@ PreloadContext::get_replicas() { 


void

PreloadContext::mark_host_failure(const gkfs::rpc::host_t host) {

    if(replica_context_.count() <= 0) {

        return;

    }

    replica_context_.mark_failure(host);

}



void

PreloadContext::mark_host_success(const gkfs::rpc::host_t host) {

    if(replica_context_.count() <= 0) {

        return;

    }

    replica_context_.mark_success(host);

}



std::set<gkfs::rpc::host_t>

PreloadContext::unavailable_hosts() {

    if(replica_context_.count() <= 0) {

        return {};

    }

    return replica_context_.unavailable_hosts();

}

src/client/rpc/forward_metadata.cpp

+43 −22
Changes for src/client/rpc/forward_metadata.cpp: 43 added lines, 22 removed lines.
Original line number Diff line number Diff line
@@ -276,6 +276,8 @@ forward_stat(const std::string& path, string& attr, string& inline_data, 
            CTX->mark_host_success(target.host);

            attr = out.db_val;

            inline_data.assign(out.inline_data.begin(), out.inline_data.end());

        } else if(out.err == ENOENT || out.err == ENOTDIR) {

            CTX->mark_host_success(target.host);

        } else {

            CTX->mark_host_failure(target.host);

        }
@@ -726,12 +728,33 @@ forward_rename(const std::string& oldpath, const std::string& newpath, 
    auto endp = CTX->hosts()->at(old_target.host);



    try {

        // 1. Create newpath

        // 1. Create newpath (preserve inline data if present)

        {

            const auto new_target = gkfs::rpc::locate_metadata_replica(

                    *CTX->distributor(), newpath, 0);

            auto endp2 = CTX->hosts()->at(new_target.host);



            if(!md.inline_data().empty() &&

               gkfs::config::metadata::use_inline_data &&

               md.inline_data().size() <= gkfs::config::metadata::inline_data_size) {

                const auto& inline_buf = md.inline_data();

                gkfs::rpc::rpc_create_write_inline_in_t in;

                in.path = newpath;

                in.mode = md.mode();

                in.data = std::vector<char>(inline_buf.begin(),

                                           inline_buf.end());

                in.count = inline_buf.size();



                auto out = gkfs::rpc::forward_call<

                        gkfs::rpc::rpc_create_write_inline_out_t>(

                        CTX->rpc_engine(), endp2,

                        gkfs::rpc::tag::create_write_inline, in, __func__,

                        newpath);



                if(out.err != 0) {

                    return out.err;

                }

            } else {

                gkfs::rpc::rpc_mk_node_in_t in;

                in.path = newpath;

                in.mode = md.mode();
@@ -744,25 +767,11 @@ forward_rename(const std::string& oldpath, const std::string& newpath, 
                    return out.err;

                }

            }



        // 2. Rename newpath -> oldpath

        {

            const auto new_target = gkfs::rpc::locate_metadata_replica(

                    *CTX->distributor(), newpath, 0);

            auto endp2 = CTX->hosts()->at(new_target.host);



            gkfs::rpc::rpc_rename_in_t in;

            in.path = newpath;

            in.target_path = oldpath;

            in.renamed_stub = false;



            auto out = gkfs::rpc::forward_call<gkfs::rpc::rpc_err_out_t>(

                    CTX->rpc_engine(), endp2, gkfs::rpc::tag::rename, in,

                    __func__, newpath);

            (void) out;

            // Ignore error from prior step to proceed to step 3

        }



        // Step 2 omitted: newpath is already created with correct mode, size, and inline data/chunks.

        // It does not point back to oldpath, so operations on newpath access newpath directly.



        // 3. Rename oldpath -> newpath

        {

            gkfs::rpc::rpc_rename_in_t in;
@@ -860,9 +869,14 @@ forward_read_inline(const std::string& path, void* buf, off64_t offset, 
    int first_error = EBUSY;

    bool saw_eagain = false;



    for(const auto& target : gkfs::rpc::locate_metadata_replicas(

                *CTX->distributor(), path, num_copies)) {

        if(CTX->unavailable_hosts().count(target.host) != 0) {

    auto targets = gkfs::rpc::locate_metadata_replicas(

                *CTX->distributor(), path, num_copies);

    const auto unavailable_hosts = num_copies > 0

                                           ? CTX->unavailable_hosts()

                                           : std::set<gkfs::rpc::host_t>{};



    for(const auto& target : targets) {

        if(unavailable_hosts.count(target.host) != 0) {

            continue;

        }

        try {
@@ -893,7 +907,11 @@ forward_read_inline(const std::string& path, void* buf, off64_t offset, 
            if(out.err != 0 && first_error == EBUSY) {

                first_error = out.err;

            }

            if(out.err == ENOENT) {

                CTX->mark_host_success(target.host);

            } else {

                CTX->mark_host_failure(target.host);

            }

        } catch(const std::exception& ex) {

            LOG(ERROR, "{}() inline read replica {} failed: '{}'", __func__,

                target.copy, ex.what());
@@ -911,7 +929,8 @@ pair<int, off64_t> 
forward_get_metadentry_size(const std::string& path, const int copy) {

    const auto target =

            gkfs::rpc::locate_metadata_replica(*CTX->distributor(), path, copy);

    if(CTX->unavailable_hosts().count(target.host) != 0) {

    if(CTX->get_replicas() > 0 &&

       CTX->unavailable_hosts().count(target.host) != 0) {

        return make_pair(EHOSTDOWN, 0);

    }

    auto endp = CTX->hosts()->at(target.host);
@@ -927,6 +946,8 @@ forward_get_metadentry_size(const std::string& path, const int copy) { 


        if(out.err == 0) {

            CTX->mark_host_success(target.host);

        } else if(out.err == ENOENT || out.err == ENOTDIR) {

            CTX->mark_host_success(target.host);

        } else {

            CTX->mark_host_failure(target.host);

        }

src/common/metadata.cpp

+1 −1
Changes for src/common/metadata.cpp: 1 added line, 1 removed line.
Original line number Diff line number Diff line
@@ -362,7 +362,7 @@ Metadata::rename_path(const std::string& rename_path) { 
    rename_path_ = rename_path;

}



std::string

const std::string&

Metadata::inline_data() const {

    return inline_data_;

}