From 738e58893470a435791e3d2d1d48197d39a4bc52 Mon Sep 17 00:00:00 2001 From: neverchanje Date: Sun, 28 Feb 2021 14:46:46 +0800 Subject: [PATCH 01/13] refactor: move primary's learning preparation of cache into another function --- src/replica/replica.h | 7 ++ src/replica/replica_learn.cpp | 141 +++++++++++++++++++--------------- 2 files changed, 85 insertions(+), 63 deletions(-) diff --git a/src/replica/replica.h b/src/replica/replica.h index 5886f7ee6b..4b206afb46 100644 --- a/src/replica/replica.h +++ b/src/replica/replica.h @@ -287,6 +287,13 @@ class replica : public serverlet, public ref_counter, public replica_ba void notify_learn_completion(); error_code apply_learned_state_from_private_log(learn_state &state); + bool prepare_cached_learn_state(const learn_request &request, + decree learn_start_decree, + decree local_committed_decree, + remote_learner_state &learner_state, + learn_response &response, + bool &delayed_replay_prepare_list); + // Gets the position where this round of the learning process should begin. // This method is called on primary-side. // TODO(wutao1): mark it const diff --git a/src/replica/replica_learn.cpp b/src/replica/replica_learn.cpp index ca8c79f544..c21513e779 100644 --- a/src/replica/replica_learn.cpp +++ b/src/replica/replica_learn.cpp @@ -437,62 +437,14 @@ void replica::on_learn(dsn::message_ex *msg, const learn_request &request) response.last_committed_decree = local_committed_decree; response.err = ERR_OK; - // set prepare_start_decree when to-be-learn state is covered by prepare list, - // note min_decree can be NOT present in prepare list when list.count == 0 - if (learn_start_decree > _prepare_list->min_decree() || - (learn_start_decree == _prepare_list->min_decree() && _prepare_list->count() > 0)) { - if (learner_state.prepare_start_decree == invalid_decree) { - // start from (last_committed_decree + 1) - learner_state.prepare_start_decree = local_committed_decree + 1; - - cleanup_preparing_mutations(false); - - // the replayed prepare msg needs to be AFTER the learning response msg - // to reduce probability that preparing messages arrive remote early than - // learning response msg. - delayed_replay_prepare_list = true; - - ddebug("%s: on_learn[%016" PRIx64 - "]: learner = %s, set prepare_start_decree = %" PRId64, - name(), - request.signature, - request.learner.to_string(), - local_committed_decree + 1); - } - - response.prepare_start_decree = learner_state.prepare_start_decree; - } else { - learner_state.prepare_start_decree = invalid_decree; - } - - // only learn mutation cache in range of [learn_start_decree, prepare_start_decree), - // in this case, the state on the PS should be contiguous (+ to-be-sent prepare list) - if (response.prepare_start_decree != invalid_decree) { - binary_writer writer; - int count = 0; - for (decree d = learn_start_decree; d < response.prepare_start_decree; d++) { - auto mu = _prepare_list->get_mutation_by_decree(d); - dassert(mu != nullptr, "mutation must not be nullptr, decree = %" PRId64 "", d); - mu->write_to(writer, nullptr); - count++; - } - response.type = learn_type::LT_CACHE; - response.state.meta = writer.get_buffer(); - ddebug("%s: on_learn[%016" PRIx64 "]: learner = %s, learn mutation cache succeed, " - "learn_start_decree = %" PRId64 ", prepare_start_decree = %" PRId64 ", " - "learn_mutation_count = %d, learn_data_size = %d", - name(), - request.signature, - request.learner.to_string(), - learn_start_decree, - response.prepare_start_decree, - count, - response.state.meta.length()); - } - // learn delta state or checkpoint // in this case, the state on the PS is still incomplete - else { + if (!prepare_cached_learn_state(request, + learn_start_decree, + local_committed_decree, + learner_state, + response, + delayed_replay_prepare_list)) { if (learn_start_decree > _app->last_durable_decree()) { ddebug("%s: on_learn[%016" PRIx64 "]: learner = %s, choose to learn private logs, " "because learn_start_decree(%" PRId64 ") > _app->last_durable_decree(%" PRId64 @@ -574,15 +526,15 @@ void replica::on_learn(dsn::message_ex *msg, const learn_request &request) err.to_string()); } else { response.base_local_dir = _app->data_dir(); - ddebug( - "%s: on_learn[%016" PRIx64 "]: learner = %s, get app learn state succeed, " - "learned_meta_size = %u, learned_file_count = %u, learned_to_decree = %" PRId64, - name(), - request.signature, - request.learner.to_string(), - response.state.meta.length(), - static_cast(response.state.files.size()), - response.state.to_decree_included); + ddebug("%s: on_learn[%016" PRIx64 "]: learner = %s, get app learn state succeed, " + "learned_meta_size = %u, learned_file_count = %u, learned_to_decree = " + "%" PRId64, + name(), + request.signature, + request.learner.to_string(), + response.state.meta.length(), + static_cast(response.state.files.size()), + response.state.to_decree_included); } } } @@ -1004,6 +956,69 @@ void replica::on_learn_reply(error_code err, learn_request &&req, learn_response } } +bool replica::prepare_learn_from_cache(const learn_request &request, + decree learn_start_decree, + decree local_committed_decree, + remote_learner_state &learner_state, + learn_response &response, + bool &delayed_replay_prepare_list) +{ + // set prepare_start_decree when to-be-learn state is covered by prepare list, + // note min_decree can be NOT present in prepare list when list.count == 0 + if (learn_start_decree > _prepare_list->min_decree() || + (learn_start_decree == _prepare_list->min_decree() && _prepare_list->count() > 0)) { + if (learner_state.prepare_start_decree == invalid_decree) { + // start from (last_committed_decree + 1) + learner_state.prepare_start_decree = local_committed_decree + 1; + + cleanup_preparing_mutations(false); + + // the replayed prepare msg needs to be AFTER the learning response msg + // to reduce probability that preparing messages arrive remote early than + // learning response msg. + delayed_replay_prepare_list = true; + + ddebug("%s: on_learn[%016" PRIx64 + "]: learner = %s, set prepare_start_decree = %" PRId64, + name(), + request.signature, + request.learner.to_string(), + local_committed_decree + 1); + } + + response.prepare_start_decree = learner_state.prepare_start_decree; + } else { + learner_state.prepare_start_decree = invalid_decree; + } + + // only learn mutation cache in range of [learn_start_decree, prepare_start_decree), + // in this case, the state on the PS should be contiguous (+ to-be-sent prepare list) + if (response.prepare_start_decree != invalid_decree) { + binary_writer writer; + int count = 0; + for (decree d = learn_start_decree; d < response.prepare_start_decree; d++) { + auto mu = _prepare_list->get_mutation_by_decree(d); + dassert(mu != nullptr, "mutation must not be nullptr, decree = %" PRId64 "", d); + mu->write_to(writer, nullptr); + count++; + } + response.type = learn_type::LT_CACHE; + response.state.meta = writer.get_buffer(); + ddebug("%s: on_learn[%016" PRIx64 "]: learner = %s, learn mutation cache succeed, " + "learn_start_decree = %" PRId64 ", prepare_start_decree = %" PRId64 ", " + "learn_mutation_count = %d, learn_data_size = %d", + name(), + request.signature, + request.learner.to_string(), + learn_start_decree, + response.prepare_start_decree, + count, + response.state.meta.length()); + return true; + } + return false; +} + void replica::on_copy_remote_state_completed(error_code err, size_t size, uint64_t copy_start_time, From d3937efece34c49b762754a65a8f208d9abe33b2 Mon Sep 17 00:00:00 2001 From: neverchanje Date: Sun, 28 Feb 2021 15:07:29 +0800 Subject: [PATCH 02/13] fix --- src/replica/replica.h | 6 +++--- src/replica/replica_learn.cpp | 12 ++++++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/replica/replica.h b/src/replica/replica.h index 4b206afb46..40fc2df195 100644 --- a/src/replica/replica.h +++ b/src/replica/replica.h @@ -290,9 +290,9 @@ class replica : public serverlet, public ref_counter, public replica_ba bool prepare_cached_learn_state(const learn_request &request, decree learn_start_decree, decree local_committed_decree, - remote_learner_state &learner_state, - learn_response &response, - bool &delayed_replay_prepare_list); + /*out*/ remote_learner_state &learner_state, + /*out*/ learn_response &response, + /*out*/ bool &delayed_replay_prepare_list); // Gets the position where this round of the learning process should begin. // This method is called on primary-side. diff --git a/src/replica/replica_learn.cpp b/src/replica/replica_learn.cpp index c21513e779..572079918f 100644 --- a/src/replica/replica_learn.cpp +++ b/src/replica/replica_learn.cpp @@ -956,12 +956,12 @@ void replica::on_learn_reply(error_code err, learn_request &&req, learn_response } } -bool replica::prepare_learn_from_cache(const learn_request &request, - decree learn_start_decree, - decree local_committed_decree, - remote_learner_state &learner_state, - learn_response &response, - bool &delayed_replay_prepare_list) +bool replica::prepare_cached_learn_state(const learn_request &request, + decree learn_start_decree, + decree local_committed_decree, + /*out*/ remote_learner_state &learner_state, + /*out*/ learn_response &response, + /*out*/ bool &delayed_replay_prepare_list) { // set prepare_start_decree when to-be-learn state is covered by prepare list, // note min_decree can be NOT present in prepare list when list.count == 0 From 180ad32a9682a3440aa78224238da9394b74694f Mon Sep 17 00:00:00 2001 From: neverchanje Date: Sun, 28 Feb 2021 15:19:22 +0800 Subject: [PATCH 03/13] add comment --- src/replica/replica.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/replica/replica.h b/src/replica/replica.h index 40fc2df195..f79d14fcdd 100644 --- a/src/replica/replica.h +++ b/src/replica/replica.h @@ -287,6 +287,8 @@ class replica : public serverlet, public ref_counter, public replica_ba void notify_learn_completion(); error_code apply_learned_state_from_private_log(learn_state &state); + // Prepares in-memory mutations for the replica's learning. + // Returns false if there's no delta data in cache (aka prepare-list). bool prepare_cached_learn_state(const learn_request &request, decree learn_start_decree, decree local_committed_decree, From 59de52d824dd48f899077f5b5dd3b1b67aff1420 Mon Sep 17 00:00:00 2001 From: neverchanje Date: Sun, 28 Feb 2021 16:08:59 +0800 Subject: [PATCH 04/13] fix github workflow --- .github/workflows/pull_request.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pull_request.yaml b/.github/workflows/pull_request.yaml index 4636b0edee..ffefa524c8 100644 --- a/.github/workflows/pull_request.yaml +++ b/.github/workflows/pull_request.yaml @@ -37,7 +37,7 @@ jobs: needs: lint runs-on: self-hosted container: - image: apachepegasus/ci-env + image: ghcr.io/pegasus-kv/thirdparties-bin:ubuntu1804 env: CCACHE_DIR: /tmp/ccache/pegasus CCACHE_MAXSIZE: 10G @@ -46,13 +46,16 @@ jobs: - /tmp/ccache/pegasus:/tmp/ccache/pegasus # Read docs at https://docs.docker.com/storage/tmpfs/ for more details of using tmpfs in docker. options: --mount type=tmpfs,destination=/tmp/pegasus,tmpfs-size=10737418240 --cap-add=SYS_PTRACE + defaults: + run: + shell: bash steps: - uses: actions/checkout@v2 with: fetch-depth: 1 - name: Unpack prebuilt third-parties if: contains(github.event.pull_request.labels.*.name, 'thirdparty-modified') == false - run: unzip /root/pegasus-thirdparty-output.zip -d ./thirdparty + run: unzip /root/thirdparties-bin.zip -d ./rdsn/thirdparty - name: Rebuild third-parties if: contains(github.event.pull_request.labels.*.name, 'thirdparty-modified') working-directory: thirdparty From 9c4a51c2fc345dd367af5770998140f2fc88c288 Mon Sep 17 00:00:00 2001 From: Wu Tao Date: Sun, 28 Feb 2021 21:22:29 +0800 Subject: [PATCH 05/13] update dockerhub image store --- .github/workflows/pull_request.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull_request.yaml b/.github/workflows/pull_request.yaml index ffefa524c8..538ee34650 100644 --- a/.github/workflows/pull_request.yaml +++ b/.github/workflows/pull_request.yaml @@ -37,7 +37,7 @@ jobs: needs: lint runs-on: self-hosted container: - image: ghcr.io/pegasus-kv/thirdparties-bin:ubuntu1804 + image: registry.cn-beijing.aliyuncs.com/apachepegasus/thirdparties-bin:ubuntu1804 env: CCACHE_DIR: /tmp/ccache/pegasus CCACHE_MAXSIZE: 10G From 02c4a76234d3ad15b92c7eed5777f05d3ca51e88 Mon Sep 17 00:00:00 2001 From: Wu Tao Date: Sun, 28 Feb 2021 21:45:35 +0800 Subject: [PATCH 06/13] acclerate rdsn clone on self-hosted runners --- .github/workflows/pull_request.yaml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pull_request.yaml b/.github/workflows/pull_request.yaml index 538ee34650..f1768ac2b1 100644 --- a/.github/workflows/pull_request.yaml +++ b/.github/workflows/pull_request.yaml @@ -49,10 +49,12 @@ jobs: defaults: run: shell: bash + working-directory: /root/rdsn steps: - - uses: actions/checkout@v2 - with: - fetch-depth: 1 + - name: Clone rdsn source + working-directory: /root + run: | + git clone --depth=1 https://hub.fastgit.org/XiaoMi/rdsn.git - name: Unpack prebuilt third-parties if: contains(github.event.pull_request.labels.*.name, 'thirdparty-modified') == false run: unzip /root/thirdparties-bin.zip -d ./rdsn/thirdparty From 7ca8b12b2c8ed14678d3a7895f67a6e0b7dd527f Mon Sep 17 00:00:00 2001 From: Wu Tao Date: Sun, 28 Feb 2021 21:56:15 +0800 Subject: [PATCH 07/13] fix pull request workflow --- .github/workflows/pull_request.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull_request.yaml b/.github/workflows/pull_request.yaml index f1768ac2b1..ef0dd1947b 100644 --- a/.github/workflows/pull_request.yaml +++ b/.github/workflows/pull_request.yaml @@ -57,7 +57,7 @@ jobs: git clone --depth=1 https://hub.fastgit.org/XiaoMi/rdsn.git - name: Unpack prebuilt third-parties if: contains(github.event.pull_request.labels.*.name, 'thirdparty-modified') == false - run: unzip /root/thirdparties-bin.zip -d ./rdsn/thirdparty + run: unzip /root/thirdparties-bin.zip -d ./thirdparty - name: Rebuild third-parties if: contains(github.event.pull_request.labels.*.name, 'thirdparty-modified') working-directory: thirdparty From d99ab5325332e8bcd2a1732e22b2fd8ddd0f60f5 Mon Sep 17 00:00:00 2001 From: Wu Tao Date: Mon, 1 Mar 2021 09:29:03 +0800 Subject: [PATCH 08/13] sets no limit on tmpfs --- .github/workflows/pull_request.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pull_request.yaml b/.github/workflows/pull_request.yaml index ef0dd1947b..bbf1f46ee0 100644 --- a/.github/workflows/pull_request.yaml +++ b/.github/workflows/pull_request.yaml @@ -45,7 +45,7 @@ jobs: # Place ccache compilation intermediate results in host memory, that's shared among containers. - /tmp/ccache/pegasus:/tmp/ccache/pegasus # Read docs at https://docs.docker.com/storage/tmpfs/ for more details of using tmpfs in docker. - options: --mount type=tmpfs,destination=/tmp/pegasus,tmpfs-size=10737418240 --cap-add=SYS_PTRACE + options: --mount type=tmpfs,destination=/tmp/pegasus --cap-add=SYS_PTRACE defaults: run: shell: bash From 2a623b6611b007d1bd8ab8ed471373f8e76926a7 Mon Sep 17 00:00:00 2001 From: Wu Tao Date: Mon, 1 Mar 2021 10:28:23 +0800 Subject: [PATCH 09/13] Update pull_request.yaml --- .github/workflows/pull_request.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pull_request.yaml b/.github/workflows/pull_request.yaml index bbf1f46ee0..e729437185 100644 --- a/.github/workflows/pull_request.yaml +++ b/.github/workflows/pull_request.yaml @@ -68,4 +68,6 @@ jobs: - name: Compilation run: ./run.sh build -c --skip_thirdparty - name: Unit Testing - run: ./run.sh test --skip_thirdparty + run: | + export LD_LIBRARY_PATH=/root/rdsn/thirdparty/output/lib:/usr/lib/jvm/java-8-openjdk-amd64/jre/lib/amd64/server + ./run.sh test --skip_thirdparty From 83c1e2d7d08d710793a57918366201f7b192bbdd Mon Sep 17 00:00:00 2001 From: neverchanje Date: Mon, 1 Mar 2021 13:37:47 +0800 Subject: [PATCH 10/13] fix --- src/replica/replica_learn.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/replica/replica_learn.cpp b/src/replica/replica_learn.cpp index 572079918f..2dbfc0a606 100644 --- a/src/replica/replica_learn.cpp +++ b/src/replica/replica_learn.cpp @@ -526,15 +526,15 @@ void replica::on_learn(dsn::message_ex *msg, const learn_request &request) err.to_string()); } else { response.base_local_dir = _app->data_dir(); - ddebug("%s: on_learn[%016" PRIx64 "]: learner = %s, get app learn state succeed, " - "learned_meta_size = %u, learned_file_count = %u, learned_to_decree = " - "%" PRId64, - name(), - request.signature, - request.learner.to_string(), - response.state.meta.length(), - static_cast(response.state.files.size()), - response.state.to_decree_included); + ddebug( + "%s: on_learn[%016" PRIx64 "]: learner = %s, get app learn state succeed, " + "learned_meta_size = %u, learned_file_count = %u, learned_to_decree = %" PRId64, + name(), + request.signature, + request.learner.to_string(), + response.state.meta.length(), + static_cast(response.state.files.size()), + response.state.to_decree_included); } } } From a353de9158dc90a438c78dd076eb298f6f846daf Mon Sep 17 00:00:00 2001 From: neverchanje Date: Mon, 1 Mar 2021 13:45:58 +0800 Subject: [PATCH 11/13] make code more clear --- src/replica/replica_learn.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/replica/replica_learn.cpp b/src/replica/replica_learn.cpp index 2dbfc0a606..aab0131211 100644 --- a/src/replica/replica_learn.cpp +++ b/src/replica/replica_learn.cpp @@ -437,14 +437,15 @@ void replica::on_learn(dsn::message_ex *msg, const learn_request &request) response.last_committed_decree = local_committed_decree; response.err = ERR_OK; + bool should_learn_cache = prepare_cached_learn_state(request, + learn_start_decree, + local_committed_decree, + learner_state, + response, + delayed_replay_prepare_list); // learn delta state or checkpoint // in this case, the state on the PS is still incomplete - if (!prepare_cached_learn_state(request, - learn_start_decree, - local_committed_decree, - learner_state, - response, - delayed_replay_prepare_list)) { + if (!should_learn_cache) { if (learn_start_decree > _app->last_durable_decree()) { ddebug("%s: on_learn[%016" PRIx64 "]: learner = %s, choose to learn private logs, " "because learn_start_decree(%" PRId64 ") > _app->last_durable_decree(%" PRId64 From 3c3891892a94313da344cb8680ab9229cd9441b7 Mon Sep 17 00:00:00 2001 From: neverchanje Date: Mon, 1 Mar 2021 13:47:28 +0800 Subject: [PATCH 12/13] make code more clear --- src/replica/replica_learn.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/replica/replica_learn.cpp b/src/replica/replica_learn.cpp index aab0131211..4a5f5459fc 100644 --- a/src/replica/replica_learn.cpp +++ b/src/replica/replica_learn.cpp @@ -437,14 +437,14 @@ void replica::on_learn(dsn::message_ex *msg, const learn_request &request) response.last_committed_decree = local_committed_decree; response.err = ERR_OK; + // learn delta state or checkpoint + // in this case, the state on the PS is still incomplete bool should_learn_cache = prepare_cached_learn_state(request, learn_start_decree, local_committed_decree, learner_state, response, delayed_replay_prepare_list); - // learn delta state or checkpoint - // in this case, the state on the PS is still incomplete if (!should_learn_cache) { if (learn_start_decree > _app->last_durable_decree()) { ddebug("%s: on_learn[%016" PRIx64 "]: learner = %s, choose to learn private logs, " From 0111d514b9f7c84ff619565826078e3b95560946 Mon Sep 17 00:00:00 2001 From: neverchanje Date: Mon, 1 Mar 2021 18:22:57 +0800 Subject: [PATCH 13/13] fix comment --- src/replica/replica_learn.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/replica/replica_learn.cpp b/src/replica/replica_learn.cpp index 4a5f5459fc..23dceb27d5 100644 --- a/src/replica/replica_learn.cpp +++ b/src/replica/replica_learn.cpp @@ -438,7 +438,6 @@ void replica::on_learn(dsn::message_ex *msg, const learn_request &request) response.err = ERR_OK; // learn delta state or checkpoint - // in this case, the state on the PS is still incomplete bool should_learn_cache = prepare_cached_learn_state(request, learn_start_decree, local_committed_decree,