vagetablechicken commented on a change in pull request #3143: Non blocking
OlapTableSink
URL: https://github.com/apache/incubator-doris/pull/3143#discussion_r395492007
##########
File path: be/src/exec/tablet_sink.h
##########
@@ -47,18 +48,101 @@ class ExprContext;
class TExpr;
namespace stream_load {
-
+
class OlapTableSink;
+// The counter of add_batch rpc of a single node
+struct AddBatchCounter {
+ // total execution time of a add_batch rpc
+ int64_t add_batch_execution_time_us = 0;
+ // lock waiting time in a add_batch rpc
+ int64_t add_batch_wait_lock_time_us = 0;
+ // number of add_batch call
+ int64_t add_batch_num = 0;
+ AddBatchCounter& operator+=(const AddBatchCounter& rhs) {
+ add_batch_execution_time_us += rhs.add_batch_execution_time_us;
+ add_batch_wait_lock_time_us += rhs.add_batch_wait_lock_time_us;
+ add_batch_num += rhs.add_batch_num;
+ return *this;
+ }
+ friend AddBatchCounter operator+(const AddBatchCounter& lhs, const
AddBatchCounter& rhs) {
+ AddBatchCounter sum = lhs;
+ sum += rhs;
+ return sum;
+ }
+};
+
+template <typename T>
+class ReusableClosure : public google::protobuf::Closure {
Review comment:
Actually, I want the NodeChannel to be the owner of add_batch_closure.
https://github.com/apache/incubator-doris/pull/3143/files#diff-4007834d7219c2282c8ae5c454f01dbaR47-R51
In NodeChannel's dtor `_add_batch_closure->release();`, release() will do
brpc::Join(). So RPC won't call Run() after channel is destructed.
If OlapTableSink is closed normally, `_add_batch_closure->release();` has no
effect.
But as you mentioned about unnecessary waiting RPC finished , if
OlapTableSink is closed with error, `_add_batch_closure->release();` in
NodeChannel dtor may take some time to wait add_batch repsonse, although the
add_batch response is useless under the circumstance that we cancelled the
whole sink.
I've considered about brpc::StartCancel(call_id), but I'm not good enough at
understanding brpc, so I wrote a comment
[here](https://github.com/apache/incubator-doris/pull/3143/files#diff-4007834d7219c2282c8ae5c454f01dbaR249).
And just let the channel wait for the useless RPC response.
Maybe we should do brpc::StartCancel(call_id)?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
