Skip to content

Commit 4052eba

Browse files
MryangeMryange
committed
[refine](storage) Strengthen enum type safety in storage code
Issue Number: N/A Related PR: #69174 Problem Summary: Use scoped enums and typed fields for storage states, character categories, compaction triggers, and binlog operations to prevent implicit integer conversions and mixing unrelated values. Preserve persisted values and Cloud source compatibility; adapt callers and existing tests. None - Test: ASAN BE build, header hygiene, and diff whitespace checks passed. Existing unit tests were adapted but not run. - Behavior changed: No - Does this need documentation: No
1 parent cca2231 commit 4052eba

102 files changed

Lines changed: 1004 additions & 804 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎be/src/agent/task_worker_pool.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1628,7 +1628,7 @@ void submit_table_compaction_callback(StorageEngine& engine, const TAgentTaskReq
16281628
tablet_ptr->set_last_full_compaction_schedule_time(UnixMillis());
16291629
Status status = engine.submit_compaction_task(tablet_ptr, CompactionType::FULL_COMPACTION,
16301630
/*force=*/false, /*eager=*/true,
1631-
/*trigger_method=*/1);
1631+
/*trigger_method=*/TriggerMethod::MANUAL);
16321632
if (!status.ok()) {
16331633
LOG(WARNING) << "failed to submit full compaction task. tablet_id="
16341634
<< tablet_ptr->tablet_id() << ", error=" << status;
@@ -1640,7 +1640,7 @@ void submit_table_compaction_callback(StorageEngine& engine, const TAgentTaskReq
16401640
auto* data_dir = tablet_ptr->data_dir();
16411641
if (!tablet_ptr->can_do_compaction(data_dir->path_hash(), compaction_type)) {
16421642
LOG(WARNING) << "could not do compaction. tablet_id=" << tablet_ptr->tablet_id()
1643-
<< ", compaction_type=" << compaction_type;
1643+
<< ", compaction_type=" << static_cast<int>(compaction_type);
16441644
return;
16451645
}
16461646

@@ -2353,7 +2353,8 @@ void set_alter_version_before_enqueue(CloudStorageEngine& engine, const TAgentTa
23532353
return;
23542354
}
23552355
auto new_tablet = engine.tablet_mgr().get_tablet(alter_req.new_tablet_id);
2356-
if (!new_tablet.has_value() || new_tablet.value()->tablet_state() == TABLET_RUNNING) {
2356+
if (!new_tablet.has_value() ||
2357+
new_tablet.value()->tablet_state() == TabletState::TABLET_RUNNING) {
23572358
return;
23582359
}
23592360
auto base_tablet = engine.tablet_mgr().get_tablet(alter_req.base_tablet_id);

‎be/src/exec/runtime_filter/runtime_filter_partition_pruner.cpp‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ Status ParsedPartitionBoundaries::parse(
169169
if (tb.__isset.range_start) { \
170170
auto parsed = parse_texpr_node(tb.range_start); \
171171
if (parsed) { \
172-
static_cast<void>(cvr.add_range(FILTER_LARGER_OR_EQUAL, *parsed)); \
172+
static_cast<void>( \
173+
cvr.add_range(SQLFilterOp::FILTER_LARGER_OR_EQUAL, *parsed)); \
173174
} else { \
174175
DORIS_CHECK(is_nullable); \
175176
range_has_null = true; \
@@ -182,8 +183,8 @@ Status ParsedPartitionBoundaries::parse(
182183
/* TPartitionBoundary.range_end_inclusive comment); single-column RANGE */ \
183184
/* keeps the natural OPEN upper bound matching Doris semantics. */ \
184185
SQLFilterOp upper_op = (tb.__isset.range_end_inclusive && tb.range_end_inclusive) \
185-
? FILTER_LESS_OR_EQUAL \
186-
: FILTER_LESS; \
186+
? SQLFilterOp::FILTER_LESS_OR_EQUAL \
187+
: SQLFilterOp::FILTER_LESS; \
187188
static_cast<void>(cvr.add_range(upper_op, *parsed)); \
188189
} \
189190
if (range_has_null) { \
@@ -574,20 +575,24 @@ Status ParsedPartitionBoundaries::get_or_compute_projected_boundaries(
574575
if (direction == TTargetExprMonotonicity::MONOTONIC_DECREASING) { \
575576
if (!hi_open[i]) { \
576577
auto hi_projected = projected_value(hi_result_col, hi_result_row[i]); \
577-
static_cast<void>(cvr.add_range(FILTER_LARGER_OR_EQUAL, hi_projected)); \
578+
static_cast<void>( \
579+
cvr.add_range(SQLFilterOp::FILTER_LARGER_OR_EQUAL, hi_projected)); \
578580
} \
579581
if (!lo_open[i]) { \
580582
auto lo_projected = projected_value(lo_result_col, lo_result_row[i]); \
581-
static_cast<void>(cvr.add_range(FILTER_LESS_OR_EQUAL, lo_projected)); \
583+
static_cast<void>( \
584+
cvr.add_range(SQLFilterOp::FILTER_LESS_OR_EQUAL, lo_projected)); \
582585
} \
583586
} else { \
584587
if (!lo_open[i]) { \
585588
auto lo_projected = projected_value(lo_result_col, lo_result_row[i]); \
586-
static_cast<void>(cvr.add_range(FILTER_LARGER_OR_EQUAL, lo_projected)); \
589+
static_cast<void>( \
590+
cvr.add_range(SQLFilterOp::FILTER_LARGER_OR_EQUAL, lo_projected)); \
587591
} \
588592
if (!hi_open[i]) { \
589593
auto hi_projected = projected_value(hi_result_col, hi_result_row[i]); \
590-
static_cast<void>(cvr.add_range(FILTER_LESS_OR_EQUAL, hi_projected)); \
594+
static_cast<void>( \
595+
cvr.add_range(SQLFilterOp::FILTER_LESS_OR_EQUAL, hi_projected)); \
591596
} \
592597
} \
593598
ParsedBoundary projected_boundary; \
@@ -645,13 +650,13 @@ Status ParsedPartitionBoundaries::get_or_compute_projected_boundaries(
645650
static std::optional<SQLFilterOp> convert_opcode_to_filter_op(TExprOpcode::type op) {
646651
switch (op) {
647652
case TExprOpcode::LE:
648-
return FILTER_LESS_OR_EQUAL;
653+
return SQLFilterOp::FILTER_LESS_OR_EQUAL;
649654
case TExprOpcode::LT:
650-
return FILTER_LESS;
655+
return SQLFilterOp::FILTER_LESS;
651656
case TExprOpcode::GE:
652-
return FILTER_LARGER_OR_EQUAL;
657+
return SQLFilterOp::FILTER_LARGER_OR_EQUAL;
653658
case TExprOpcode::GT:
654-
return FILTER_LARGER;
659+
return SQLFilterOp::FILTER_LARGER;
655660
default:
656661
return std::nullopt;
657662
}

‎be/src/exec/scan/scanner_scheduler.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,9 +84,10 @@ Status ScannerScheduler::submit(std::shared_ptr<ScannerContext> ctx,
8484
Status submit_status = sumbit_task();
8585
if (!submit_status.ok()) {
8686
// User will see TooManyTasks error. It looks like a more reasonable error.
87-
Status scan_task_status = Status::TooManyTasks(
88-
"Failed to submit scanner to scanner pool reason:" +
89-
std::string(submit_status.msg()) + "|type:" + std::to_string(type));
87+
Status scan_task_status =
88+
Status::TooManyTasks("Failed to submit scanner to scanner pool reason:" +
89+
std::string(submit_status.msg()) +
90+
"|type:" + std::to_string(static_cast<int>(type)));
9091
scan_task->set_status(scan_task_status);
9192
return scan_task_status;
9293
}

‎be/src/service/http/action/compaction_action.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ Status CompactionAction::_handle_run_compaction(HttpRequest* req, std::string* j
158158
for (const auto& tablet : tablet_vec) {
159159
tablet->set_last_full_compaction_schedule_time(UnixMillis());
160160
RETURN_IF_ERROR(_engine.submit_compaction_task(tablet, CompactionType::FULL_COMPACTION,
161-
force, true, 1));
161+
force, true, TriggerMethod::MANUAL));
162162
}
163163
} else {
164164
// 2. fetch the tablet by tablet_id
@@ -181,7 +181,7 @@ Status CompactionAction::_handle_run_compaction(HttpRequest* req, std::string* j
181181
// 3. submit full compaction task to thread pool
182182
tablet->set_last_full_compaction_schedule_time(UnixMillis());
183183
RETURN_IF_ERROR(_engine.submit_compaction_task(tablet, CompactionType::FULL_COMPACTION,
184-
force, true, 1));
184+
force, true, TriggerMethod::MANUAL));
185185
} else {
186186
// 3. execute base/cumulative compaction task in a detached thread
187187
std::packaged_task<Status()> task([this, tablet, compaction_type]() {

‎be/src/service/http/action/compaction_score_action.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ struct CloudCompactionScoresAccessor final : CompactionScoresAccessor {
123123
tablets.reserve(weak_tablets.size());
124124
for (auto& weak_tablet : weak_tablets) {
125125
if (auto tablet = weak_tablet.lock();
126-
tablet != nullptr and tablet->tablet_state() == TABLET_RUNNING) {
126+
tablet != nullptr and tablet->tablet_state() == TabletState::TABLET_RUNNING) {
127127
tablets.push_back(std::move(tablet));
128128
}
129129
}

‎be/src/storage/changelog.h‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "common/logging.h" // DCHECK
3030
#include "common/status.h"
3131
#include "exec/sink/autoinc_buffer.h"
32+
#include "storage/changelog_types.h"
3233
#include "storage/olap_common.h"
3334
#include "storage/olap_define.h" // DataWriteType
3435
#include "storage/rowset/rowset_fwd.h"
@@ -42,12 +43,6 @@ class AutoIncIDBuffer;
4243
struct PartialUpdateInfo;
4344
struct MowContext;
4445

45-
// Changelog op type.
46-
// NOTE: The value is persisted into changelog data, so keep it stable.
47-
static constexpr int64_t CHANGELOG_OP_APPEND = 0;
48-
static constexpr int64_t CHANGELOG_OP_UPDATE = 1;
49-
static constexpr int64_t CHANGELOG_OP_DELETE = 2;
50-
5146
namespace changelog {
5247

5348
// ATTN(changelog-rename): formerly "__BEFORE__<col>__"

‎be/src/storage/changelog_ttl_scanner.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ Status prepare_changelog_ttl(BaseStorageEngine& engine, int64_t tablet_id, bool
5454
RETURN_IF_ERROR(cloud_tablet->sync_meta());
5555
RETURN_IF_ERROR(cloud_tablet->sync_rowsets());
5656
}
57-
if (tablet->tablet_state() != TABLET_RUNNING || !tablet->scan_expired_changelog_rowsets()) {
57+
if (tablet->tablet_state() != TabletState::TABLET_RUNNING ||
58+
!tablet->scan_expired_changelog_rowsets()) {
5859
return Status::OK();
5960
}
6061
ttl_discovered << 1;

‎be/src/storage/changelog_types.h‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
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+
#pragma once
19+
20+
#include <cstdint>
21+
22+
namespace doris {
23+
24+
// The value is persisted into changelog data, so keep it stable.
25+
enum class ChangelogOp : int64_t { APPEND = 0, UPDATE = 1, DELETE = 2, UNKNOWN = 3 };
26+
27+
namespace changelog {
28+
29+
enum class StreamChangeOp : int64_t {
30+
INSERT = 0,
31+
DELETE = 1,
32+
UPDATE_BEFORE = 2,
33+
UPDATE_AFTER = 3,
34+
};
35+
36+
} // namespace changelog
37+
} // namespace doris

‎be/src/storage/compaction/base_compaction.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ Status BaseCompaction::execute_compact() {
103103
st = CompactionMixin::execute_compact();
104104
RETURN_IF_ERROR(st);
105105

106-
DCHECK_EQ(_state, CompactionState::SUCCESS);
106+
DCHECK(_state == CompactionState::SUCCESS);
107107

108108
DorisMetrics::instance()->base_compaction_deltas_total->increment(_input_rowsets.size());
109109
DorisMetrics::instance()->base_compaction_bytes_total->increment(_input_rowsets_total_size);

‎be/src/storage/compaction/cold_data_compaction.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ Status ColdDataCompaction::execute_compact() {
7070
return Status::Aborted<false>("this replica is not cooldown replica");
7171
}
7272
RETURN_IF_ERROR(CompactionMixin::execute_compact());
73-
DCHECK_EQ(_state, CompactionState::SUCCESS);
73+
DCHECK(_state == CompactionState::SUCCESS);
7474
return Status::OK();
7575
}
7676

0 commit comments

Comments
 (0)