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 0128b5fba2f branch-4.2: [fix](cloud) Get packed slice location through
the FileWriter interface #68106 (#69413)
0128b5fba2f is described below
commit 0128b5fba2fd4e811a403c032f2232f1c90e05d6
Author: Xin Liao <[email protected]>
AuthorDate: Fri Oct 23 10:01:58 2026 +0800
branch-4.2: [fix](cloud) Get packed slice location through the FileWriter
interface #68106 (#69413)
Pick apache/doris#68106
---
be/src/cloud/cloud_rowset_writer.cpp | 11 ++-------
be/src/cloud/delete_bitmap_file_writer.cpp | 3 +--
be/src/io/fs/file_writer.h | 12 ++++++---
be/src/io/fs/packed_file_manager.h | 13 +---------
be/src/io/fs/packed_file_writer.h | 5 +---
be/src/io/fs/packed_slice_location.h | 39 ++++++++++++++++++++++++++++++
be/test/io/fs/packed_file_system_test.cpp | 7 +++---
be/test/io/fs/packed_file_writer_test.cpp | 23 ++++++++++++++++++
8 files changed, 80 insertions(+), 33 deletions(-)
diff --git a/be/src/cloud/cloud_rowset_writer.cpp
b/be/src/cloud/cloud_rowset_writer.cpp
index 10540ece259..1b217a6ca8b 100644
--- a/be/src/cloud/cloud_rowset_writer.cpp
+++ b/be/src/cloud/cloud_rowset_writer.cpp
@@ -21,7 +21,6 @@
#include "common/status.h"
#include "io/cache/block_file_cache_factory.h"
#include "io/fs/packed_file_manager.h"
-#include "io/fs/packed_file_writer.h"
#include "storage/rowset/rowset_factory.h"
namespace doris {
@@ -208,18 +207,12 @@ Status
CloudRowsetWriter::_collect_packed_slice_location(io::FileWriter* file_wr
return Status::OK();
}
- // Check if file is actually in packed file (not direct write for large
files)
- if (!file_writer->is_in_packed_file()) {
- return Status::OK();
- }
-
// Ask the writer, which holds a reference to its own slice location.
Looking it up by
// path in PackedFileManager would race with the retention based cleanup
of the index.
io::PackedSliceLocation index;
- RETURN_IF_ERROR(
-
static_cast<io::PackedFileWriter*>(file_writer)->get_packed_slice_location(&index));
+ RETURN_IF_ERROR(file_writer->get_packed_slice_location(&index));
if (index.packed_file_path.empty()) {
- return Status::OK(); // File not in packed file, skip
+ return Status::OK(); // File not in packed file (e.g. direct write for
large files), skip
}
rowset_meta->add_packed_slice_location(file_path, index.packed_file_path,
index.offset,
diff --git a/be/src/cloud/delete_bitmap_file_writer.cpp
b/be/src/cloud/delete_bitmap_file_writer.cpp
index 59f0c6c0274..e8b8149f23b 100644
--- a/be/src/cloud/delete_bitmap_file_writer.cpp
+++ b/be/src/cloud/delete_bitmap_file_writer.cpp
@@ -98,9 +98,8 @@ Status DeleteBitmapFileWriter::close() {
// Check if file was written to packed file
if (_enable_packed_file) {
- auto* packed_writer =
static_cast<io::PackedFileWriter*>(_file_writer.get());
io::PackedSliceLocation loc;
- st = packed_writer->get_packed_slice_location(&loc);
+ st = _file_writer->get_packed_slice_location(&loc);
if (!st.ok()) {
LOG(WARNING) << "failed to get packed slice location for delete
bitmap file=" << _path
<< ", st=" << st.to_string();
diff --git a/be/src/io/fs/file_writer.h b/be/src/io/fs/file_writer.h
index 92e4475d637..1ab0d14745b 100644
--- a/be/src/io/fs/file_writer.h
+++ b/be/src/io/fs/file_writer.h
@@ -25,6 +25,7 @@
#include "io/cache/block_file_cache_factory.h"
#include "io/cache/file_cache_common.h"
#include "io/fs/file_reader_writer_fwd.h"
+#include "io/fs/packed_slice_location.h"
#include "io/fs/path.h"
#include "util/slice.h"
@@ -93,9 +94,14 @@ public:
virtual State state() const = 0;
- // Returns true if this file's data was written to a packed file.
- // Used to determine whether to collect packed slice location from
PackedFileManager.
- virtual bool is_in_packed_file() const { return false; }
+ // Gets the location of this file's slice in the packed file. Must be
called after the writer
+ // is closed. An empty packed_file_path means the file is not in a packed
file. Writers that
+ // wrap another writer must forward this call, so callers never need to
downcast to
+ // PackedFileWriter.
+ virtual Status get_packed_slice_location(PackedSliceLocation* location)
const {
+ *location = PackedSliceLocation {};
+ return Status::OK();
+ }
FileCacheAllocatorBuilder* cache_builder() const {
return _cache_builder == nullptr ? nullptr : _cache_builder.get();
diff --git a/be/src/io/fs/packed_file_manager.h
b/be/src/io/fs/packed_file_manager.h
index ab79d5bc7e4..46f26a2b12a 100644
--- a/be/src/io/fs/packed_file_manager.h
+++ b/be/src/io/fs/packed_file_manager.h
@@ -36,23 +36,12 @@
#include "common/status.h"
#include "io/fs/file_system.h"
#include "io/fs/file_writer.h"
+#include "io/fs/packed_slice_location.h"
#include "io/fs/path.h"
#include "util/slice.h"
namespace doris::io {
-struct PackedSliceLocation {
- std::string packed_file_path;
- int64_t offset;
- int64_t size;
- int64_t create_time = 0;
- int64_t tablet_id = 0;
- std::string rowset_id;
- std::string resource_id;
- int64_t txn_id = 0;
- int64_t packed_file_size = -1; // Total size of the packed file, -1 means
not set
-};
-
// Upload state of the packed file a slice belongs to
enum class PackedSliceUploadState : uint8_t {
PENDING = 0,
diff --git a/be/src/io/fs/packed_file_writer.h
b/be/src/io/fs/packed_file_writer.h
index a53d412e667..f571b188acb 100644
--- a/be/src/io/fs/packed_file_writer.h
+++ b/be/src/io/fs/packed_file_writer.h
@@ -55,10 +55,7 @@ public:
// Get merge file segment index information
// This method should be called after close(true) to get the index
information
// Returns empty index if file is not in merge file
- Status get_packed_slice_location(PackedSliceLocation* location) const;
-
- // Returns true if this file's data was written to a packed file (not
direct write)
- bool is_in_packed_file() const override { return !_is_direct_write; }
+ Status get_packed_slice_location(PackedSliceLocation* location) const
override;
#ifdef BE_TEST
size_t buffer_capacity_for_test() const { return _buffer.capacity(); }
diff --git a/be/src/io/fs/packed_slice_location.h
b/be/src/io/fs/packed_slice_location.h
new file mode 100644
index 00000000000..a6d9e48a49e
--- /dev/null
+++ b/be/src/io/fs/packed_slice_location.h
@@ -0,0 +1,39 @@
+// 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.
+
+#pragma once
+
+#include <cstdint>
+#include <string>
+
+namespace doris::io {
+
+// Location of a small file's slice inside a packed file.
+// An empty packed_file_path means the file is not in a packed file.
+struct PackedSliceLocation {
+ std::string packed_file_path;
+ int64_t offset;
+ int64_t size;
+ int64_t create_time = 0;
+ int64_t tablet_id = 0;
+ std::string rowset_id;
+ std::string resource_id;
+ int64_t txn_id = 0;
+ int64_t packed_file_size = -1; // Total size of the packed file, -1 means
not set
+};
+
+} // namespace doris::io
diff --git a/be/test/io/fs/packed_file_system_test.cpp
b/be/test/io/fs/packed_file_system_test.cpp
index 7902618d720..a5d37ee8f99 100644
--- a/be/test/io/fs/packed_file_system_test.cpp
+++ b/be/test/io/fs/packed_file_system_test.cpp
@@ -30,6 +30,7 @@
#include "io/fs/file_reader.h"
#include "io/fs/file_system.h"
#include "io/fs/file_writer.h"
+#include "io/fs/packed_file_writer.h"
#include "io/fs/path.h"
#include "util/slice.h"
@@ -247,7 +248,7 @@ TEST_F(PackedFileSystemTest,
FirstSegmentDataFileUsesPackedWriter) {
ASSERT_NE(_inner_fs->last_writer(), nullptr);
EXPECT_EQ(_inner_fs->last_writer()->bytes_appended(), 0);
- EXPECT_TRUE(writer->is_in_packed_file());
+ EXPECT_NE(dynamic_cast<PackedFileWriter*>(writer.get()), nullptr);
}
TEST_F(PackedFileSystemTest, LaterSegmentDataFileUsesDirectWriter) {
@@ -264,7 +265,7 @@ TEST_F(PackedFileSystemTest,
LaterSegmentDataFileUsesDirectWriter) {
ASSERT_NE(_inner_fs->last_writer(), nullptr);
EXPECT_EQ(_inner_fs->last_writer()->bytes_appended(), data.size());
- EXPECT_FALSE(writer->is_in_packed_file());
+ EXPECT_EQ(dynamic_cast<PackedFileWriter*>(writer.get()), nullptr);
}
TEST_F(PackedFileSystemTest, LaterSegmentIndexFileUsesDirectWriter) {
@@ -281,7 +282,7 @@ TEST_F(PackedFileSystemTest,
LaterSegmentIndexFileUsesDirectWriter) {
ASSERT_NE(_inner_fs->last_writer(), nullptr);
EXPECT_EQ(_inner_fs->last_writer()->bytes_appended(), data.size());
- EXPECT_FALSE(writer->is_in_packed_file());
+ EXPECT_EQ(dynamic_cast<PackedFileWriter*>(writer.get()), nullptr);
}
TEST_F(PackedFileSystemTest, OpenFileNotInMergeFile) {
diff --git a/be/test/io/fs/packed_file_writer_test.cpp
b/be/test/io/fs/packed_file_writer_test.cpp
index e26a5ec4743..d5b7dc2da79 100644
--- a/be/test/io/fs/packed_file_writer_test.cpp
+++ b/be/test/io/fs/packed_file_writer_test.cpp
@@ -275,4 +275,27 @@ TEST_F(PackedFileWriterTest, EmptyAppend) {
EXPECT_EQ(writer.bytes_appended(), 0);
}
+TEST_F(PackedFileWriterTest, GetPackedSliceLocationThroughFileWriterInterface)
{
+ // Callers only hold a FileWriter, which may be a wrapper around
PackedFileWriter,
+ // so the location must be reachable through the virtual interface.
+ MockFileWriterForMerge plain_writer("plain_file");
+ ASSERT_TRUE(plain_writer.close(false).ok());
+ FileWriter* plain = &plain_writer;
+ PackedSliceLocation location;
+ location.packed_file_path = "stale";
+ ASSERT_TRUE(plain->get_packed_slice_location(&location).ok());
+ EXPECT_TRUE(location.packed_file_path.empty());
+
+ PackedFileWriter packed_writer(std::move(_inner_writer),
Path("large_file"), _append_info);
+ FileWriter* writer = &packed_writer;
+ std::string data(150, 'x');
+ ASSERT_TRUE(writer->append(Slice(data)).ok());
+ ASSERT_TRUE(writer->close(false).ok());
+ ASSERT_EQ(writer->state(), FileWriter::State::CLOSED);
+
+ location.packed_file_path = "stale";
+ ASSERT_TRUE(writer->get_packed_slice_location(&location).ok());
+ EXPECT_TRUE(location.packed_file_path.empty());
+}
+
} // namespace doris::io
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
