[GitHub] [incubator-pegasus] empiredan commented on a diff in pull request #1453: fix(ddl_client): after receiving ERR_BUSY_CREATING or ERR_BUSY_DROPPING sleep for a while before retry

["empiredan (via GitHub)" <gi...@apache.org>]

empiredan commented on code in PR #1453:
URL: https://github.com/apache/incubator-pegasus/pull/1453#discussion_r1169961465


##########
src/client/replication_ddl_client.cpp:
##########
@@ -1434,23 +1446,83 @@ bool replication_ddl_client::valid_app_char(int c)
     return (bool)std::isalnum(c) || c == '_' || c == '.' || c == ':';
 }
 
+namespace {
+
+bool is_busy(const dsn::error_code &err)
+{
+    return err == dsn::ERR_BUSY_CREATING || err == dsn::ERR_BUSY_DROPPING;
+}
+
+bool need_retry(uint32_t attempt_count, uint32_t busy_attempt_count, const dsn::error_code &err)
+{
+    if (err == dsn::ERR_OK) {
+        return false;
+    }
+
+    if (attempt_count < FLAGS_ddl_client_max_attempt_count) {
+        return true;
+    }
+
+    if (!is_busy(err)) {
+        return false;
+    }
+
+    return busy_attempt_count < FLAGS_ddl_client_max_attempt_count;
+}
+
+bool should_sleep_before_retry(const dsn::error_code &err, uint32_t &sleep_ms)
+{
+    if (is_busy(err)) {
+        sleep_ms = FLAGS_ddl_client_busy_retry_interval_ms;
+        return true;
+    }
+
+    return false;
+}
+
+} // anonymous namespace
+
 void replication_ddl_client::end_meta_request(const rpc_response_task_ptr &callback,
-                                              int retry_times,
-                                              error_code err,
+                                              uint32_t attempt_count,
+                                              uint32_t busy_attempt_count,
+                                              const error_code &err,
                                               dsn::message_ex *request,
                                               dsn::message_ex *resp)
 {
-    if (err != dsn::ERR_OK && retry_times < 2) {
-        rpc::call(_meta_server,
-                  request,
-                  &_tracker,
-                  [this, retry_times, callback](
-                      error_code err, dsn::message_ex *request, dsn::message_ex *response) mutable {
-                      end_meta_request(callback, retry_times + 1, err, request, response);
-                  });
-    } else {
-        callback->enqueue(err, (message_ex *)resp);
+    ++attempt_count;
+    if (is_busy(err)) {
+        ++busy_attempt_count;
     }
+
+    LOG_INFO("attempt {}: attempt_count={}, busy_attempt_count={}, max_replica_count={}, err={}",
+             request->local_rpc_code,
+             attempt_count,
+             busy_attempt_count,
+             FLAGS_ddl_client_max_attempt_count,
+             err);
+
+    if (!need_retry(attempt_count, busy_attempt_count, err)) {
+        callback->enqueue(err, (message_ex *)resp);
+        return;
+    }
+
+    uint32_t sleep_ms = 0;

Review Comment:
   OK, I'll use `FLAGS_ddl_client_busy_retry_interval_ms` instead.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org