You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2022/06/21 06:51:03 UTC

[GitHub] [doris] weizuo93 opened a new pull request, #10298: [Enhancement] Support single replica load

weizuo93 opened a new pull request, #10298:
URL: https://github.com/apache/doris/pull/10298

   # Proposed changes
   
   Issue Number: close #9847 
   
   ## Problem Summary:
   
   During load process, the same operation are performed on all replicas such as sort and aggregation, which are resource-intensive. Concurrent data load would consume much CPU and memory resources. It's better to perform write process (writing data into MemTable and then data flush) on single replica and synchronize data files to other replicas before transaction finished.
   
   ## Checklist(Required)
   
   1. Does it affect the original behavior: No
   2. Has unit tests been added: No
   3. Has document been added or modified: No
   4. Does it need to update dependencies: No
   5. Are there any changes that cannot be rolled back:No
   
   


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] morningman merged pull request #10298: [feature-wip](load) Support single replica load

Posted by GitBox <gi...@apache.org>.
morningman merged PR #10298:
URL: https://github.com/apache/doris/pull/10298


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] github-actions[bot] commented on pull request #10298: [feature-wip](load) Support single replica load

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10298:
URL: https://github.com/apache/doris/pull/10298#issuecomment-1201274366

   PR approved by anyone and no changes requested.


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923424151


##########
gensrc/proto/internal_service.proto:
##########
@@ -469,6 +492,32 @@ message PResetRPCChannelResponse {
 
 message PEmptyRequest {};
 
+message PTabletWriteSlaveRequest {
+    required RowsetMetaPB rowset_meta = 1;

Review Comment:
   > use optional for all fields. Same to other newly added structs.
   
   done



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] morningman commented on pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
morningman commented on PR #10298:
URL: https://github.com/apache/doris/pull/10298#issuecomment-1192140363

   As we discussed earlier, this feature may need to continue to be modified.
   So I suggest the following:
   1. Add a function switch configuration on the BE side, which is disabled by default. And if it is disabled, all related services and thread pools are not started (because the new port is likely to cause the original service to fail to start due to port conflict)
   2. Combine the 3 function switch configurations of FE into one (too many configurations..)


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [feature-wip](load) Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r934572269


##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -511,6 +513,9 @@ public class SessionVariable implements Serializable, Writable {
     @VariableMgr.VarAttr(name = SESSION_CONTEXT, needForward = true)
     public String sessionContext = "";
 
+    @VariableMgr.VarAttr(name = ENABLE_SINGLE_REPLICA_INSERT)

Review Comment:
   > 
   
   done



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923435779


##########
be/src/common/config.h:
##########
@@ -35,6 +35,11 @@ CONF_Int32(brpc_port, "8060");
 // the number of bthreads for brpc, the default value is set to -1, which means the number of bthreads is #cpu-cores
 CONF_Int32(brpc_num_threads, "-1");
 
+// port to brpc server for single replica load
+CONF_Int32(single_replica_load_brpc_port, "8070");

Review Comment:
   > The notification rpc is a light weight rpc. Do we really need to create a new brpc port for this?
   
   Based on the test results, notification rpc will have an effect on RPC for data distribution when data load concurrency is high .



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] morningman commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r913369254


