Skip to content

Commit 77cacdb

Browse files
rootroot
root
authored and
root
committed
[fix](storage) Fix linker error for SegmentWriter::_is_mow() with -O1/-O3
1 parent e2aaee4 commit 77cacdb

5 files changed

Lines changed: 212 additions & 20 deletions

File tree

‎be/src/storage/segment/segment_writer.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1244,13 +1244,5 @@ Status SegmentWriter::_generate_short_key_index(std::vector<IOlapColumnDataAcces
12441244
return Status::OK();
12451245
}
12461246

1247-
bool SegmentWriter::_is_mow() {
1248-
return _tablet_schema->keys_type() == UNIQUE_KEYS && _opts.enable_unique_key_merge_on_write;
1249-
}
1250-
1251-
bool SegmentWriter::_is_mow_with_cluster_key() {
1252-
return _is_mow() && !_tablet_schema->cluster_key_uids().empty();
1253-
}
1254-
12551247
} // namespace segment_v2
12561248
} // namespace doris

‎be/src/storage/segment/segment_writer.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,12 @@ class SegmentWriter {
189189
IOlapColumnDataAccessor* seq_column, size_t num_rows, bool need_sort);
190190
Status _generate_short_key_index(std::vector<IOlapColumnDataAccessor*>& key_columns,
191191
size_t num_rows, const std::vector<size_t>& short_key_pos);
192-
bool _is_mow();
193-
bool _is_mow_with_cluster_key();
192+
bool _is_mow() {
193+
return _tablet_schema->keys_type() == UNIQUE_KEYS && _opts.enable_unique_key_merge_on_write;
194+
}
195+
bool _is_mow_with_cluster_key() {
196+
return _is_mow() && !_tablet_schema->cluster_key_uids().empty();
197+
}
194198

195199
protected:
196200
// Build key index for derived writers that override append_block.

‎be/src/storage/segment/vertical_segment_writer.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,12 +1516,4 @@ void VerticalSegmentWriter::_set_max_key(const Slice& key) {
15161516
_max_key.append(key.get_data(), key.get_size());
15171517
}
15181518

1519-
inline bool VerticalSegmentWriter::_is_mow() {
1520-
return _tablet_schema->keys_type() == UNIQUE_KEYS && _opts.enable_unique_key_merge_on_write;
1521-
}
1522-
1523-
inline bool VerticalSegmentWriter::_is_mow_with_cluster_key() {
1524-
return _is_mow() && !_tablet_schema->cluster_key_uids().empty();
1525-
}
1526-
15271519
} // namespace doris::segment_v2

