This is an automated email from the ASF dual-hosted git repository.
liaoxin01 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 91e566eaa9d [fix](cloud) Resolve packed segments when checking tablet
encryption (#68063)
91e566eaa9d is described below
commit 91e566eaa9d54c6c5e8ea7799192df4db4b1632e
Author: Xin Liao <[email protected]>
AuthorDate: Thu Sep 17 00:31:08 2026 +0800
[fix](cloud) Resolve packed segments when checking tablet encryption
(#68063)
### What problem does this PR solve?
Problem Summary:
When small files are packed (`enable_packed_file`, on by default in
cloud mode
on S3), a rowset's segment no longer exists as a standalone object: its
bytes
are a slice inside a shared packed object, and the mapping from the
segment
path to that slice lives in `RowsetMetaPB.packed_slice_locations`.
`GET /api/check_tablet_encryption?tablet_id=<id>&get_footer=true` fails
on such
a tablet with HTTP 500 and
NOT_FOUND: failed to head s3 file
<endpoint>/<bucket>/data/<tablet_id>/<rowset_id>_0.dat
Root cause: `CheckEncryptionAction` opens segments through
`RowsetMeta::physical_fs()`, which is the bare remote file system. The
`PackedFileSystem` wrapper that resolves a segment path to its slice is
only
applied in `RowsetMeta::fs()`, so the action ends up asking S3 for an
object
that was never written. It cannot simply switch to `fs()` either: `fs()`
also
layers encryption on top, which would decrypt the segment and hide the
very
magic code and `FileEncryptionInfoPB` footer this check inspects.
Fix: extract the packed wrapping out of `RowsetMeta::fs()` into a helper
and
expose `RowsetMeta::packed_physical_fs()` - packed-aware, but without
the
encryption layer - then use it at both read sites in
`CheckEncryptionAction`.
`PackedFileReader` reports the slice size and takes slice-relative
offsets, so
the existing footer read logic keeps working unchanged. After the fix
the
action returns the footer for packed and non-packed tablets alike.
Two defects found while reading the same code are fixed as well:
- `get_last_encrypt_footer()` dereferenced the file system without the
null
check its sibling `is_tablet_encrypted()` has, so a rowset whose storage
resource cannot be resolved crashed the BE instead of returning an
error.
- The footer length buffer used `reserve()` where `resize()` was meant,
writing
8 bytes past the end of an empty vector.
---
.../http/action/check_encryption_action.cpp | 78 ++++++++++++++---
be/src/storage/rowset/rowset_meta.cpp | 59 +++++++------
be/src/storage/rowset/rowset_meta.h | 15 ++++
be/test/storage/rowset/rowset_meta_test.cpp | 98 ++++++++++++++++++++++
4 files changed, 215 insertions(+), 35 deletions(-)
diff --git a/be/src/service/http/action/check_encryption_action.cpp
b/be/src/service/http/action/check_encryption_action.cpp
index df395622edc..82d77e04f49 100644
--- a/be/src/service/http/action/check_encryption_action.cpp
+++ b/be/src/service/http/action/check_encryption_action.cpp
@@ -47,6 +47,12 @@ namespace doris {
const std::string GET_FOOTER = "get_footer";
+// An encrypted file ends with a fixed-size footer region laid out as
+// [1 byte][uint64 encryption info length][encryption info][padding][8 byte
magic code].
+constexpr size_t ENCRYPTION_FOOTER_SIZE = 256;
+constexpr size_t MAX_ENCRYPTION_INFO_SIZE =
+ ENCRYPTION_FOOTER_SIZE - sizeof(uint8_t) - sizeof(uint64_t);
+
CheckEncryptionAction::CheckEncryptionAction(ExecEnv* exec_env,
TPrivilegeHier::type hier,
TPrivilegeType::type type)
: HttpHandlerWithAuth(exec_env, hier, type) {}
@@ -68,7 +74,11 @@ Result<bool> is_tablet_encrypted(const BaseTabletSPtr&
tablet) {
rs_meta->end_version() == 1) {
return;
}
- auto fs = rs_meta->physical_fs();
+ // 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
+ // encryption footer this check is looking for.
+ auto fs = rs_meta->packed_physical_fs();
if (fs == nullptr) {
st = Status::InternalError("failed to get fs for rowset:
tablet={}, rs={}",
tablet->tablet_id(),
rs->rowset_id().to_string());
@@ -84,22 +94,27 @@ Result<bool> is_tablet_encrypted(const BaseTabletSPtr&
tablet) {
return;
}
- std::vector<std::string_view> file_paths;
+ // Owning strings: the V2 index path is built here and must outlive
the loop below.
+ std::vector<std::string> file_paths;
const auto& first_seg_path = maybe_seg_path.value();
file_paths.emplace_back(first_seg_path);
if (tablet->tablet_schema()->has_inverted_index() &&
tablet->tablet_schema()->get_inverted_index_storage_format() ==
V2) {
- std::string inverted_index_file_path =
InvertedIndexDescriptor::get_index_file_path_v2(
-
InvertedIndexDescriptor::get_index_file_path_prefix(first_seg_path));
- file_paths.emplace_back(inverted_index_file_path);
+
file_paths.emplace_back(InvertedIndexDescriptor::get_index_file_path_v2(
+
InvertedIndexDescriptor::get_index_file_path_prefix(first_seg_path)));
}
- for (const auto path : file_paths) {
+ for (const auto& path : file_paths) {
io::FileReaderSPtr reader;
st = fs->open_file(path, &reader);
if (!st) {
return;
}
+ if (reader->size() < sizeof(uint64_t)) {
+ st = Status::Corruption("file is too small to hold a magic
code: path={}, size={}",
+ path, reader->size());
+ return;
+ }
std::vector<uint8_t> magic_code_buf;
magic_code_buf.resize(sizeof(uint64_t));
Slice magic_code(magic_code_buf.data(), sizeof(uint64_t));
@@ -108,6 +123,12 @@ Result<bool> is_tablet_encrypted(const BaseTabletSPtr&
tablet) {
if (!st) {
return;
}
+ if (bytes_read != magic_code.size) {
+ st = Status::Corruption(
+ "short read of the magic code: path={}, expected={},
got={}", path,
+ magic_code.size, bytes_read);
+ return;
+ }
std::vector<uint8_t> answer = {'A', 'B', 'C', 'D', 'E', 'A', 'B',
'C'};
is_encrypted &= Slice::mem_equal(answer.data(), magic_code.data,
magic_code.size);
@@ -137,23 +158,58 @@ Result<std::string> get_last_encrypt_footer(const
BaseTabletSPtr& tablet) {
if (config::is_cloud_mode() && rs_meta->start_version() == 0 &&
rs_meta->end_version() == 1) {
return "{}";
}
- auto fs = rs_meta->physical_fs();
+ // See the comment in `is_tablet_encrypted()` for why this is neither
`physical_fs()`
+ // nor `fs()`.
+ auto fs = rs_meta->packed_physical_fs();
+ if (fs == nullptr) {
+ return ResultError(Status::InternalError("failed to get fs for rowset:
tablet={}, rs={}",
+ tablet->tablet_id(),
rs->rowset_id().to_string()));
+ }
io::FileReaderSPtr reader;
RETURN_IF_ERROR_RESULT(fs->open_file(maybe_seg_path.value(), &reader));
+ // Every offset below is relative to the end of the file, so a file
shorter than the footer
+ // region makes them underflow. On a packed slice such an underflow does
not fail the read:
+ // it wraps into a neighbouring slice, whose bytes would then be decoded
as this segment's
+ // footer.
+ if (reader->size() < ENCRYPTION_FOOTER_SIZE) {
+ return ResultError(Status::Corruption(
+ "file is too small to hold an encryption footer: path={},
size={}",
+ maybe_seg_path.value(), reader->size()));
+ }
+ const size_t footer_offset = reader->size() - ENCRYPTION_FOOTER_SIZE;
+
std::vector<uint8_t> pb_len_buf;
- pb_len_buf.reserve(sizeof(uint64_t));
+ pb_len_buf.resize(sizeof(uint64_t));
Slice pb_len_slice(pb_len_buf.data(), sizeof(uint64_t));
size_t bytes_read;
RETURN_IF_ERROR_RESULT(
- reader->read_at(reader->size() - 256 + sizeof(uint8_t),
pb_len_slice, &bytes_read));
+ reader->read_at(footer_offset + sizeof(uint8_t), pb_len_slice,
&bytes_read));
+ if (bytes_read != pb_len_slice.size) {
+ return ResultError(Status::Corruption(
+ "short read of the encryption info length: path={},
expected={}, got={}",
+ maybe_seg_path.value(), pb_len_slice.size, bytes_read));
+ }
auto info_pb_size = decode_fixed64_le(pb_len_buf.data());
+ // `get_footer=true` reaches this parser even when the scan above found
unencrypted files, so
+ // the decoded length may well be arbitrary bytes. It can never exceed
what the footer holds.
+ if (info_pb_size > MAX_ENCRYPTION_INFO_SIZE) {
+ return ResultError(Status::Corruption(
+ "encryption info length {} exceeds the {} bytes available in
the footer: path={}",
+ info_pb_size, MAX_ENCRYPTION_INFO_SIZE,
maybe_seg_path.value()));
+ }
+
std::vector<uint8_t> info_pb_buf;
info_pb_buf.resize(info_pb_size);
Slice pb_slice(info_pb_buf.data(), info_pb_size);
- RETURN_IF_ERROR_RESULT(reader->read_at(
- reader->size() - 256 + sizeof(uint8_t) + sizeof(uint64_t),
pb_slice, &bytes_read));
+ RETURN_IF_ERROR_RESULT(reader->read_at(footer_offset + sizeof(uint8_t) +
sizeof(uint64_t),
+ pb_slice, &bytes_read));
+ if (bytes_read != pb_slice.size) {
+ return ResultError(Status::Corruption(
+ "short read of the encryption info: path={}, expected={},
got={}",
+ maybe_seg_path.value(), pb_slice.size, bytes_read));
+ }
FileEncryptionInfoPB info_pb;
if (!info_pb.ParseFromArray(info_pb_buf.data(),
static_cast<int>(info_pb_buf.size()))) {
diff --git a/be/src/storage/rowset/rowset_meta.cpp
b/be/src/storage/rowset/rowset_meta.cpp
index e1259bd4c0f..05ca4be46b7 100644
--- a/be/src/storage/rowset/rowset_meta.cpp
+++ b/be/src/storage/rowset/rowset_meta.cpp
@@ -127,6 +127,39 @@ io::FileSystemSPtr RowsetMeta::physical_fs() {
}
}
+io::FileSystemSPtr RowsetMeta::_wrap_packed_fs(io::FileSystemSPtr fs) {
+ if (fs == nullptr || _rowset_meta_pb.packed_slice_locations_size() == 0) {
+ return fs;
+ }
+
+ std::unordered_map<std::string, io::PackedSliceLocation> index_map;
+ for (const auto& [path, index_pb] :
_rowset_meta_pb.packed_slice_locations()) {
+ io::PackedSliceLocation index;
+ index.packed_file_path = index_pb.packed_file_path();
+ index.offset = index_pb.offset();
+ index.size = index_pb.size();
+ index.packed_file_size = index_pb.has_packed_file_size() ?
index_pb.packed_file_size() : -1;
+ index.tablet_id = tablet_id();
+ index.rowset_id = _rowset_id.to_string();
+ index.resource_id = fs->id();
+ index_map[path] = index;
+ }
+ if (index_map.empty()) {
+ return fs;
+ }
+
+ io::PackedAppendContext append_info;
+ append_info.tablet_id = tablet_id();
+ append_info.rowset_id = _rowset_id.to_string();
+ append_info.txn_id = txn_id();
+ return std::make_shared<io::PackedFileSystem>(std::move(fs),
std::move(index_map),
+ std::move(append_info));
+}
+
+io::FileSystemSPtr RowsetMeta::packed_physical_fs() {
+ return _wrap_packed_fs(physical_fs());
+}
+
io::FileSystemSPtr RowsetMeta::fs() {
auto fs = physical_fs();
@@ -145,30 +178,8 @@ io::FileSystemSPtr RowsetMeta::fs() {
return nullptr;
}
- // Apply packed file system first if enabled and index_map is not empty
- io::FileSystemSPtr wrapped = fs;
- if (_rowset_meta_pb.packed_slice_locations_size() > 0) {
- std::unordered_map<std::string, io::PackedSliceLocation> index_map;
- for (const auto& [path, index_pb] :
_rowset_meta_pb.packed_slice_locations()) {
- io::PackedSliceLocation index;
- index.packed_file_path = index_pb.packed_file_path();
- index.offset = index_pb.offset();
- index.size = index_pb.size();
- index.packed_file_size =
- index_pb.has_packed_file_size() ?
index_pb.packed_file_size() : -1;
- index.tablet_id = tablet_id();
- index.rowset_id = _rowset_id.to_string();
- index.resource_id = wrapped->id();
- index_map[path] = index;
- }
- if (!index_map.empty()) {
- io::PackedAppendContext append_info;
- append_info.tablet_id = tablet_id();
- append_info.rowset_id = _rowset_id.to_string();
- append_info.txn_id = txn_id();
- wrapped = std::make_shared<io::PackedFileSystem>(wrapped,
index_map, append_info);
- }
- }
+ // Apply packed file system first
+ io::FileSystemSPtr wrapped = _wrap_packed_fs(std::move(fs));
// Then apply encryption on top
wrapped = io::make_file_system(wrapped, algorithm.value());
diff --git a/be/src/storage/rowset/rowset_meta.h
b/be/src/storage/rowset/rowset_meta.h
index 9aa4b471f0c..9255cdb9a09 100644
--- a/be/src/storage/rowset/rowset_meta.h
+++ b/be/src/storage/rowset/rowset_meta.h
@@ -72,8 +72,19 @@ public:
// Note that if the resource id cannot be found for the corresponding
remote file system, nullptr will be returned.
MOCK_FUNCTION io::FileSystemSPtr fs();
+ // The bare file system holding this rowset's files. It resolves neither
packed files nor
+ // encryption, so it cannot open a segment whose bytes live inside a
packed object. Prefer
+ // `fs()`, or `packed_physical_fs()` when the raw bytes are what you are
after.
io::FileSystemSPtr physical_fs();
+ // Same as `physical_fs()`, but additionally wrapped with
`PackedFileSystem` when this
+ // rowset's files are packed into shared objects, so that segment/index
paths still
+ // resolve. Unlike `fs()`, no encryption layer is applied, i.e. reads
return the raw
+ // on-disk bytes. Callers that inspect the physical layout of a file
(encryption footer,
+ // magic code, ...) must use this instead of `physical_fs()`, otherwise
packed files
+ // cannot be opened at all.
+ io::FileSystemSPtr packed_physical_fs();
+
Result<const StorageResource*> remote_storage_resource();
void set_remote_storage_resource(StorageResource resource);
@@ -572,6 +583,10 @@ public:
}
private:
+ // Wraps `fs` with `PackedFileSystem` if this rowset has packed slice
locations,
+ // otherwise returns `fs` unchanged.
+ io::FileSystemSPtr _wrap_packed_fs(io::FileSystemSPtr fs);
+
bool _deserialize_from_pb(std::string_view value);
bool _serialize_to_pb(std::string* value);
diff --git a/be/test/storage/rowset/rowset_meta_test.cpp
b/be/test/storage/rowset/rowset_meta_test.cpp
index 45ca3a5f251..633c4f4999a 100644
--- a/be/test/storage/rowset/rowset_meta_test.cpp
+++ b/be/test/storage/rowset/rowset_meta_test.cpp
@@ -31,9 +31,14 @@
#include "common/status.h"
#include "cpp/sync_point.h"
#include "gtest/gtest_pred_impl.h"
+#include "io/fs/file_reader.h"
+#include "io/fs/file_system.h"
+#include "io/fs/local_file_system.h"
#include "storage/olap_common.h"
#include "storage/olap_meta.h"
#include "storage/tablet/tablet_schema.h"
+#include "util/defer_op.h"
+#include "util/slice.h"
using ::testing::_;
using ::testing::Return;
@@ -683,4 +688,97 @@ TEST_F(RowsetMetaTest, TestSegmentMetaView) {
EXPECT_EQ(segment_ids, std::vector<int64_t>({0, 2, 5}));
}
+TEST_F(RowsetMetaTest, TestPackedPhysicalFs) {
+ RowsetMeta rowset_meta;
+ EXPECT_TRUE(rowset_meta.init_from_json(_json_rowset_meta));
+
+ // Without packed slice locations there is nothing to map, so the physical
fs is returned
+ // unchanged.
+ EXPECT_EQ(rowset_meta.packed_physical_fs(), rowset_meta.physical_fs());
+
+ // Lay out a packed file the way PackedFileWriter does: the segment bytes
live at a
+ // non-zero offset inside a shared object, and no object exists at the
segment path.
+ const std::string packed_file_path = "./packed_file.dat";
+ const std::string padding(7, 'x');
+ const std::string segment_content = "segment-payload";
+ {
+ std::ofstream out(packed_file_path, std::ios::binary);
+ out << padding << segment_content;
+ }
+ Defer defer {[&]() {
static_cast<void>(std::filesystem::remove(packed_file_path)); }};
+
+ const std::string segment_path = "./data/15673/540081_0.dat";
+ io::FileReaderSPtr reader;
+ EXPECT_FALSE(rowset_meta.physical_fs()->open_file(segment_path,
&reader).ok());
+
+ rowset_meta.add_packed_slice_location(
+ segment_path, packed_file_path, cast_set<int64_t>(padding.size()),
+ cast_set<int64_t>(segment_content.size()),
+ cast_set<int64_t>(padding.size() + segment_content.size()));
+
+ // The packed-aware fs resolves the segment path to its slice, while still
handing out the
+ // raw bytes (no decryption layer on top).
+ auto fs = rowset_meta.packed_physical_fs();
+ ASSERT_NE(fs, nullptr);
+ ASSERT_TRUE(fs->open_file(segment_path, &reader).ok());
+
+ // Size and offsets stay slice-relative, so callers can address the
segment as if it were a
+ // standalone file.
+ EXPECT_EQ(reader->size(), segment_content.size());
+ std::string buf(segment_content.size(), '\0');
+ size_t bytes_read = 0;
+ ASSERT_TRUE(reader->read_at(0, Slice(buf.data(), buf.size()),
&bytes_read).ok());
+ EXPECT_EQ(bytes_read, segment_content.size());
+ EXPECT_EQ(buf, segment_content);
+
+ // A path with no packed slice location falls through to the physical fs,
so non-packed
+ // rowsets and files keep their previous behaviour.
+ io::FileReaderSPtr missing;
+ EXPECT_FALSE(fs->open_file("./data/15673/540081_1.dat", &missing).ok());
+}
+
+TEST_F(RowsetMetaTest, TestPackedPhysicalFsResolvesV2IndexFile) {
+ RowsetMeta rowset_meta;
+ EXPECT_TRUE(rowset_meta.init_from_json(_json_rowset_meta));
+
+ // A V2 inverted index file is packed alongside its segment, as its own
slice of the same
+ // packed object. `is_tablet_encrypted()` reads both, so both must resolve.
+ const std::string packed_file_path = "./packed_file_with_index.dat";
+ const std::string segment_content = "segment-payload";
+ const std::string index_content = "index-payload";
+ {
+ std::ofstream out(packed_file_path, std::ios::binary);
+ out << segment_content << index_content;
+ }
+ Defer defer {[&]() {
static_cast<void>(std::filesystem::remove(packed_file_path)); }};
+
+ const std::string segment_path = "./data/15673/540081_0.dat";
+ const std::string index_path = "./data/15673/540081_0.idx";
+ const int64_t packed_file_size =
+ cast_set<int64_t>(segment_content.size() + index_content.size());
+ rowset_meta.add_packed_slice_location(segment_path, packed_file_path, 0,
+
cast_set<int64_t>(segment_content.size()),
+ packed_file_size);
+ rowset_meta.add_packed_slice_location(
+ index_path, packed_file_path,
cast_set<int64_t>(segment_content.size()),
+ cast_set<int64_t>(index_content.size()), packed_file_size);
+
+ auto fs = rowset_meta.packed_physical_fs();
+ ASSERT_NE(fs, nullptr);
+
+ auto read_all = [&](const std::string& path, size_t expected_size) {
+ io::FileReaderSPtr reader;
+ EXPECT_TRUE(fs->open_file(path, &reader).ok());
+ EXPECT_EQ(reader->size(), expected_size);
+ std::string buf(expected_size, '\0');
+ size_t bytes_read = 0;
+ EXPECT_TRUE(reader->read_at(0, Slice(buf.data(), buf.size()),
&bytes_read).ok());
+ EXPECT_EQ(bytes_read, expected_size);
+ return buf;
+ };
+
+ EXPECT_EQ(read_all(segment_path, segment_content.size()), segment_content);
+ EXPECT_EQ(read_all(index_path, index_content.size()), index_content);
+}
+
} // namespace doris
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
