You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by al...@apache.org on 2025/06/20 04:36:42 UTC
(kudu) branch branch-1.18.x updated (4a08dde1c -> cd1c16a87)
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a change to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
from 4a08dde1c [docs] refresh list of supported OSes and distros
new f2f9978ba [tablet] less re-allocation of vectors in RowsetTree
new f3925af02 [arm64] Fix TestLeafNode unit test by ensuring proper node alignment
new cd1c16a87 [thirdparty] don't use system curl for cmake build
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
src/kudu/tablet/cbtree-test.cc | 22 +++++++++++++----
src/kudu/tablet/rowset_tree-test.cc | 7 ++++--
src/kudu/tablet/rowset_tree.cc | 47 +++++++++++++++++++------------------
src/kudu/tablet/rowset_tree.h | 4 ++--
src/kudu/tablet/tablet.cc | 30 +++++++++++++++--------
thirdparty/build-definitions.sh | 20 ++++++++++++----
6 files changed, 83 insertions(+), 47 deletions(-)
(kudu) 02/03: [arm64] Fix TestLeafNode unit test by ensuring proper node alignment
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit f3925af02e6de1862df1e98319f529b9f6e7b67f
Author: kedeng <kd...@gmail.com>
AuthorDate: Wed Jun 4 10:58:06 2025 +0800
[arm64] Fix TestLeafNode unit test by ensuring proper node alignment
The following error occurs when running `bin/cbtree-test
--gtest_filter=TestCBTree.TestLeafNode` in an Rocky ARM environment.
*** Aborted at 1748486900 (unix time) try "date -d @1748486900" if you are using GNU date ***
PC: @ 0x0 (unknown)
*** SIGBUS (@0xffffc794b2af) received by PID 32172 (TID 0xffff838a0040) from PID 18446744072762995375; stack trace: ***
@ 0xffff83f0e890 google::(anonymous namespace)::FailureSignalHandler()
@ 0xffff853b07a0 ([vdso]+0x79f)
@ 0x279efc kudu::tablet::btree::VersionField::Lock()
@ 0x2815e4 kudu::tablet::btree::NodeBase<>::Lock()
@ 0x2788a4 kudu::tablet::btree::TestCBTree::InsertInLeaf<>()
@ 0x269d18 kudu::tablet::btree::TestCBTree_TestLeafNode_Test::TestBody()
@ 0xffff83de0608 testing::internal::HandleExceptionsInMethodIfSupported<>()
@ 0xffff83dd2348 testing::Test::Run()
@ 0xffff83dd24e8 testing::TestInfo::Run()
@ 0xffff83dd2b90 testing::TestSuite::Run()
@ 0xffff83dd7274 testing::internal::UnitTestImpl::RunAllTests()
@ 0xffff83dd25f0 testing::UnitTest::Run()
@ 0xffff85335f84 (unknown)
@ 0xffff853353e4 (unknown)
@ 0xffff838e7280 __libc_start_call_main
@ 0xffff838e7358 __libc_start_main_alias_2
@ 0x2698b0 _start
Bus error
The TestLeafNode unit test was triggering SIGBUS crashes on ARM64
architectures due to unaligned access of atomic version fields.
This occurred because the test created LeafNode instances on the stack
without guaranteed alignment, while ARM64 strictly requires 8-byte
alignment for atomic uint64_t operations.
This test-specific fix complements the production code alignment
measures and ensures:
- Unit tests respect the same alignment requirements as production
- ARM64 environments don't trigger SIGBUS during test execution
- Test reliability across different architectures
Testing environment:
- ARM64 (Neoverse-N1) with Rocky Linux 9.5
- Docker container runtime
- Strict alignment enforcement causing SIGBUS on unaligned access
We extend our gratitude to Zoltan for his essential testing validation
and remediation recommendations in this fix.
Change-Id: Ib838699ce3f966b2e7c371d5c74701f2f8d21a55
Change-I: I6fea46308f942ded97d93d27ae466fd13a31b53
Reviewed-on: http://gerrit.cloudera.org:8080/23049
Reviewed-by: Zoltan Martonka <zm...@cloudera.com>
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
(cherry picked from commit fd5d0225880ae36f2437c83fa46c3a65c86f8931)
Reviewed-on: http://gerrit.cloudera.org:8080/23053
Reviewed-by: KeDeng <kd...@gmail.com>
Tested-by: KeDeng <kd...@gmail.com>
---
src/kudu/tablet/cbtree-test.cc | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/src/kudu/tablet/cbtree-test.cc b/src/kudu/tablet/cbtree-test.cc
index e76d96738..33289d285 100644
--- a/src/kudu/tablet/cbtree-test.cc
+++ b/src/kudu/tablet/cbtree-test.cc
@@ -69,6 +69,17 @@ namespace kudu {
namespace tablet {
namespace btree {
+// In unit tests, we explicitly enforce 8-byte alignment for LeafNode/InternalNode
+// instances created on the stack. This is critical because:
+// 1. ARM64 architectures require 8-byte alignment for atomic operations on uint64_t.
+// Unaligned access triggers SIGBUS.
+// 2. Production code guarantees alignment through:
+// a) PACKED in NodeBase definition
+// b) Arena allocator's alignment-aware allocations
+// 3. Stack-allocated test nodes lack these guarantees, making explicit alignment
+// necessary to match production behavior and prevent architecture-specific crashes.
+// 4. This ensures test validity across x86 (tolerant to unaligned access) and ARM
+// (strict alignment requirements) platforms.
class TestCBTree : public KuduTest {
protected:
template<class T>
@@ -97,7 +108,7 @@ class TestCBTree : public KuduTest {
Slice key(kbuf, key_size);
Slice val(vbuf, val_size);
- LeafNode<BTreeTraits> lnode(false);
+ alignas(sizeof(AtomicVersion)) LeafNode<BTreeTraits> lnode(false);
ASSERT_EQ(INSERT_SUCCESS,
InsertInLeaf(&lnode, &arena, key, val));
}
@@ -114,16 +125,17 @@ class TestCBTree : public KuduTest {
TEST_F(TestCBTree, TestNodeSizes) {
ThreadSafeArena arena(1024);
- LeafNode<BTreeTraits> lnode(false);
+ alignas(sizeof(AtomicVersion)) LeafNode<BTreeTraits> lnode(false);
ASSERT_LE(sizeof(lnode), BTreeTraits::kLeafNodeSize);
- InternalNode<BTreeTraits> inode(Slice("split"), &lnode, &lnode, &arena);
+ alignas(sizeof(AtomicVersion)) InternalNode<BTreeTraits> inode(Slice("split"), &lnode,
+ &lnode, &arena);
ASSERT_LE(sizeof(inode), BTreeTraits::kInternalNodeSize);
}
TEST_F(TestCBTree, TestLeafNode) {
- LeafNode<BTreeTraits> lnode(false);
+ alignas(sizeof(AtomicVersion)) LeafNode<BTreeTraits> lnode(false);
ThreadSafeArena arena(1024);
Slice k1("key1");
@@ -178,7 +190,7 @@ TEST_F(TestCBTree, TestLeafNode) {
// Directly test leaf node with keys and values which are large (such that
// only zero or one would fit in the actual allocated space)
TEST_F(TestCBTree, TestLeafNodeBigKVs) {
- LeafNode<BTreeTraits> lnode(false);
+ alignas(sizeof(AtomicVersion)) LeafNode<BTreeTraits> lnode(false);
DoBigKVTest(1000, 1000);
}
(kudu) 01/03: [tablet] less re-allocation of vectors in RowsetTree
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit f2f9978ba018f7efbb9b5f55e10861132aba0dbe
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Wed Jun 11 00:53:41 2025 -0700
[tablet] less re-allocation of vectors in RowsetTree
Since row check presence is in the hot path for high intensity ingestion
and update workloads, it makes sense to minimize the number of memory
reallocations to help with lock contention in tcmalloc. I found it's
quite trivial to do so for Tablet::CaptureConsistentIterators() and
RowSetTree::FindRowSets{WithKeyInRange,IntersectingInterval}(), and this
is addressed here. For more context, in the field we saw extreme
situations when tens (or even 100+) rowset iterators were present
for a scanner per reported metrics, so several reallocations might
be done in RowSetTree::FindRowSets...() methods when growing std::vector
with the standard capacity-doubling strategy.
I also updated rowset_tree-test.cc to use std::stable_sort() instead
of std::sort() to match how it's done in Tablet::BulkCheckPresence().
With that, the performance numbers on bulk key presence checks reported
by TestRowSetTreePerformance scenarios in rowset_tree.cc should be
closer to what it takes in Tablet::BulkCheckPresence().
I verified that the performance with this patch didn't degrade at least
as per the numbers reported by benchmarking scenarios in rowset_tree.cc.
This changelist doesn't contains any functional modifications.
Change-Id: Ia5ffa0cce6b7a94567a7316c4909c7bbcfa1439c
Reviewed-on: http://gerrit.cloudera.org:8080/23018
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
(cherry picked from commit 675a571201ad4ae2a2e36f7aa27b604ebbf3a180)
Reviewed-on: http://gerrit.cloudera.org:8080/23036
Tested-by: Alexey Serbin <al...@apache.org>
---
src/kudu/tablet/rowset_tree-test.cc | 7 ++++--
src/kudu/tablet/rowset_tree.cc | 47 +++++++++++++++++++------------------
src/kudu/tablet/rowset_tree.h | 4 ++--
src/kudu/tablet/tablet.cc | 30 +++++++++++++++--------
4 files changed, 51 insertions(+), 37 deletions(-)
diff --git a/src/kudu/tablet/rowset_tree-test.cc b/src/kudu/tablet/rowset_tree-test.cc
index b64032665..7c7646d7e 100644
--- a/src/kudu/tablet/rowset_tree-test.cc
+++ b/src/kudu/tablet/rowset_tree-test.cc
@@ -307,6 +307,7 @@ TEST_P(TestRowSetTreePerformance, TestPerformance) {
ASSERT_OK(tree.Reset(vec));
vector<string> queries;
+ queries.reserve(kNumQueries);
for (int i = 0; i < kNumQueries; i++) {
int query = rand() % 10000;
queries.emplace_back(StringPrintf("%04d", query));
@@ -325,15 +326,17 @@ TEST_P(TestRowSetTreePerformance, TestPerformance) {
one_at_time_timer.stop();
vector<Slice> query_slices;
+ query_slices.reserve(queries.size());
for (const auto& q : queries) {
query_slices.emplace_back(q);
}
batch_timer.resume();
- std::sort(query_slices.begin(), query_slices.end(), Slice::Comparator());
+ // Using std::stable_sort() to match how the row presence check in bulk
+ // is implemented in Tablet::BulkCheckPresence().
+ std::stable_sort(query_slices.begin(), query_slices.end(), Slice::Comparator());
int bulk_matches = 0;
{
- vector<RowSet *> out;
tree.ForEachRowSetContainingKeys(
query_slices, [&](RowSet* rs, int slice_idx) {
bulk_matches++;
diff --git a/src/kudu/tablet/rowset_tree.cc b/src/kudu/tablet/rowset_tree.cc
index 42c0b109b..4b67ab9c1 100644
--- a/src/kudu/tablet/rowset_tree.cc
+++ b/src/kudu/tablet/rowset_tree.cc
@@ -126,7 +126,11 @@ struct RowSetIntervalTraits {
};
RowSetTree::RowSetTree()
- : initted_(false) {
+ : initted_(false) {
+}
+
+RowSetTree::~RowSetTree() {
+ STLDeleteElements(&entries_);
}
Status RowSetTree::Reset(const RowSetVector &rowsets) {
@@ -201,36 +205,38 @@ void RowSetTree::FindRowSetsIntersectingInterval(const optional<Slice>& lower_bo
vector<RowSet*>* rowsets) const {
DCHECK(initted_);
+ vector<RowSetWithBounds*> from_tree;
+ from_tree.reserve(all_rowsets_.size());
+ tree_->FindIntersectingInterval(lower_bound, upper_bound, &from_tree);
+
+ rowsets->reserve(rowsets->size() + unbounded_rowsets_.size() + from_tree.size());
+
// All rowsets with unknown bounds need to be checked.
- for (const shared_ptr<RowSet> &rs : unbounded_rowsets_) {
+ for (const auto& rs : unbounded_rowsets_) {
rowsets->push_back(rs.get());
}
-
- vector<RowSetWithBounds *> from_tree;
- from_tree.reserve(all_rowsets_.size());
- tree_->FindIntersectingInterval(lower_bound, upper_bound, &from_tree);
- rowsets->reserve(rowsets->size() + from_tree.size());
- for (RowSetWithBounds *rs : from_tree) {
+ for (auto* rs : from_tree) {
rowsets->push_back(rs->rowset);
}
}
-void RowSetTree::FindRowSetsWithKeyInRange(const Slice &encoded_key,
- vector<RowSet *> *rowsets) const {
+void RowSetTree::FindRowSetsWithKeyInRange(const Slice& encoded_key,
+ vector<RowSet*>* rowsets) const {
DCHECK(initted_);
- // All rowsets with unknown bounds need to be checked.
- for (const shared_ptr<RowSet> &rs : unbounded_rowsets_) {
- rowsets->push_back(rs.get());
- }
-
// Query the interval tree to efficiently find rowsets with known bounds
// whose ranges overlap the probe key.
- vector<RowSetWithBounds *> from_tree;
+ vector<RowSetWithBounds*> from_tree;
from_tree.reserve(all_rowsets_.size());
tree_->FindContainingPoint(encoded_key, &from_tree);
- rowsets->reserve(rowsets->size() + from_tree.size());
- for (RowSetWithBounds *rs : from_tree) {
+
+ rowsets->reserve(rowsets->size() + unbounded_rowsets_.size() + from_tree.size());
+
+ // All rowsets with unknown bounds need to be checked.
+ for (const auto& rs : unbounded_rowsets_) {
+ rowsets->push_back(rs.get());
+ }
+ for (auto* rs : from_tree) {
rowsets->push_back(rs->rowset);
}
}
@@ -264,10 +270,5 @@ void RowSetTree::ForEachRowSetContainingKeys(
});
}
-
-RowSetTree::~RowSetTree() {
- STLDeleteElements(&entries_);
-}
-
} // namespace tablet
} // namespace kudu
diff --git a/src/kudu/tablet/rowset_tree.h b/src/kudu/tablet/rowset_tree.h
index 0af21d902..ba75d96af 100644
--- a/src/kudu/tablet/rowset_tree.h
+++ b/src/kudu/tablet/rowset_tree.h
@@ -46,7 +46,7 @@ struct RowSetWithBounds;
// intervals generated by the row sets (for instance, if a tablet has
// rowsets [0, 2] and [1, 3] it has three implicit contiguous intervals:
// [0, 1], [1, 2], and [2, 3].
-class RowSetTree {
+class RowSetTree final {
public:
// An RSEndpoint is a POD which associates a rowset, an EndpointType
// (either the START or STOP of an interval), and the key at which the
@@ -65,8 +65,8 @@ class RowSetTree {
};
RowSetTree();
- Status Reset(const RowSetVector &rowsets);
~RowSetTree();
+ Status Reset(const RowSetVector &rowsets);
// Return all RowSets whose range may contain the given encoded key.
//
diff --git a/src/kudu/tablet/tablet.cc b/src/kudu/tablet/tablet.cc
index 345bdb2a9..c2ad6590d 100644
--- a/src/kudu/tablet/tablet.cc
+++ b/src/kudu/tablet/tablet.cc
@@ -2475,9 +2475,27 @@ Status Tablet::CaptureConsistentIterators(
shared_lock<rw_spinlock> l(component_lock_);
RETURN_IF_STOPPED_OR_CHECK_STATE(kOpen);
+ // Cull row-sets in the case of key-range queries.
+ const bool is_range_query = spec != nullptr &&
+ (spec->lower_bound_key() || spec->exclusive_upper_bound_key());
+ vector<RowSet*> interval_sets;
+ if (is_range_query) {
+ optional<Slice> lower_bound = spec->lower_bound_key() ?
+ optional<Slice>(spec->lower_bound_key()->encoded_key()) : nullopt;
+ optional<Slice> upper_bound = spec->exclusive_upper_bound_key() ?
+ optional<Slice>(spec->exclusive_upper_bound_key()->encoded_key()) : nullopt;
+ components_->rowsets->FindRowSetsIntersectingInterval(
+ lower_bound, upper_bound, &interval_sets);
+ }
+
+ const size_t ret_size =
+ (is_range_query ? interval_sets.size() : components_->rowsets->all_rowsets().size()) +
+ components_->txn_memrowsets.size() + 1;
+
// Construct all the iterators locally first, so that if we fail
// in the middle, we don't modify the output arguments.
vector<IterWithBounds> ret;
+ ret.reserve(ret_size);
// Grab the memrowset iterator.
{
@@ -2498,14 +2516,7 @@ Status Tablet::CaptureConsistentIterators(
ret.emplace_back(std::move(txn_mrs_iwb));
}
- // Cull row-sets in the case of key-range queries.
- if (spec != nullptr && (spec->lower_bound_key() || spec->exclusive_upper_bound_key())) {
- optional<Slice> lower_bound = spec->lower_bound_key() ?
- optional<Slice>(spec->lower_bound_key()->encoded_key()) : nullopt;
- optional<Slice> upper_bound = spec->exclusive_upper_bound_key() ?
- optional<Slice>(spec->exclusive_upper_bound_key()->encoded_key()) : nullopt;
- vector<RowSet*> interval_sets;
- components_->rowsets->FindRowSetsIntersectingInterval(lower_bound, upper_bound, &interval_sets);
+ if (is_range_query) {
for (const auto* rs : interval_sets) {
IterWithBounds iwb;
RETURN_NOT_OK_PREPEND(rs->NewRowIteratorWithBounds(opts, &iwb),
@@ -3331,8 +3342,7 @@ Status Tablet::Iterator::Init(ScanSpec *spec) {
break;
}
- RETURN_NOT_OK(iter_->Init(spec));
- return Status::OK();
+ return iter_->Init(spec);
}
bool Tablet::Iterator::HasNext() const {
(kudu) 03/03: [thirdparty] don't use system curl for cmake build
Posted by al...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit cd1c16a8746e9c25f4b7125d5661956f44042016
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Wed Jun 18 12:49:00 2025 -0700
[thirdparty] don't use system curl for cmake build
An update was introduced with curl v8.13.0 [1] that breaks cmake 3.25.3
compilation if using curl's v8.13.0 and newer header files.
This changelist disables using system curl header files for a while.
Hopefully, next cmake version in Kudu's 3rd-party will have the issue
addressed, so it will be possible to use system curl again to speed up
3rd-party compilation.
It would be possible to make a decision on using the system curl based
on the version of libcurl reported by pkg-config, but I'm not exactly
sure it's worth it.
[1] https://github.com/curl/curl/commit/2ec00372a
Change-Id: I2ec9f079667a374f04664abf4a9e840c86b3b7bf
Reviewed-on: http://gerrit.cloudera.org:8080/23048
Tested-by: Marton Greber <gr...@gmail.com>
Reviewed-by: Gabriella Lotz <lo...@gmail.com>
Reviewed-by: Marton Greber <gr...@gmail.com>
(cherry picked from commit 68c30f181d2241489fb08580071e03c3a956cbcc)
Reviewed-on: http://gerrit.cloudera.org:8080/23054
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
Tested-by: Alexey Serbin <al...@apache.org>
---
thirdparty/build-definitions.sh | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/thirdparty/build-definitions.sh b/thirdparty/build-definitions.sh
index 60e309f53..b6f39e308 100644
--- a/thirdparty/build-definitions.sh
+++ b/thirdparty/build-definitions.sh
@@ -89,11 +89,21 @@ fixup_libtool() {
}
build_cmake() {
- # Use system curl if it is available: this shortens build times a bit.
- local use_system_curl=""
- if pkg-config --exists libcurl; then
- use_system_curl="--system-curl"
- fi
+ # It would be nice to use system curl if it is available to shorten build
+ # times, but with curl v8.13.0 and newer some of the CURL_NETRC_OPTION
+ # enumerators were made constants of type long [1], and compiling
+ # cmake v3.25.3 fails when trying to assign those constants to strings.
+ #
+ # [1] https://github.com/curl/curl/commit/2ec00372a
+ local use_system_curl="--no-system-curl"
+
+ # TODO(aserbin): switch to using system curl when migrating to newer
+ # cmake version that's able to build with curl v8.13.0
+ # and newer
+ #if pkg-config --exists libcurl; then
+ # use_system_curl="--system-curl"
+ #fi
+
CMAKE_BDIR=$TP_BUILD_DIR/$CMAKE_NAME$MODE_SUFFIX
mkdir -p $CMAKE_BDIR
pushd $CMAKE_BDIR