This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.2
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.2 by this push:
new ce306f3689e branch-4.2: [fix](cloud) Skip empty rowsets before
resolving fs when checking tablet encryption #69694 (#70115)
ce306f3689e is described below
commit ce306f3689ed86833e8f3046d2b5bbe43243c7c7
Author: Xin Liao <[email protected]>
AuthorDate: Thu Nov 5 09:23:12 2026 +0800
branch-4.2: [fix](cloud) Skip empty rowsets before resolving fs when
checking tablet encryption #69694 (#70115)
### What problem does this PR solve?
Issue Number: None
Related PR: #69694
Problem Summary:
Backport #69694 to `branch-4.2`.
An encrypted tablet containing an empty version-hole rowset could be
reported as unencrypted when that rowset has no resolvable storage
resource. Skip empty rowsets before filesystem resolution and return
failures as `ResultError`, so errors are not implicitly converted into
`false`.
The conflict resolution retains this branch's `Rowset::segment_path(0)`
API. Includes tests for empty rowsets and error propagation.
### Release note
Fix tablet encryption checks misreporting encrypted tablets with empty
rowsets and correctly report filesystem-resolution errors.
### Check List (For Author)
- Test: clang-format 16 and `git diff --check` passed. Test constructor
and rowset APIs were checked against `branch-4.2`. BE unit tests were
not run locally because the local Xcode/toolchain environment blocked
the test entry point. CI `run buildall` is requested separately after PR
creation.
- Behavior changed: Yes. Empty rowsets are skipped and inspection errors
are surfaced as errors.
- Does this need documentation: No.
---
.../http/action/check_encryption_action.cpp | 12 ++-
.../service/http/action/check_encryption_action.h | 6 ++
.../service/http/check_encryption_action_test.cpp | 92 ++++++++++++++++++++++
3 files changed, 106 insertions(+), 4 deletions(-)
diff --git a/be/src/service/http/action/check_encryption_action.cpp
b/be/src/service/http/action/check_encryption_action.cpp
index 1c11fc25b64..3b66f104e2c 100644
--- a/be/src/service/http/action/check_encryption_action.cpp
+++ b/be/src/service/http/action/check_encryption_action.cpp
@@ -74,6 +74,11 @@ Result<bool> is_tablet_encrypted(const BaseTabletSPtr&
tablet) {
rs_meta->end_version() == 1) {
return;
}
+ // Must be checked before resolving the fs: an empty rowset has no
file to check, and a
+ // version hole rowset may carry a resource id that resolves to no
storage resource.
+ if (rs->num_segments() == 0) {
+ return;
+ }
// Must not be `physical_fs()`: packed rowsets keep their segments as
slices inside a
// shared object, and only the `PackedFileSystem` wrapper can resolve
a segment path to
// that slice. `fs()` is not an option either, since it decrypts and
would hide the
@@ -85,9 +90,6 @@ Result<bool> is_tablet_encrypted(const BaseTabletSPtr&
tablet) {
return;
}
- if (rs->num_segments() == 0) {
- return;
- }
auto maybe_seg_path = rs->segment_path(0);
if (!maybe_seg_path) {
st = std::move(maybe_seg_path.error());
@@ -141,7 +143,9 @@ Result<bool> is_tablet_encrypted(const BaseTabletSPtr&
tablet) {
if (st) {
return is_encrypted;
}
- return st;
+ // Must be wrapped: `Status` converts to `bool`, so a bare `return st;`
would report the
+ // error as "not encrypted".
+ return ResultError(std::move(st));
}
Result<std::string> get_last_encrypt_footer(const BaseTabletSPtr& tablet) {
diff --git a/be/src/service/http/action/check_encryption_action.h
b/be/src/service/http/action/check_encryption_action.h
index ae4ce978b16..32912ce07c4 100644
--- a/be/src/service/http/action/check_encryption_action.h
+++ b/be/src/service/http/action/check_encryption_action.h
@@ -18,12 +18,18 @@
#include <gen_cpp/FrontendService_types.h>
#include "cloud/cloud_tablet_mgr.h"
+#include "common/status.h"
#include "runtime/exec_env.h"
#include "service/http/http_handler_with_auth.h"
#include "service/http/http_request.h"
+#include "storage/tablet/tablet_fwd.h"
#include "storage/tablet/tablet_manager.h"
namespace doris {
+// Returns whether every non-empty rowset of `tablet` ends with the encryption
magic code, or an
+// error if any file could not be checked.
+Result<bool> is_tablet_encrypted(const BaseTabletSPtr& tablet);
+
class CheckEncryptionAction : public HttpHandlerWithAuth {
public:
explicit CheckEncryptionAction(ExecEnv* exec_env, TPrivilegeHier::type
hier,
diff --git a/be/test/service/http/check_encryption_action_test.cpp
b/be/test/service/http/check_encryption_action_test.cpp
new file mode 100644
index 00000000000..2767b27ce63
--- /dev/null
+++ b/be/test/service/http/check_encryption_action_test.cpp
@@ -0,0 +1,92 @@
+// 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 "service/http/action/check_encryption_action.h"
+
+#include <gen_cpp/olap_file.pb.h>
+#include <gtest/gtest.h>
+
+#include <memory>
+#include <mutex>
+#include <string>
+#include <vector>
+
+#include "cloud/cloud_storage_engine.h"
+#include "cloud/cloud_tablet.h"
+#include "storage/rowset/rowset.h"
+#include "storage/rowset/rowset_factory.h"
+#include "storage/rowset/rowset_meta.h"
+#include "storage/tablet/tablet_meta.h"
+
+namespace doris {
+
+class CheckEncryptionActionTest : public testing::Test {
+public:
+ CheckEncryptionActionTest() : _engine(CloudStorageEngine(EngineOptions
{})) {}
+
+ void SetUp() override {
+ auto tablet_meta = std::make_shared<TabletMeta>(
+ 1, 2, 15673, 15674, 4, 5, TTabletSchema(), 6,
+ std::unordered_map<uint32_t, uint32_t> {{7, 8}}, UniqueId(9,
10),
+ TTabletType::TABLET_TYPE_DISK, TCompressionType::LZ4F, 0,
false, std::nullopt,
+ "size_based", 1024, 1000, 3600, 5, 1,
TInvertedIndexFileStorageFormat::V2,
+ TEncryptionAlgorithm::AES256);
+ ASSERT_EQ(tablet_meta->encryption_algorithm(),
EncryptionAlgorithmPB::AES_256_CTR);
+ _tablet = std::make_shared<CloudTablet>(_engine,
std::move(tablet_meta));
+ }
+
+protected:
+ void add_rowset(Version version, int64_t num_segments, const std::string&
resource_id) {
+ auto rs_meta = std::make_shared<RowsetMeta>();
+ rs_meta->set_rowset_type(BETA_ROWSET);
+ rs_meta->set_version(version);
+ rs_meta->set_rowset_id(_engine.next_rowset_id());
+ rs_meta->set_num_segments(num_segments);
+ // A set resource id makes the rowset remote, so its fs comes from the
storage resource.
+ rs_meta->set_resource_id(resource_id);
+ RowsetSharedPtr rowset;
+ ASSERT_TRUE(RowsetFactory::create_rowset(nullptr, "", rs_meta,
&rowset).ok());
+ std::unique_lock wlock(_tablet->get_header_lock());
+ _tablet->add_rowsets({rowset}, false, wlock, false);
+ }
+
+ CloudStorageEngine _engine;
+ std::shared_ptr<CloudTablet> _tablet;
+};
+
+// A version hole is filled with an empty rowset copied from the previous
rowset's meta. In cloud
+// mode that may be the initial [0-1] rowset, whose resource id is empty, so
no fs can be resolved
+// for it. It has no file, so it must be skipped rather than fail the whole
check.
+TEST_F(CheckEncryptionActionTest, EmptyRowsetWithoutStorageResourceIsSkipped) {
+ add_rowset(Version(2, 2), 0, "");
+
+ auto res = is_tablet_encrypted(_tablet);
+ ASSERT_TRUE(res.has_value()) << res.error();
+ EXPECT_TRUE(res.value());
+}
+
+// A rowset whose files cannot be checked must surface the error, not be
reported as unencrypted.
+TEST_F(CheckEncryptionActionTest, UncheckableRowsetReportsError) {
+ add_rowset(Version(2, 2), 1, "not_exist_resource");
+
+ auto res = is_tablet_encrypted(_tablet);
+ ASSERT_FALSE(res.has_value()) << "unexpected value: " << res.value();
+ EXPECT_NE(res.error().to_string().find("failed to get fs for rowset"),
std::string::npos)
+ << res.error();
+}
+
+} // namespace doris
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
