Commit 477005cd authored by Marc Vef's avatar Marc Vef
Browse files

Merge branch 'marc/293-remove-superfluous-rpc-during-remove' into 'master' 

Resolve "Remove superfluous RPC during remove"

This MR improves remove() performance by avoiding one RPC. 

Before the change, the metadata of a path was fetched to check whether it is a directory or file. This is important because `rmdir()` and `unlink()` should not delete the wrong object. However, this meant that two RPCs were done per remove operation.

This update changes this behavior by checking the directory/file on the server during any remove operation. For this, an additional RPC field was added, which communicates whether the intent is to remove a directory. Overall, the semantics stay the same. The special case for `rename()` still requires the metadata check beforehand and is unchanged.

IO500 has shown that this optimization doubles throughput for latency-sensitive operations, i.e., zero-byte files or small files.

Depends on !191

Closes #293

Closes #293

See merge request !195
parents 9dd6ebef b0ba5789

CHANGELOG.md

+1 −0
Original line number Diff line number Diff line
@@ -8,6 +8,7 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). 
## [Unreleased]

### New



- Remove optimization, removing one RPC per operation ([!195](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_request/195)).

- Added the GekkoFS proxy as an optional gateway between client and daemon. The proxy is started on each compute node

  that houses clients ([!191](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_request/191)).

  - Additional options for the GekkoFS daemon were added to integrate the GekkoFS proxy.

include/client/rpc/forward_metadata.hpp

+1 −1
Original line number Diff line number Diff line
@@ -62,7 +62,7 @@ forward_rename(const std::string& oldpath, const std::string& newpath, 
#endif // HAS_RENAME



int

forward_remove(const std::string& path, const int8_t num_copies);

forward_remove(const std::string& path, bool rm_dir, const int8_t num_copies);



int

forward_decr_size(const std::string& path, size_t length, const int copy);

include/client/rpc/forward_metadata_proxy.hpp

+1 −1
Original line number Diff line number Diff line
@@ -23,7 +23,7 @@ int 
forward_stat_proxy(const std::string& path, std::string& attr);



int

forward_remove_proxy(const std::string& path);

forward_remove_proxy(const std::string& path, bool rm_dir);



int

forward_decr_size_proxy(const std::string& path, size_t length);

include/client/rpc/rpc_types.hpp

+22 −6
Original line number Diff line number Diff line
@@ -520,7 +520,8 @@ struct remove_metadata { 
        hermes::detail::post_to_mercury(ExecutionContext*);



    public:

        input(const std::string& path) : m_path(path) {}

        input(const std::string& path, bool rm_dir)

            : m_path(path), m_rm_dir(rm_dir) {}



        input(input&& rhs) = default;
@@ -537,14 +538,21 @@ struct remove_metadata { 
            return m_path;

        }



        explicit input(const rpc_rm_node_in_t& other) : m_path(other.path) {}

        bool

        rm_dir() const {

            return m_rm_dir;

        }



        explicit input(const rpc_rm_node_in_t& other)

            : m_path(other.path), m_rm_dir(other.rm_dir) {}



        explicit operator rpc_rm_node_in_t() {

            return {m_path.c_str()};

            return {m_path.c_str(), m_rm_dir};

        }



    private:

        std::string m_path;

        bool m_rm_dir;

    };



    class output {
@@ -3113,7 +3121,8 @@ struct remove_proxy { 
        hermes::detail::post_to_mercury(ExecutionContext*);



    public:

        input(const std::string& path) : m_path(path) {}

        input(const std::string& path, bool rm_dir)

            : m_path(path), m_rm_dir(rm_dir) {}



        input(input&& rhs) = default;
@@ -3130,14 +3139,21 @@ struct remove_proxy { 
            return m_path;

        }



        explicit input(const rpc_rm_node_in_t& other) : m_path(other.path) {}

        bool

        rm_dir() const {

            return m_rm_dir;

        }



        explicit input(const rpc_rm_node_in_t& other)

            : m_path(other.path), m_rm_dir(other.rm_dir) {}



        explicit operator rpc_rm_node_in_t() {

            return {m_path.c_str()};

            return {m_path.c_str(), m_rm_dir};

        }



    private:

        std::string m_path;

        bool m_rm_dir;

    };



    class output {

include/common/common_defs.hpp

+8 −0
Original line number Diff line number Diff line
@@ -90,9 +90,17 @@ constexpr auto ucx_all = "ucx+all"; 
constexpr auto ucx_tcp = "ucx+tcp";

constexpr auto ucx_rc = "ucx+rc";

constexpr auto ucx_ud = "ucx+ud";

/*

 * Disable warning for unused variable

 * This is a bug in gcc that was fixed in version 13.

 * XXX Can be removed in the future

 */

#pragma GCC diagnostic push

#pragma GCC diagnostic ignored "-Wunused-variable"

constexpr auto all_remote_protocols = {ofi_sockets, ofi_tcp, ofi_verbs,

                                       ofi_psm2,    ucx_all, ucx_tcp,

                                       ucx_rc,      ucx_ud};

#pragma GCC diagnostic pop

} // namespace protocol

} // namespace gkfs::rpc