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

Harden mutation boundary with replication

parent c36e3cac

README.md

+2 −0
Changes for README.md: 2 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -1046,6 +1046,8 @@ Client-metrics require the CMake argument `-DGKFS_ENABLE_CLIENT_METRICS=ON` (see 
  environment.

- `LIBGKFS_PROXY_PID_FILE` - Path to the proxy pid file (when using the GekkoFS proxy).

- `LIBGKFS_NUM_REPL` - Number of replicas for data.

- Mutation/redistribution is rejected while `LIBGKFS_NUM_REPL` is nonzero until

  replica-aware placement and recovery are implemented.

- `LIBGKFS_REPAIR_STATUS_PATH` - Optional path for client-local repair counters and queue status.

- `LIBGKFS_REPAIR_JOURNAL_PATH` - Optional path for client-local pending repair tasks. The journal is

  single-owner: do not share it between client processes. Its `.lock` sidecar records the owning host,

docs/sphinx/users/running.md

+3 −0
Changes for docs/sphinx/users/running.md: 3 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -368,6 +368,9 @@ The user can enable the data replication feature by setting the replication envi 
The number of replicas should go from `0` to the `number of servers - 1`. The replication environment variable can be

set up for each client independently.



Mutation and redistribution are currently rejected while replication is enabled;

replica-aware placement during topology changes is not yet supported.



For client-local repair recovery, set `LIBGKFS_REPAIR_JOURNAL_PATH` to a writable

journal path. One client process owns a journal at a time. The adjacent `.lock`

sidecar records the owner and a fingerprint of the configured hosts file; do not

include/client/rpc/replica.hpp

+5 −0
Changes for include/client/rpc/replica.hpp: 5 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -150,6 +150,11 @@ mutation_supported_with_replicas(const int replica_count) { 
    return replica_count == 0;

}



inline int

mutation_error_with_replicas(const int replica_count) {

    return mutation_supported_with_replicas(replica_count) ? 0 : ENOTSUP;

}



inline void

validate_replica_count(const int replica_count, const std::size_t host_count) {

    if(replica_count < 0) {

src/client/malleability.cpp

+14 −2
Changes for src/client/malleability.cpp: 14 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -54,13 +54,15 @@ int 
mutate_start(int old_server_conf, int new_server_conf,

             const std::string& new_hosts_file) {

    LOG(INFO, "{}() Mutate operation enter", __func__);

    if(!gkfs::rpc::mutation_supported_with_replicas(CTX->get_replicas())) {

    if(const auto error =

               gkfs::rpc::mutation_error_with_replicas(CTX->get_replicas());

       error != 0) {

        LOG(ERROR,

            "{}() mutation is not supported when LIBGKFS_NUM_REPL={} is enabled",

            __func__, CTX->get_replicas());

        cerr << "ERR: Mutation is not supported with replication enabled"

             << endl;

        return ENOTSUP;

        return error;

    }

    // sanity checks

    auto hf = std::getenv("LIBGKFS_HOSTS_FILE");
@@ -160,6 +162,16 @@ mutate_status_detailed() { 
int

mutate_finalize() {

    LOG(INFO, "{}() enter", __func__);

    if(const auto error =

               gkfs::rpc::mutation_error_with_replicas(CTX->get_replicas());

       error != 0) {

        LOG(ERROR,

            "{}() mutation finalize is not supported when LIBGKFS_NUM_REPL={} is enabled",

            __func__, CTX->get_replicas());

        cerr << "ERR: Mutation is not supported with replication enabled"

             << endl;

        return error;

    }

    auto res = gkfs::malleable::rpc::forward_mutate_finalize();

    const auto* hf = std::getenv("LIBGKFS_HOSTS_FILE");

    if(res == 0 && hf != nullptr) {

tests/unit/test_replica_placement.cpp

+7 −0
Changes for tests/unit/test_replica_placement.cpp: 7 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -275,6 +275,13 @@ TEST_CASE("mutation is supported only without replicas", "[replication]") { 
    REQUIRE_FALSE(gkfs::rpc::mutation_supported_with_replicas(2));

}



TEST_CASE("mutation returns ENOTSUP when replicas are enabled",

          "[replication]") {

    REQUIRE(gkfs::rpc::mutation_error_with_replicas(0) == 0);

    REQUIRE(gkfs::rpc::mutation_error_with_replicas(1) == ENOTSUP);

    REQUIRE(gkfs::rpc::mutation_error_with_replicas(2) == ENOTSUP);

}



TEST_CASE("primary_required accepts primary success with degraded replicas",

          "[replication]") {

    const auto result = gkfs::rpc::classify_replica_operation(