From 0cd5c394564bce31a2b6345f47c1a6b66b0ff186 Mon Sep 17 00:00:00 2001 From: Pavel Kartavyy Date: Thu, 26 Jun 2014 15:35:12 +0400 Subject: [PATCH 1/6] zk: added retry to exists [#METR-11718] --- libs/libzkutil/src/ZooKeeper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/libzkutil/src/ZooKeeper.cpp b/libs/libzkutil/src/ZooKeeper.cpp index a66a3c42019..98a5e956636 100644 --- a/libs/libzkutil/src/ZooKeeper.cpp +++ b/libs/libzkutil/src/ZooKeeper.cpp @@ -262,7 +262,7 @@ int32_t ZooKeeper::existsImpl(const std::string & path, Stat * stat_, WatchFutur bool ZooKeeper::exists(const std::string & path, Stat * stat_, WatchFuture * watch) { - int32_t code = existsImpl(path, stat_, watch); + int32_t code = retry(boost::bind(&ZooKeeper::existsImpl, this, path, stat_, watch)); if (!( code == ZOK || code == ZNONODE)) From 0eedb5ee84dab3f22fe592853b34d72428ee2188 Mon Sep 17 00:00:00 2001 From: Pavel Kartavyy Date: Thu, 26 Jun 2014 23:08:48 +0400 Subject: [PATCH 2/6] zkutil: changed exception policy [#METR-10969] --- libs/libzkutil/include/zkutil/ZooKeeper.h | 18 +++++++++++++---- libs/libzkutil/src/ZooKeeper.cpp | 24 +++++++++++++++-------- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/libs/libzkutil/include/zkutil/ZooKeeper.h b/libs/libzkutil/include/zkutil/ZooKeeper.h index 89aaa7af5b4..15b6691e2a7 100644 --- a/libs/libzkutil/include/zkutil/ZooKeeper.h +++ b/libs/libzkutil/include/zkutil/ZooKeeper.h @@ -17,8 +17,12 @@ typedef WatchWithPromise * WatchWithPromisePtr; /** Сессия в ZooKeeper. Интерфейс существенно отличается от обычного API ZooKeeper. * Вместо callback-ов для watch-ей используются std::future. - * Методы с названиями, не начинающимися с try, бросают исключение при любой ошибке кроме OperationTimeout. - * При OperationTimeout пытаемся попробоватть еще retry_num раз. + * + * Методы на чтение при восстанавливаемых ошибках OperationTimeout, ConnectionLoss пытаются еще retry_num раз. + * Методы на запись не пытаются повторить при восстанавливаемых ошибках, т.к. это приводит к проблеммам типа удаления дважды одного и того же. + * + * Методы с названиями, не начинающимися с try, бросают исключение при любой ошибке. + * * Методы с названиями, начинающимися с try, не бросают исключение только при перечисленных видах ошибок. * Например, исключение бросается в любом случае, если сессия разорвалась или если не хватает прав или ресурсов. */ @@ -75,6 +79,8 @@ public: * - Нет родителя создаваемой ноды. * - Родитель эфемерный. * - Такая нода уже есть. + * - ZCONNECTIONLOSS + * - ZOPERATIONTIMEOUT * При остальных ошибках бросает исключение. */ int32_t tryCreate(const std::string & path, const std::string & data, int32_t mode, std::string & pathCreated); @@ -88,6 +94,8 @@ public: * - Такой ноды нет. * - У ноды другая версия. * - У ноды есть дети. + * - ZCONNECTIONLOSS + * - ZOPERATIONTIMEOUT */ int32_t tryRemove(const std::string & path, int32_t version = -1); @@ -106,6 +114,8 @@ public: /** Не бросает исключение при следующих ошибках: * - Такой ноды нет. * - У ноды другая версия. + * - ZCONNECTIONLOSS + * - ZOPERATIONTIMEOUT */ int32_t trySet(const std::string & path, const std::string & data, int32_t version = -1, Stat * stat = nullptr); @@ -153,7 +163,7 @@ private: int32_t retry(const T & operation) { int32_t code = operation(); - for (size_t i = 0; (i < retry_num) && (code == ZOPERATIONTIMEOUT); ++i) + for (size_t i = 0; (i < retry_num) && (code == ZOPERATIONTIMEOUT || code == ZCONNECTIONLOSS); ++i) { code = operation(); } @@ -182,7 +192,7 @@ private: WatchFunction * state_watch; std::unordered_set watch_store; - /// Количество попыток повторить операцию при OperationTimeout + /// Количество попыток повторить операцию чтения при OperationTimeout, ConnectionLoss size_t retry_num = 3; }; diff --git a/libs/libzkutil/src/ZooKeeper.cpp b/libs/libzkutil/src/ZooKeeper.cpp index 98a5e956636..6f9cdfa204b 100644 --- a/libs/libzkutil/src/ZooKeeper.cpp +++ b/libs/libzkutil/src/ZooKeeper.cpp @@ -203,12 +203,14 @@ std::string ZooKeeper::create(const std::string & path, const std::string & data int32_t ZooKeeper::tryCreate(const std::string & path, const std::string & data, int32_t mode, std::string & pathCreated) { - int code = retry(boost::bind(&ZooKeeper::createImpl, this, boost::ref(path), boost::ref(data), mode, boost::ref(pathCreated))); + int code = createImpl(path, data, mode, pathCreated); if (!( code == ZOK || code == ZNONODE || code == ZNODEEXISTS || - code == ZNOCHILDRENFOREPHEMERALS)) + code == ZNOCHILDRENFOREPHEMERALS || + code == ZCONNECTIONLOSS || + code == ZOPERATIONTIMEOUT)) throw KeeperException(code, path); return code; @@ -233,11 +235,13 @@ void ZooKeeper::remove(const std::string & path, int32_t version) int32_t ZooKeeper::tryRemove(const std::string & path, int32_t version) { - int32_t code = retry(boost::bind(&ZooKeeper::removeImpl, this, boost::ref(path), version)); + int32_t code = removeImpl(path, version); if (!( code == ZOK || code == ZNONODE || code == ZBADVERSION || - code == ZNOTEMPTY)) + code == ZNOTEMPTY || + code == ZCONNECTIONLOSS || + code == ZOPERATIONTIMEOUT)) throw KeeperException(code, path); return code; } @@ -337,11 +341,13 @@ void ZooKeeper::set(const std::string & path, const std::string & data, int32_t int32_t ZooKeeper::trySet(const std::string & path, const std::string & data, int32_t version, Stat * stat_) { - int32_t code = retry(boost::bind(&ZooKeeper::setImpl, this, boost::ref(path), boost::ref(data), version, stat_)); + int32_t code = setImpl(path, data, version, stat_); if (!( code == ZOK || code == ZNONODE || - code == ZBADVERSION)) + code == ZBADVERSION || + code == ZCONNECTIONLOSS || + code == ZOPERATIONTIMEOUT)) throw KeeperException(code, path); return code; } @@ -377,14 +383,16 @@ OpResultsPtr ZooKeeper::multi(const Ops & ops) int32_t ZooKeeper::tryMulti(const Ops & ops_, OpResultsPtr * out_results_) { - int32_t code = retry(boost::bind(&ZooKeeper::multiImpl, this, boost::ref(ops_), out_results_)); + int32_t code = multiImpl(ops_, out_results_); if (code != ZOK && code != ZNONODE && code != ZNODEEXISTS && code != ZNOCHILDRENFOREPHEMERALS && code != ZBADVERSION && - code != ZNOTEMPTY) + code != ZNOTEMPTY && + code != ZCONNECTIONLOSS && + code != ZOPERATIONTIMEOUT) throw KeeperException(code); return code; } From 9feab97b19874dc82ea75b2e153703ca2d454396 Mon Sep 17 00:00:00 2001 From: Pavel Kartavyy Date: Fri, 27 Jun 2014 21:52:50 +0400 Subject: [PATCH 3/6] =?UTF-8?q?=E2=96=88=E2=96=88=E2=96=88=E2=96=88?= =?UTF-8?q?=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88=E2=96=88?= =?UTF-8?q?:=20write=20recount=20requests=20robustly=20[#METR-11718]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- libs/libzkutil/include/zkutil/ZooKeeper.h | 7 +++++++ libs/libzkutil/src/ZooKeeper.cpp | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/libs/libzkutil/include/zkutil/ZooKeeper.h b/libs/libzkutil/include/zkutil/ZooKeeper.h index 15b6691e2a7..049df3db597 100644 --- a/libs/libzkutil/include/zkutil/ZooKeeper.h +++ b/libs/libzkutil/include/zkutil/ZooKeeper.h @@ -9,6 +9,7 @@ namespace zkutil { const UInt32 DEFAULT_SESSION_TIMEOUT = 30000; +const UInt32 DEFAULT_RETRY_NUM = 3; struct WatchWithPromise; @@ -86,6 +87,12 @@ public: int32_t tryCreate(const std::string & path, const std::string & data, int32_t mode, std::string & pathCreated); int32_t tryCreate(const std::string & path, const std::string & data, int32_t mode); + /** создает Persistent ноду. + * Игнорирует, если нода уже создана. + * Пытается сделать retry при ConnectionLoss или OperationTimeout + */ + void createIfNotExists(const std::string & path, const std::string & data); + /** Удалить ноду, если ее версия равна version (если -1, подойдет любая версия). */ void remove(const std::string & path, int32_t version = -1); diff --git a/libs/libzkutil/src/ZooKeeper.cpp b/libs/libzkutil/src/ZooKeeper.cpp index 6f9cdfa204b..187c4a48a8f 100644 --- a/libs/libzkutil/src/ZooKeeper.cpp +++ b/libs/libzkutil/src/ZooKeeper.cpp @@ -222,6 +222,27 @@ int32_t ZooKeeper::tryCreate(const std::string & path, const std::string & data, return tryCreate(path, data, mode, pathCreated); } +void ZooKeeper::createIfNotExists(const std::string & path, const std::string & data) +{ + int32_t code = tryCreate(path, "", zkutil::CreateMode::Persistent); + + if (code == ZOK || code == ZNODEEXISTS) + return; + + if (!(code == ZOPERATIONTIMEOUT || code == ZCONNECTIONLOSS)) + throw KeeperException(code, path); + + for (size_t attempt = 0; attempt < retry_num && (code == ZOPERATIONTIMEOUT || code == ZCONNECTIONLOSS); ++attempt) + { + code = tryCreate(path, "", zkutil::CreateMode::Persistent); + }; + + if (code == ZOK || code == ZNODEEXISTS) + return; + else + throw KeeperException(code, path); +} + int32_t ZooKeeper::removeImpl(const std::string & path, int32_t version) { int32_t code = zoo_delete(impl, path.c_str(), version); From 93b9844f0379fd02c7d6e95af5ae83417fbd21d5 Mon Sep 17 00:00:00 2001 From: Pavel Kartavyy Date: Mon, 30 Jun 2014 18:21:39 +0400 Subject: [PATCH 4/6] CommonChunkProcessor: avoid duplication of written data if removing chunks from ZK failed [#METR-11178] --- libs/libzkutil/include/zkutil/ZooKeeper.h | 2 ++ libs/libzkutil/src/ZooKeeper.cpp | 12 +++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/libs/libzkutil/include/zkutil/ZooKeeper.h b/libs/libzkutil/include/zkutil/ZooKeeper.h index 049df3db597..f56b4602227 100644 --- a/libs/libzkutil/include/zkutil/ZooKeeper.h +++ b/libs/libzkutil/include/zkutil/ZooKeeper.h @@ -145,6 +145,8 @@ public: /** Бросает исключение только если какая-нибудь операция вернула "неожиданную" ошибку - такую ошибку, * увидев которую соответствующий метод try* бросил бы исключение. */ int32_t tryMulti(const Ops & ops, OpResultsPtr * out_results = nullptr); + /** Использовать только для методов на чтение */ + int32_t tryMultiWithRetries(const Ops & ops, OpResultsPtr * out_results = nullptr); /** Удаляет ноду вместе с поддеревом. Если в это время кто-то добавит иили удалит ноду в поддереве, результат не определен. diff --git a/libs/libzkutil/src/ZooKeeper.cpp b/libs/libzkutil/src/ZooKeeper.cpp index 187c4a48a8f..0528e5d6591 100644 --- a/libs/libzkutil/src/ZooKeeper.cpp +++ b/libs/libzkutil/src/ZooKeeper.cpp @@ -386,11 +386,8 @@ int32_t ZooKeeper::multiImpl(const Ops & ops_, OpResultsPtr * out_results_) int32_t code = zoo_multi(impl, ops.size(), ops.data(), out_results->data()); - if (code == ZOK) - { - if (out_results_) - *out_results_ = out_results; - } + if (out_results_) + *out_results_ = out_results; return code; } @@ -418,6 +415,11 @@ int32_t ZooKeeper::tryMulti(const Ops & ops_, OpResultsPtr * out_results_) return code; } +int32_t ZooKeeper::tryMultiWithRetries(const Ops & ops, OpResultsPtr * out_results) +{ + return retry(boost::bind(&ZooKeeper::tryMulti, this, boost::ref(ops), out_results)); +} + void ZooKeeper::removeChildrenRecursive(const std::string & path) { Strings children = getChildren(path); From 499420f2cdc0fc92fa34e15eb2989c01beb1359c Mon Sep 17 00:00:00 2001 From: Pavel Kartavyy Date: Mon, 30 Jun 2014 22:12:18 +0400 Subject: [PATCH 5/6] improvements [#METR-11178] --- libs/libzkutil/src/ZooKeeper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/libzkutil/src/ZooKeeper.cpp b/libs/libzkutil/src/ZooKeeper.cpp index 0528e5d6591..5231b7d8e44 100644 --- a/libs/libzkutil/src/ZooKeeper.cpp +++ b/libs/libzkutil/src/ZooKeeper.cpp @@ -70,6 +70,7 @@ void ZooKeeper::processPromise(zhandle_t * zh, int type, int state, const char * void ZooKeeper::init(const std::string & hosts_, int32_t sessionTimeoutMs_, WatchFunction * watch_) { + zoo_set_debug_level(ZOO_LOG_LEVEL_ERROR); hosts = hosts_; sessionTimeoutMs = sessionTimeoutMs_; state_watch = watch_; @@ -87,7 +88,6 @@ void ZooKeeper::init(const std::string & hosts_, int32_t sessionTimeoutMs_, Watc ZooKeeper::ZooKeeper(const std::string & hosts, int32_t sessionTimeoutMs, WatchFunction * watch_) { - zoo_set_debug_level(ZOO_LOG_LEVEL_ERROR); init(hosts, sessionTimeoutMs, watch_); } From b2f029dc55c31848891e6960e212223f226af846 Mon Sep 17 00:00:00 2001 From: Pavel Kartavyy Date: Tue, 1 Jul 2014 17:40:07 +0400 Subject: [PATCH 6/6] libzkutil: wait when ConnectionLoss happened [#METR-11178] --- libs/libzkutil/include/zkutil/ZooKeeper.h | 7 +++++++ libs/libzkutil/src/ZooKeeper.cpp | 1 + 2 files changed, 8 insertions(+) diff --git a/libs/libzkutil/include/zkutil/ZooKeeper.h b/libs/libzkutil/include/zkutil/ZooKeeper.h index f56b4602227..87fda5d908e 100644 --- a/libs/libzkutil/include/zkutil/ZooKeeper.h +++ b/libs/libzkutil/include/zkutil/ZooKeeper.h @@ -3,6 +3,7 @@ #include #include #include +#include namespace zkutil @@ -174,6 +175,11 @@ private: int32_t code = operation(); for (size_t i = 0; (i < retry_num) && (code == ZOPERATIONTIMEOUT || code == ZCONNECTIONLOSS); ++i) { + /// если потеряно соединение подождем timeout/3, авось восстановится + if (code == ZCONNECTIONLOSS) + usleep(sessionTimeoutMs*1000/3); + + LOG_WARNING(log, "Error happened " << error2string(code) << ". Retry"); code = operation(); } return code; @@ -203,6 +209,7 @@ private: /// Количество попыток повторить операцию чтения при OperationTimeout, ConnectionLoss size_t retry_num = 3; + Logger * log = nullptr; }; typedef ZooKeeper::Ptr ZooKeeperPtr; diff --git a/libs/libzkutil/src/ZooKeeper.cpp b/libs/libzkutil/src/ZooKeeper.cpp index 5231b7d8e44..d4e96224b39 100644 --- a/libs/libzkutil/src/ZooKeeper.cpp +++ b/libs/libzkutil/src/ZooKeeper.cpp @@ -70,6 +70,7 @@ void ZooKeeper::processPromise(zhandle_t * zh, int type, int state, const char * void ZooKeeper::init(const std::string & hosts_, int32_t sessionTimeoutMs_, WatchFunction * watch_) { + log = &Logger::get("ZooKeeper"); zoo_set_debug_level(ZOO_LOG_LEVEL_ERROR); hosts = hosts_; sessionTimeoutMs = sessionTimeoutMs_;