‎be/src/storage/segment/vertical_segment_writer.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,12 @@ class VerticalSegmentWriter {
199199
Status _check_column_writer_disk_capacity(size_t cid);
200200
Status _finalize_column_writer_and_update_meta(size_t cid);
201201

202-
bool _is_mow();
203-
bool _is_mow_with_cluster_key();
202+
bool _is_mow() {
203+
return _tablet_schema->keys_type() == UNIQUE_KEYS && _opts.enable_unique_key_merge_on_write;
204+
}
205+
bool _is_mow_with_cluster_key() {
206+
return _is_mow() && !_tablet_schema->cluster_key_uids().empty();
207+
}
204208

205209
private:
206210
friend class ::doris::BlockAggregator;

‎be/test/storage/segment/segment_writer_mow_check_test.cpp‎

Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
#include <gtest/gtest.h>
19+
20+
#include <memory>
21+
#include <string>
22+
23+
#include "io/fs/local_file_system.h"
24+
#include "storage/olap_common.h"
25+
#include "storage/rowset/rowset_id_generator.h"
26+
#include "storage/segment/segment_writer.h"
27+
#include "storage/segment/vertical_segment_writer.h"
28+
#include "storage/tablet/tablet_schema.h"
29+
30+
namespace doris::segment_v2 {
31+
32+
// Subclass in a different translation unit that calls _is_mow() and _is_mow_with_cluster_key().
33+
// This test verifies that these inline private methods are visible across translation units.
34+
// Previously they were defined with `inline` in .cpp files, which caused linker errors
35+
// when compiled with -O1 or higher (TSAN/RELEASE), because the compiler inlined them
36+
// and did not export the symbols.
37+
class TestSegmentWriterMowCheck : public SegmentWriter {
38+
public:
39+
using SegmentWriter::SegmentWriter;
40+
41+
bool check_is_mow() { return _is_mow(); }
42+
bool check_is_mow_with_cluster_key() { return _is_mow_with_cluster_key(); }
43+
};
44+
45+
class TestVerticalSegmentWriterMowCheck : public VerticalSegmentWriter {
46+
public:
47+
using VerticalSegmentWriter::VerticalSegmentWriter;
48+
49+
bool check_is_mow() { return _is_mow(); }
50+
bool check_is_mow_with_cluster_key() { return _is_mow_with_cluster_key(); }
51+
};
52+
53+
static const std::string kSegmentDir = "./ut_dir/segment_writer_mow_check_test";
54+
55+
TabletColumnPtr create_int_key(int32_t id) {
56+
auto column = std::make_shared<TabletColumn>();
57+
column->_unique_id = id;
58+
column->_col_name = std::to_string(id);
59+
column->_type = FieldType::OLAP_FIELD_TYPE_INT;
60+
column->_is_key = true;
61+
column->_is_nullable = false;
62+
column->_length = 4;
63+
column->_index_length = 4;
64+
return column;
65+
}
66+
67+
TabletColumnPtr create_int_value(int32_t id) {
68+
auto column = std::make_shared<TabletColumn>();
69+
column->_unique_id = id;
70+
column->_col_name = std::to_string(id);
71+
column->_type = FieldType::OLAP_FIELD_TYPE_INT;
72+
column->_is_key = false;
73+
column->_is_nullable = true;
74+
column->_length = 4;
75+
column->_index_length = 4;
76+
return column;
77+
}
78+
79+
TabletSchemaSPtr create_unique_key_schema() {
80+
TabletSchemaSPtr schema = std::make_shared<TabletSchema>();
81+
schema->append_column(*create_int_key(0));
82+
schema->append_column(*create_int_value(1));
83+
schema->_keys_type = UNIQUE_KEYS;
84+
return schema;
85+
}
86+
87+
TabletSchemaSPtr create_dup_key_schema() {
88+
TabletSchemaSPtr schema = std::make_shared<TabletSchema>();
89+
schema->append_column(*create_int_key(0));
90+
schema->append_column(*create_int_value(1));
91+
schema->_keys_type = DUP_KEYS;
92+
return schema;
93+
}
94+
95+
TabletSchemaSPtr create_unique_key_schema_with_cluster_key() {
96+
auto schema = create_unique_key_schema();
97+
schema->_cluster_key_uids = {1};
98+
return schema;
99+
}
100+
101+
class SegmentWriterMowCheckTest : public testing::Test {
102+
public:
103+
void SetUp() override {
104+
auto fs = io::global_local_filesystem();
105+
auto st = fs->delete_directory(kSegmentDir);
106+
ASSERT_TRUE(st.ok() || st.is<ErrorCode::NOT_FOUND>()) << st;
107+
st = fs->create_directory(kSegmentDir);
108+
ASSERT_TRUE(st.ok()) << st;
109+
}
110+
111+
void TearDown() override {
112+
EXPECT_TRUE(io::global_local_filesystem()->delete_directory(kSegmentDir).ok());
113+
}
114+
115+
io::FileWriterPtr create_file_writer(size_t segment_id) {
116+
RowsetId rowset_id;
117+
rowset_id.init(1);
118+
std::string filename = fmt::format("{}_{}.dat", rowset_id.to_string(), segment_id);
119+
std::string path = fmt::format("{}/{}", kSegmentDir, filename);
120+
io::FileWriterPtr file_writer;
121+
auto st = io::global_local_filesystem()->create_file(path, &file_writer);
122+
EXPECT_TRUE(st.ok()) << st;
123+
return file_writer;
124+
}
125+
};
126+
127+
TEST_F(SegmentWriterMowCheckTest, segment_writer_is_mow_false_for_dup_key) {
128+
auto schema = create_dup_key_schema();
129+
SegmentWriterOptions opts;
130+
opts.enable_unique_key_merge_on_write = true;
131+
auto file_writer = create_file_writer(0);
132+
TestSegmentWriterMowCheck writer(file_writer.get(), 0, schema, nullptr, nullptr, opts, nullptr);
133+
EXPECT_FALSE(writer.check_is_mow());
134+
EXPECT_FALSE(writer.check_is_mow_with_cluster_key());
135+
}
136+
137+
TEST_F(SegmentWriterMowCheckTest, segment_writer_is_mow_true_for_unique_mow) {
138+
auto schema = create_unique_key_schema();
139+
SegmentWriterOptions opts;
140+
opts.enable_unique_key_merge_on_write = true;
141+
auto file_writer = create_file_writer(1);
142+
TestSegmentWriterMowCheck writer(file_writer.get(), 1, schema, nullptr, nullptr, opts, nullptr);
143+
EXPECT_TRUE(writer.check_is_mow());
144+
EXPECT_FALSE(writer.check_is_mow_with_cluster_key());
145+
}
146+
147+
TEST_F(SegmentWriterMowCheckTest, segment_writer_is_mow_false_when_mow_disabled) {
148+
auto schema = create_unique_key_schema();
149+
SegmentWriterOptions opts;
150+
opts.enable_unique_key_merge_on_write = false;
151+
auto file_writer = create_file_writer(2);
152+
TestSegmentWriterMowCheck writer(file_writer.get(), 2, schema, nullptr, nullptr, opts, nullptr);
153+
EXPECT_FALSE(writer.check_is_mow());
154+
EXPECT_FALSE(writer.check_is_mow_with_cluster_key());
155+
}
156+
157+
TEST_F(SegmentWriterMowCheckTest, segment_writer_is_mow_with_cluster_key) {
158+
auto schema = create_unique_key_schema_with_cluster_key();
159+
SegmentWriterOptions opts;
160+
opts.enable_unique_key_merge_on_write = true;
161+
auto file_writer = create_file_writer(3);
162+
TestSegmentWriterMowCheck writer(file_writer.get(), 3, schema, nullptr, nullptr, opts, nullptr);
163+
EXPECT_TRUE(writer.check_is_mow());
164+
EXPECT_TRUE(writer.check_is_mow_with_cluster_key());
165+
}
166+
167+
TEST_F(SegmentWriterMowCheckTest, vertical_segment_writer_is_mow_false_for_dup_key) {
168+
auto schema = create_dup_key_schema();
169+
VerticalSegmentWriterOptions opts;
170+
opts.enable_unique_key_merge_on_write = true;
171+
auto file_writer = create_file_writer(4);
172+
TestVerticalSegmentWriterMowCheck writer(file_writer.get(), 4, schema, nullptr, nullptr, opts,
173+
nullptr);
174+
EXPECT_FALSE(writer.check_is_mow());
175+
EXPECT_FALSE(writer.check_is_mow_with_cluster_key());
176+
}
177+
178+
TEST_F(SegmentWriterMowCheckTest, vertical_segment_writer_is_mow_true_for_unique_mow) {
179+
auto schema = create_unique_key_schema();
180+
VerticalSegmentWriterOptions opts;
181+
opts.enable_unique_key_merge_on_write = true;
182+
auto file_writer = create_file_writer(5);
183+
TestVerticalSegmentWriterMowCheck writer(file_writer.get(), 5, schema, nullptr, nullptr, opts,
184+
nullptr);
185+
EXPECT_TRUE(writer.check_is_mow());
186+
EXPECT_FALSE(writer.check_is_mow_with_cluster_key());
187+
}
188+
189+
TEST_F(SegmentWriterMowCheckTest, vertical_segment_writer_is_mow_with_cluster_key) {
190+
auto schema = create_unique_key_schema_with_cluster_key();
191+
VerticalSegmentWriterOptions opts;
192+
opts.enable_unique_key_merge_on_write = true;
193+
auto file_writer = create_file_writer(6);
194+
TestVerticalSegmentWriterMowCheck writer(file_writer.get(), 6, schema, nullptr, nullptr, opts,
195+
nullptr);
196+
EXPECT_TRUE(writer.check_is_mow());
197+
EXPECT_TRUE(writer.check_is_mow_with_cluster_key());
198+
}
199+
200+
} // namespace doris::segment_v2

0 commit comments

Comments
 (0)