##########
gensrc/proto/internal_service.proto:
##########
@@ -469,6 +492,32 @@ message PResetRPCChannelResponse {
 
 message PEmptyRequest {};
 
+message PTabletWriteSlaveRequest {
+    required RowsetMetaPB rowset_meta = 1;

Review Comment:
   use optional for all fields.
   Same to other newly added structs.



##########
be/src/exec/tablet_sink.h:
##########
@@ -287,6 +291,7 @@ class NodeChannel {
     RefCountClosure<PTabletWriterOpenResult>* _open_closure = nullptr;
 
     std::vector<TTabletWithPartition> _all_tablets;
+    std::unordered_map<int64_t, std::vector<int64_t>> _slave_tablet_nodes;

Review Comment:
   Add comment to explain these `int64_t`



##########
fe/fe-core/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -722,6 +722,24 @@ public class Config extends ConfigBase {
     @ConfField(mutable = true, masterOnly = true)
     public static boolean disable_show_stream_load = false;
 
+    /**
+     * Whether to enable to write single replica for stream load.
+     */
+    @ConfField(mutable = true, masterOnly = true)
+    public static boolean enable_single_replica_stream_load = false;
+
+    /**
+     * Whether to enable to write single replica for broker load.
+     */
+    @ConfField(mutable = true, masterOnly = true)
+    public static boolean enable_single_replica_broker_load = false;
+
+    /**
+     * Whether to enable to write single replica for insert.
+     */
+    @ConfField(mutable = true, masterOnly = true)
+    public static boolean enable_single_replica_insert = false;

Review Comment:
   better to use a load property for stream and broker load.
   and use session variable to control the insert operation.



##########
be/src/common/config.h:
##########
@@ -35,6 +35,11 @@ CONF_Int32(brpc_port, "8060");
 // the number of bthreads for brpc, the default value is set to -1, which means the number of bthreads is #cpu-cores
 CONF_Int32(brpc_num_threads, "-1");
 
+// port to brpc server for single replica load
+CONF_Int32(single_replica_load_brpc_port, "8070");

Review Comment:
   The notification rpc is a light weight rpc. Do we really need to create a new brpc port for this?



##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -102,7 +111,7 @@ Status TabletsChannel::close(int sender_id, int64_t backend_id, bool* finished,
         _state = kFinished;
         // All senders are closed
         // 1. close all delta writers
-        std::vector<DeltaWriter*> need_wait_writers;
+        std::set<DeltaWriter*> need_wait_writers;

Review Comment:
   Why change to std::set?



##########
be/src/olap/delta_writer.h:
##########
@@ -67,7 +67,13 @@ class DeltaWriter {
     Status close();
     // wait for all memtables to be flushed.
     // mem_consumption() should be 0 after this function returns.
-    Status close_wait();
+    Status close_wait(PSlaveTabletNodes slave_tablet_nodes, const bool write_single_replica);

Review Comment:
   ```suggestion
       Status close_wait(const PSlaveTabletNodes& slave_tablet_nodes, bool write_single_replica);
   ```



##########
be/src/exec/tablet_sink.h:
##########
@@ -174,6 +174,10 @@ class NodeChannel {
 
     virtual Status init(RuntimeState* state);
 
+    void add_slave_tablet_nodes(int64_t tablet_id, std::vector<int64_t> slave_nodes) {

Review Comment:
   ```suggestion
       void add_slave_tablet_nodes(int64_t tablet_id, const std::vector<int64_t>& slave_nodes) {
   ```



##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -124,20 +133,46 @@ Status TabletsChannel::close(int sender_id, int64_t backend_id, bool* finished,
             }
         }
 
+        _write_single_replica = write_single_replica;
+
         // 2. wait delta writers and build the tablet vector
         for (auto writer : need_wait_writers) {
+            PSlaveTabletNodes slave_nodes;
+            if (write_single_replica) {
+                slave_nodes = slave_tablet_nodes.at(writer->tablet_id());
+            }
             // close may return failed, but no need to handle it here.
             // tablet_vec will only contains success tablet, and then let FE judge it.
-            _close_wait(writer, tablet_vec, tablet_errors);
+            _close_wait(writer, tablet_vec, tablet_errors, slave_nodes, write_single_replica);
+        }
+
+        if (write_single_replica) {
+            CountDownLatch latch(1);
+            while (need_wait_writers.size() > 0 &&
+                   (time(nullptr) - parent->last_updated_time()) < (parent->timeout() * 0.9)) {
+                for (auto writer : need_wait_writers) {
+                    bool is_done = writer->check_slave_replicas_done(success_slave_tablet_node_ids);
+                    if (is_done) {
+                        need_wait_writers.erase(writer);

Review Comment:
   Is it safe to erase a map's element when it is being iterated?



##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -124,20 +133,46 @@ Status TabletsChannel::close(int sender_id, int64_t backend_id, bool* finished,
             }
         }
 
+        _write_single_replica = write_single_replica;
+
         // 2. wait delta writers and build the tablet vector
         for (auto writer : need_wait_writers) {
+            PSlaveTabletNodes slave_nodes;
+            if (write_single_replica) {
+                slave_nodes = slave_tablet_nodes.at(writer->tablet_id());
+            }
             // close may return failed, but no need to handle it here.
             // tablet_vec will only contains success tablet, and then let FE judge it.
-            _close_wait(writer, tablet_vec, tablet_errors);
+            _close_wait(writer, tablet_vec, tablet_errors, slave_nodes, write_single_replica);
+        }
+
+        if (write_single_replica) {
+            CountDownLatch latch(1);
+            while (need_wait_writers.size() > 0 &&
+                   (time(nullptr) - parent->last_updated_time()) < (parent->timeout() * 0.9)) {

Review Comment:
   Add comment to explain why it multiply 0.9.



##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -124,20 +133,46 @@ Status TabletsChannel::close(int sender_id, int64_t backend_id, bool* finished,
             }
         }
 
+        _write_single_replica = write_single_replica;
+
         // 2. wait delta writers and build the tablet vector
         for (auto writer : need_wait_writers) {
+            PSlaveTabletNodes slave_nodes;
+            if (write_single_replica) {
+                slave_nodes = slave_tablet_nodes.at(writer->tablet_id());
+            }
             // close may return failed, but no need to handle it here.
             // tablet_vec will only contains success tablet, and then let FE judge it.
-            _close_wait(writer, tablet_vec, tablet_errors);
+            _close_wait(writer, tablet_vec, tablet_errors, slave_nodes, write_single_replica);
+        }
+
+        if (write_single_replica) {
+            CountDownLatch latch(1);
+            while (need_wait_writers.size() > 0 &&
+                   (time(nullptr) - parent->last_updated_time()) < (parent->timeout() * 0.9)) {
+                for (auto writer : need_wait_writers) {
+                    bool is_done = writer->check_slave_replicas_done(success_slave_tablet_node_ids);
+                    if (is_done) {
+                        need_wait_writers.erase(writer);
+                    }
+                }
+                latch.wait_for(std::chrono::milliseconds(100));

Review Comment:
   Why using a CountDownLatch here?
   I think we can just call `sleep()`?



##########
be/src/service/internal_service.h:
##########
@@ -175,9 +183,13 @@ class PInternalServiceImpl : public PBackendService {
                                   PTabletWriterAddBlockResult* response,
                                   google::protobuf::Closure* done);
 
+    void _response_pull_slave_rowset(std::string remote_host, int64_t brpc_port, int64_t txn_id,

Review Comment:
   ```suggestion
       void _response_pull_slave_rowset(const std::string& remote_host, int64_t brpc_port, int64_t txn_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.

To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923424425


##########
fe/fe-core/src/main/java/org/apache/doris/common/Config.java:
##########
@@ -722,6 +722,24 @@ public class Config extends ConfigBase {
     @ConfField(mutable = true, masterOnly = true)
     public static boolean disable_show_stream_load = false;
 
+    /**
+     * Whether to enable to write single replica for stream load.
+     */
+    @ConfField(mutable = true, masterOnly = true)
+    public static boolean enable_single_replica_stream_load = false;
+
+    /**
+     * Whether to enable to write single replica for broker load.
+     */
+    @ConfField(mutable = true, masterOnly = true)
+    public static boolean enable_single_replica_broker_load = false;
+
+    /**
+     * Whether to enable to write single replica for insert.
+     */
+    @ConfField(mutable = true, masterOnly = true)
+    public static boolean enable_single_replica_insert = false;

Review Comment:
   > better to use a load property for stream and broker load. and use session variable to control the insert operation.
   
   done



##########
be/src/exec/tablet_sink.h:
##########
@@ -287,6 +291,7 @@ class NodeChannel {
     RefCountClosure<PTabletWriterOpenResult>* _open_closure = nullptr;
 
     std::vector<TTabletWithPartition> _all_tablets;
+    std::unordered_map<int64_t, std::vector<int64_t>> _slave_tablet_nodes;

Review Comment:
   > Add comment to explain these `int64_t`
   
   done



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923425768


##########
be/src/olap/delta_writer.h:
##########
@@ -67,7 +67,13 @@ class DeltaWriter {
     Status close();
     // wait for all memtables to be flushed.
     // mem_consumption() should be 0 after this function returns.
-    Status close_wait();
+    Status close_wait(PSlaveTabletNodes slave_tablet_nodes, const bool write_single_replica);

Review Comment:
   done



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923425106


##########
be/src/exec/tablet_sink.h:
##########
@@ -174,6 +174,10 @@ class NodeChannel {
 
     virtual Status init(RuntimeState* state);
 
+    void add_slave_tablet_nodes(int64_t tablet_id, std::vector<int64_t> slave_nodes) {

Review Comment:
   done



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923426238


##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -124,20 +133,46 @@ Status TabletsChannel::close(int sender_id, int64_t backend_id, bool* finished,
             }
         }
 
+        _write_single_replica = write_single_replica;
+
         // 2. wait delta writers and build the tablet vector
         for (auto writer : need_wait_writers) {
+            PSlaveTabletNodes slave_nodes;
+            if (write_single_replica) {
+                slave_nodes = slave_tablet_nodes.at(writer->tablet_id());
+            }
             // close may return failed, but no need to handle it here.
             // tablet_vec will only contains success tablet, and then let FE judge it.
-            _close_wait(writer, tablet_vec, tablet_errors);
+            _close_wait(writer, tablet_vec, tablet_errors, slave_nodes, write_single_replica);
+        }
+
+        if (write_single_replica) {
+            CountDownLatch latch(1);
+            while (need_wait_writers.size() > 0 &&
+                   (time(nullptr) - parent->last_updated_time()) < (parent->timeout() * 0.9)) {

Review Comment:
   > Add comment to explain why it multiply 0.9.
   
   done



##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -124,20 +133,46 @@ Status TabletsChannel::close(int sender_id, int64_t backend_id, bool* finished,
             }
         }
 
+        _write_single_replica = write_single_replica;
+
         // 2. wait delta writers and build the tablet vector
         for (auto writer : need_wait_writers) {
+            PSlaveTabletNodes slave_nodes;
+            if (write_single_replica) {
+                slave_nodes = slave_tablet_nodes.at(writer->tablet_id());
+            }
             // close may return failed, but no need to handle it here.
             // tablet_vec will only contains success tablet, and then let FE judge it.
-            _close_wait(writer, tablet_vec, tablet_errors);
+            _close_wait(writer, tablet_vec, tablet_errors, slave_nodes, write_single_replica);
+        }
+
+        if (write_single_replica) {
+            CountDownLatch latch(1);
+            while (need_wait_writers.size() > 0 &&
+                   (time(nullptr) - parent->last_updated_time()) < (parent->timeout() * 0.9)) {
+                for (auto writer : need_wait_writers) {
+                    bool is_done = writer->check_slave_replicas_done(success_slave_tablet_node_ids);
+                    if (is_done) {
+                        need_wait_writers.erase(writer);
+                    }
+                }
+                latch.wait_for(std::chrono::milliseconds(100));

Review Comment:
   > Why using a CountDownLatch here? I think we can just call `sleep()`?
   
   done



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] morningman commented on pull request #10298: [Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
morningman commented on PR #10298:
URL: https://github.com/apache/doris/pull/10298#issuecomment-1174603965

   https://cwiki.apache.org/confluence/display/DORIS/DSIP-015%3A+Support+single+replica+load+for+load


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] caiconghui commented on pull request #10298: [Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
caiconghui commented on PR #10298:
URL: https://github.com/apache/doris/pull/10298#issuecomment-1161377990

   wonderful contribution!


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923435779


##########
be/src/common/config.h:
##########
@@ -35,6 +35,11 @@ CONF_Int32(brpc_port, "8060");
 // the number of bthreads for brpc, the default value is set to -1, which means the number of bthreads is #cpu-cores
 CONF_Int32(brpc_num_threads, "-1");
 
+// port to brpc server for single replica load
+CONF_Int32(single_replica_load_brpc_port, "8070");

Review Comment:
   > The notification rpc is a light weight rpc. Do we really need to create a new brpc port for this?
   
   When data load concurrency is high, notification rpc will affect RPC for data distribution.



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on pull request #10298: [feature-wip](load) Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on PR #10298:
URL: https://github.com/apache/doris/pull/10298#issuecomment-1201102748

   > As we discussed earlier, this feature may need to continue to be modified. So I suggest the following:
   > 
   > 1. Add a function switch configuration on the BE side, which is disabled by default. And if it is disabled, all related services and thread pools are not started (because the new port is likely to cause the original service to fail to start due to port conflict)
   > 2. Combine the 3 function switch configurations of FE into one (too many configurations..)
   > 3. And if BE is disabled, it should return an error to tell user.
   
   done.


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] github-actions[bot] commented on pull request #10298: [feature-wip](load) Support single replica load

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10298:
URL: https://github.com/apache/doris/pull/10298#issuecomment-1201976270

   PR approved by at least one committer and no changes requested.


-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923426751


##########
be/src/service/internal_service.h:
##########
@@ -175,9 +183,13 @@ class PInternalServiceImpl : public PBackendService {
                                   PTabletWriterAddBlockResult* response,
                                   google::protobuf::Closure* done);
 
+    void _response_pull_slave_rowset(std::string remote_host, int64_t brpc_port, int64_t txn_id,

Review Comment:
   done



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923433526


##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -124,20 +133,46 @@ Status TabletsChannel::close(int sender_id, int64_t backend_id, bool* finished,
             }
         }
 
+        _write_single_replica = write_single_replica;
+
         // 2. wait delta writers and build the tablet vector
         for (auto writer : need_wait_writers) {
+            PSlaveTabletNodes slave_nodes;
+            if (write_single_replica) {
+                slave_nodes = slave_tablet_nodes.at(writer->tablet_id());
+            }
             // close may return failed, but no need to handle it here.
             // tablet_vec will only contains success tablet, and then let FE judge it.
-            _close_wait(writer, tablet_vec, tablet_errors);
+            _close_wait(writer, tablet_vec, tablet_errors, slave_nodes, write_single_replica);
+        }
+
+        if (write_single_replica) {
+            CountDownLatch latch(1);
+            while (need_wait_writers.size() > 0 &&
+                   (time(nullptr) - parent->last_updated_time()) < (parent->timeout() * 0.9)) {
+                for (auto writer : need_wait_writers) {
+                    bool is_done = writer->check_slave_replicas_done(success_slave_tablet_node_ids);
+                    if (is_done) {
+                        need_wait_writers.erase(writer);

Review Comment:
   > Is it safe to erase a map's element when it is being iterated?
   
   done



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] weizuo93 commented on a diff in pull request #10298: [Load][Enhancement] Support single replica load

Posted by GitBox <gi...@apache.org>.
weizuo93 commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r923433112


##########
be/src/runtime/tablets_channel.cpp:
##########
@@ -102,7 +111,7 @@ Status TabletsChannel::close(int sender_id, int64_t backend_id, bool* finished,
         _state = kFinished;
         // All senders are closed
         // 1. close all delta writers
-        std::vector<DeltaWriter*> need_wait_writers;
+        std::set<DeltaWriter*> need_wait_writers;

Review Comment:
   > Why change to std::set?
   
   Element erase operation will follow and  std::set is more efficient than std::vector to execute `erase`.



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] morningman commented on a diff in pull request #10298: [feature-wip](load) Support single replica load

Posted by GitBox <gi...@apache.org>.
morningman commented on code in PR #10298:
URL: https://github.com/apache/doris/pull/10298#discussion_r934561831


##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -511,6 +513,9 @@ public class SessionVariable implements Serializable, Writable {
     @VariableMgr.VarAttr(name = SESSION_CONTEXT, needForward = true)
     public String sessionContext = "";
 
+    @VariableMgr.VarAttr(name = ENABLE_SINGLE_REPLICA_INSERT)

Review Comment:
   ```suggestion
       @VariableMgr.VarAttr(name = ENABLE_SINGLE_REPLICA_INSERT, needForward = true)
   ```



-- 
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-unsubscribe@doris.apache.org

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


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


[GitHub] [doris] github-actions[bot] commented on pull request #10298: [feature-wip](load) Support single replica load

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #10298:
URL: https://github.com/apache/doris/pull/10298#issuecomment-1201274325

   PR approved by at least one committer and no changes requested.


-- 
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-unsubscribe@doris.apache.org

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


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