github-actions[bot] commented on code in PR #26567:
URL: https://github.com/apache/doris/pull/26567#discussion_r1386093154


##########
be/src/util/ref_count_closure.h:
##########
@@ -54,4 +54,88 @@ class RefCountClosure : public google::protobuf::Closure {
     std::atomic<int> _refs;
 };
 
+template <typename Response>
+class DummyBrpcCallback {
+    ENABLE_FACTORY_CREATOR(DummyBrpcCallback);
+
+public:
+    using ResponseType = Response;
+    DummyBrpcCallback() {
+        cntl_ = std::make_shared<brpc::Controller>();
+        response_ = std::make_shared<Response>();
+    }
+
+public:

Review Comment:
   warning: redundant access specifier has the same accessibility as the 
previous access specifier [readability-redundant-access-specifiers]
   
   ```suggestion
   
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/util/ref_count_closure.h:61:** previously declared here
   ```cpp
   public:
   ^
   ```
   
   </details>
   



##########
be/src/util/ref_count_closure.h:
##########
@@ -54,4 +54,88 @@
     std::atomic<int> _refs;
 };
 
+template <typename Response>
+class DummyBrpcCallback {
+    ENABLE_FACTORY_CREATOR(DummyBrpcCallback);
+
+public:
+    using ResponseType = Response;
+    DummyBrpcCallback() {
+        cntl_ = std::make_shared<brpc::Controller>();
+        response_ = std::make_shared<Response>();
+    }
+
+public:
+    void call() {}
+
+    void join() { brpc::Join(cntl_->call_id()); }
+
+public:
+    // controller has to be the same lifecycle with the closure, because brpc 
may use
+    // it in any stage of the rpc.
+    std::shared_ptr<brpc::Controller> cntl_;
+    // We do not know if brpc will use request or response after brpc method 
returns.
+    // So that we need keep a shared ptr here to ensure that brpc could use 
req/rep
+    // at any stage.
+    std::shared_ptr<Response> response_;
+};
+
+// The closure will be deleted after callback.
+// It could only be created by using shared ptr or unique ptr.
+// It will hold a weak ptr of T and call run of T
+// Callback() {
+//  xxxx;
+//  public
+//  void run() {
+//      logxxx
+//  }
+//  }
+//
+//  std::shared_ptr<Callback> b;
+//
+//  std::unique_ptr<AutoReleaseClosure> a(b);
+//  brpc_call(a.release());
+
+template <typename Request, typename Callback>
+class AutoReleaseClosure : public google::protobuf::Closure {
+    using Weak = typename std::shared_ptr<Callback>::weak_type;
+    using ResponseType = typename Callback::ResponseType;
+    ENABLE_FACTORY_CREATOR(AutoReleaseClosure);
+
+public:
+    AutoReleaseClosure(std::shared_ptr<Request> req, std::shared_ptr<Callback> 
callback)
+            : callback_(callback) {
+        this->cntl_ = callback->cntl_;
+        this->response_ = callback->response_;
+    }
+
+    ~AutoReleaseClosure() override = default;
+
+    //  Will delete itself
+    void Run() override {
+        SCOPED_TRACK_MEMORY_TO_UNKNOWN();
+        Defer defer {[&]() { delete this; }};
+        // If lock failed, it means the callback object is deconstructed, then 
no need
+        // to deal with the callback any more.
+        if (auto tmp = callback_.lock()) {
+            tmp->call();
+        }
+    }
+
+public:

Review Comment:
   warning: redundant access specifier has the same accessibility as the 
previous access specifier [readability-redundant-access-specifiers]
   
   ```suggestion
   
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/util/ref_count_closure.h:104:** previously declared here
   ```cpp
   public:
   ^
   ```
   
   </details>
   



##########
be/src/util/ref_count_closure.h:
##########
@@ -54,4 +54,88 @@
     std::atomic<int> _refs;
 };
 
+template <typename Response>
+class DummyBrpcCallback {
+    ENABLE_FACTORY_CREATOR(DummyBrpcCallback);
+
+public:
+    using ResponseType = Response;
+    DummyBrpcCallback() {
+        cntl_ = std::make_shared<brpc::Controller>();
+        response_ = std::make_shared<Response>();
+    }
+
+public:
+    void call() {}
+
+    void join() { brpc::Join(cntl_->call_id()); }
+
+public:

Review Comment:
   warning: redundant access specifier has the same accessibility as the 
previous access specifier [readability-redundant-access-specifiers]
   
   ```suggestion
   
   ```
   <details>
   <summary>Additional context</summary>
   
   **be/src/util/ref_count_closure.h:61:** previously declared here
   ```cpp
   public:
   ^
   ```
   
   </details>
   



-- 
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: commits-unsubscr...@doris.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to