This is an automated email from the ASF dual-hosted git repository.
dataroaring pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new f70aee99146 [Fix](Compaction) atomic should not be implicitly
converted to int64_t (#49427)
f70aee99146 is described below
commit f70aee99146e277117b1c600425d8c13da0d624f
Author: abmdocrt <lianyuk...@selectdb.com>
AuthorDate: Tue Mar 25 14:06:37 2025 +0800
[Fix](Compaction) atomic should not be implicitly converted to int64_t
(#49427)
atomic<int64_t> cannot be implicitly converted to int64_t, which may
lead to data errors in high concurrency situations.
---
be/src/olap/compaction_permit_limiter.h | 2 +-
be/src/olap/tablet.cpp | 9 ++-
be/test/olap/compaction_permit_limiter_test.cpp | 78 +++++++++++++++++++++++++
3 files changed, 85 insertions(+), 4 deletions(-)
diff --git a/be/src/olap/compaction_permit_limiter.h
b/be/src/olap/compaction_permit_limiter.h
index e92c35522b3..d0edbd73815 100644
--- a/be/src/olap/compaction_permit_limiter.h
+++ b/be/src/olap/compaction_permit_limiter.h
@@ -40,7 +40,7 @@ public:
void release(int64_t permits);
- int64_t usage() const { return _used_permits; }
+ int64_t usage() const { return
_used_permits.load(std::memory_order_relaxed); }
private:
// sum of "permits" held by executing compaction tasks currently
diff --git a/be/src/olap/tablet.cpp b/be/src/olap/tablet.cpp
index c3c6d1a5955..619429149bb 100644
--- a/be/src/olap/tablet.cpp
+++ b/be/src/olap/tablet.cpp
@@ -1763,7 +1763,8 @@ Status Tablet::prepare_compaction_and_calculate_permits(
}
if (!res.is<CUMULATIVE_NO_SUITABLE_VERSION>()) {
DorisMetrics::instance()->cumulative_compaction_request_failed->increment(1);
- return Status::InternalError("prepare cumulative compaction
with err: {}", res);
+ return Status::InternalError("prepare cumulative compaction
with err: {}",
+ res.to_string());
}
// return OK if OLAP_ERR_CUMULATIVE_NO_SUITABLE_VERSION, so that
we don't need to
// print too much useless logs.
@@ -1798,7 +1799,8 @@ Status Tablet::prepare_compaction_and_calculate_permits(
permits = 0;
if (!res.is<BE_NO_SUITABLE_VERSION>()) {
DorisMetrics::instance()->base_compaction_request_failed->increment(1);
- return Status::InternalError("prepare base compaction with
err: {}", res);
+ return Status::InternalError("prepare base compaction with
err: {}",
+ res.to_string());
}
// return OK if OLAP_ERR_BE_NO_SUITABLE_VERSION, so that we don't
need to
// print too much useless logs.
@@ -1814,7 +1816,8 @@ Status Tablet::prepare_compaction_and_calculate_permits(
tablet->set_last_full_compaction_failure_time(UnixMillis());
permits = 0;
if (!res.is<FULL_NO_SUITABLE_VERSION>()) {
- return Status::InternalError("prepare full compaction with
err: {}", res);
+ return Status::InternalError("prepare full compaction with
err: {}",
+ res.to_string());
}
// return OK if OLAP_ERR_BE_NO_SUITABLE_VERSION, so that we don't
need to
// print too much useless logs.
diff --git a/be/test/olap/compaction_permit_limiter_test.cpp
b/be/test/olap/compaction_permit_limiter_test.cpp
new file mode 100644
index 00000000000..23d4915d0f7
--- /dev/null
+++ b/be/test/olap/compaction_permit_limiter_test.cpp
@@ -0,0 +1,78 @@
+// 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.
+
+#include "olap/compaction_permit_limiter.h"
+
+#include <gmock/gmock-actions.h>
+#include <gmock/gmock-matchers.h>
+#include <gtest/gtest-message.h>
+#include <gtest/gtest-test-part.h>
+#include <gtest/gtest.h>
+
+#include "common/config.h"
+
+namespace doris {
+
+class CompactionPermitLimiterTest : public testing::Test {};
+
+TEST(CompactionPermitLimiterTest, UsageCorrectness) {
+ CompactionPermitLimiter limiter;
+
+ // Initial usage should be 0
+ EXPECT_EQ(0, limiter.usage());
+
+ // Test single request
+ limiter.request(10);
+ EXPECT_EQ(10, limiter.usage());
+
+ // Test release
+ limiter.release(5);
+ EXPECT_EQ(5, limiter.usage());
+
+ // Release all
+ limiter.release(5);
+ EXPECT_EQ(0, limiter.usage());
+
+ // Test multiple concurrent requests
+ const int num_threads = 10;
+ const int permits_per_thread = 100;
+ std::vector<std::thread> threads;
+
+ for (int i = 0; i < num_threads; i++) {
+ threads.emplace_back([&limiter]() {
limiter.request(permits_per_thread); });
+ }
+
+ for (auto& t : threads) {
+ t.join();
+ }
+
+ EXPECT_EQ(num_threads * permits_per_thread, limiter.usage());
+
+ // Test multiple concurrent releases
+ threads.clear();
+ for (int i = 0; i < num_threads; i++) {
+ threads.emplace_back([&limiter]() {
limiter.release(permits_per_thread); });
+ }
+
+ for (auto& t : threads) {
+ t.join();
+ }
+
+ EXPECT_EQ(0, limiter.usage());
+}
+
+} // namespace doris
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
