From 56ca93ca3f5325bd5712e9589d6ee84492b6103c Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Wed, 3 Aug 2022 14:14:57 +0000 Subject: [PATCH 1/5] Block memory tracker in Keeper during commit --- contrib/NuRaft | 2 +- src/Coordination/KeeperServer.cpp | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/contrib/NuRaft b/contrib/NuRaft index 2ef198694e1..d73e12adf75 160000 --- a/contrib/NuRaft +++ b/contrib/NuRaft @@ -1 +1 @@ -Subproject commit 2ef198694e10c86175ee6ead389346d199060437 +Subproject commit d73e12adf7557a0ebca7f7ecde68c064dee22fa0 diff --git a/src/Coordination/KeeperServer.cpp b/src/Coordination/KeeperServer.cpp index 7238c86cc50..a7c9b0836f1 100644 --- a/src/Coordination/KeeperServer.cpp +++ b/src/Coordination/KeeperServer.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -173,6 +174,12 @@ struct KeeperServer::KeeperRaftServer : public nuraft::raft_server reconfigure(new_config); } + void commit_in_bg() override + { + MemoryTrackerBlockerInThread blocker; + nuraft::raft_server::commit_in_bg(); + } + using nuraft::raft_server::raft_server; // peers are initially marked as responding because at least one cycle From fa98338ce158e4a714a7440853cf22985db4b036 Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Thu, 4 Aug 2022 07:13:42 +0000 Subject: [PATCH 2/5] Add comment for memory tracker blocking --- src/Coordination/KeeperServer.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Coordination/KeeperServer.cpp b/src/Coordination/KeeperServer.cpp index a7c9b0836f1..6e017e36919 100644 --- a/src/Coordination/KeeperServer.cpp +++ b/src/Coordination/KeeperServer.cpp @@ -176,6 +176,11 @@ struct KeeperServer::KeeperRaftServer : public nuraft::raft_server void commit_in_bg() override { + // For NuRaft, if any commit fails (uncaught exception) the whole server aborts as a safety + // This includes failed allocation which can produce an unknown state for the storage, + // making it impossible to handle correctly. + // We block the memory tracker for all the commit operations (including KeeperStateMachine::commit) + // assuming that the allocations are small MemoryTrackerBlockerInThread blocker; nuraft::raft_server::commit_in_bg(); } From 278921be3b57d6a2ce74af03bad44a6e346398f0 Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Thu, 4 Aug 2022 07:25:28 +0000 Subject: [PATCH 3/5] Fix style --- src/Coordination/KeeperServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Coordination/KeeperServer.cpp b/src/Coordination/KeeperServer.cpp index 6e017e36919..82a7a54401d 100644 --- a/src/Coordination/KeeperServer.cpp +++ b/src/Coordination/KeeperServer.cpp @@ -179,7 +179,7 @@ struct KeeperServer::KeeperRaftServer : public nuraft::raft_server // For NuRaft, if any commit fails (uncaught exception) the whole server aborts as a safety // This includes failed allocation which can produce an unknown state for the storage, // making it impossible to handle correctly. - // We block the memory tracker for all the commit operations (including KeeperStateMachine::commit) + // We block the memory tracker for all the commit operations (including KeeperStateMachine::commit) // assuming that the allocations are small MemoryTrackerBlockerInThread blocker; nuraft::raft_server::commit_in_bg(); From 0a7d2e7b8ab4e45e18b1cf95eca008109f7dcbae Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Thu, 4 Aug 2022 13:03:05 +0000 Subject: [PATCH 4/5] Use LockMemoryExceptionThread --- src/Coordination/KeeperServer.cpp | 4 ++-- src/Coordination/KeeperStorage.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Coordination/KeeperServer.cpp b/src/Coordination/KeeperServer.cpp index 82a7a54401d..24551f27ec2 100644 --- a/src/Coordination/KeeperServer.cpp +++ b/src/Coordination/KeeperServer.cpp @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include #include @@ -181,7 +181,7 @@ struct KeeperServer::KeeperRaftServer : public nuraft::raft_server // making it impossible to handle correctly. // We block the memory tracker for all the commit operations (including KeeperStateMachine::commit) // assuming that the allocations are small - MemoryTrackerBlockerInThread blocker; + LockMemoryExceptionInThread blocker{VariableContext::Global}; nuraft::raft_server::commit_in_bg(); } diff --git a/src/Coordination/KeeperStorage.cpp b/src/Coordination/KeeperStorage.cpp index bbc647fd951..693a1b16f0d 100644 --- a/src/Coordination/KeeperStorage.cpp +++ b/src/Coordination/KeeperStorage.cpp @@ -14,7 +14,7 @@ #include #include #include -#include +#include #include #include #include @@ -2127,7 +2127,7 @@ void KeeperStorage::rollbackRequest(int64_t rollback_zxid, bool allow_missing) // if an exception occurs during rollback, the best option is to terminate because we can end up in an inconsistent state // we block memory tracking so we can avoid terminating if we're rollbacking because of memory limit - MemoryTrackerBlockerInThread temporarily_ignore_any_memory_limits; + LockMemoryExceptionInThread blocker{VariableContext::Global}; try { uncommitted_transactions.pop_back(); From f63e4ba261b4ba3ddbb03f3ff591507eb6590d1a Mon Sep 17 00:00:00 2001 From: Antonio Andelic Date: Thu, 4 Aug 2022 13:14:57 +0000 Subject: [PATCH 5/5] Update NuRaft --- contrib/NuRaft | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/NuRaft b/contrib/NuRaft index d73e12adf75..1b0af760b35 160000 --- a/contrib/NuRaft +++ b/contrib/NuRaft @@ -1 +1 @@ -Subproject commit d73e12adf7557a0ebca7f7ecde68c064dee22fa0 +Subproject commit 1b0af760b3506b8e35b50cb7df098cbad5064ff2