Copilot commented on code in PR #63722:
URL: https://github.com/apache/doris/pull/63722#discussion_r3308955731
##########
be/src/load/group_commit/group_commit_mgr.cpp:
##########
@@ -199,7 +203,7 @@ bool LoadBlockQueue::contain_load_id(const UniqueId&
load_id) {
Status LoadBlockQueue::add_load_id(const UniqueId& load_id,
const std::shared_ptr<Dependency>
put_block_dep) {
std::unique_lock l(mutex);
- if (_need_commit) {
+ if (_need_commit.load() || !status.ok() || process_finish.load()) {
return Status::InternalError<false>("block queue is set need commit,
id=" +
load_instance_id.to_string());
Review Comment:
The error message always says the queue is "set need commit", but this
branch also triggers when the queue status is already non-OK or when
process_finish is true. Updating the message to reflect the actual conditions
(and/or including the current flags/status) would make troubleshooting much
easier.
##########
be/src/common/config.cpp:
##########
@@ -1420,6 +1420,9 @@ DEFINE_mInt32(group_commit_queue_mem_limit, "67108864");
// group_commit_wal_max_disk_limit=1024 or group_commit_wal_max_disk_limit=10%
can be automatically identified.
DEFINE_String(group_commit_wal_max_disk_limit, "10%");
DEFINE_Bool(group_commit_wait_replay_wal_finish, "false");
+// Max time(ms) to wait for creating group commit plan fragment.
+// 0 means no timeout, default 5min.
Review Comment:
The comment says the default timeout is 5 minutes, but the configured
default here is 120000ms (2 minutes). Please align the comment and the actual
default value (either change the value to 300000ms or update the comment).
##########
regression-test/suites/load_p0/stream_load/test_group_commit_stream_load_high_concurrency_async.groovy:
##########
@@ -0,0 +1,138 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+import org.awaitility.Awaitility
+
+import static java.util.concurrent.TimeUnit.SECONDS
+
+import java.util.Collections
+import java.util.concurrent.atomic.AtomicBoolean
+import java.util.concurrent.atomic.AtomicInteger
+
+suite("test_group_commit_stream_load_high_concurrency_async", "p0") {
+ def tableName = "test_group_commit_stream_load_high_concurrency_async"
+ int concurrentClients = 100
+ int loadsPerClient = 1000
Review Comment:
The test spawns 100 threads and performs 1000 stream loads per thread (100k
HTTP loads). For a p0 regression suite this is likely to be slow and flaky in
CI; consider reducing the concurrency/iterations and/or moving this scenario to
a higher-level suite (e.g. p2) while keeping a smaller smoke case in p0.
##########
be/src/common/config.h:
##########
@@ -1508,6 +1508,9 @@ DECLARE_mInt32(group_commit_queue_mem_limit);
// group_commit_wal_max_disk_limit=1024 or group_commit_wal_max_disk_limit=10%
can be automatically identified.
DECLARE_mString(group_commit_wal_max_disk_limit);
DECLARE_Bool(group_commit_wait_replay_wal_finish);
+// Max time(ms) to wait for creating group commit plan fragment.
+// 0 means no timeout, default 5min.
Review Comment:
The comment states the default is 5 minutes, but config.cpp sets 120000ms.
Please keep the header comment consistent with the actual default to avoid
confusion when tuning this setting.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
