This is an automated email from the ASF dual-hosted git repository.
hello-stephen 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 07e355865eb [fix](cloud) Fix schema change version hole test fixture
(#66669)
07e355865eb is described below
commit 07e355865eb5416c6be76b4d3af076f496153c21
Author: Gavin Chou <[email protected]>
AuthorDate: Thu Aug 13 10:21:31 2026 +0800
[fix](cloud) Fix schema change version hole test fixture (#66669)
## Proposed changes
Fix `CloudSchemaChangeJobTest.FillVersionHolesBeforeNewTabletRunning` by
making the test fixture match the current cloud schema change flow:
- inject the base tablet `[2-2]` rowset returned by the mocked schema
change `alter_version=2`
- provide a local-backed remote filesystem for cloud rowset writer
initialization
- mock `prepare_rowset`/`commit_rowset` for historical rowset conversion
- make zero-segment fixture rowsets internally consistent (`empty=true`,
`num_rows=0`, `newest_write_timestamp` set)
## Problem
The test mocked `prepare_tablet_job` with `alter_version=2`, but only
injected rowsets into the new tablet. Current master refreshes and reads
the base tablet historical rowsets for `[2, alter_version]`, so the test
failed with `failed to find path in version_graph. spec_version: 2-2`.
## Test intent and correctness
This regression models the schema change finalization path with the
following version layout:
- the FE request still carries `alter_version=1`
- MetaService returns the authoritative registered boundary `V1=2`
- the base tablet contains historical version `[2-2]`
- the new tablet contains the placeholder `[0-1]` and a post-V1 rowset
`[4-4]`, leaving version 3 missing
- schema change converts `[2-2]`, then finalization must create the
`[3-3]` hole rowset so that `[3,4]` is capturable before the tablet is
exposed as RUNNING
The added base `[2-2]` rowset is a required precondition, not the
expected result being injected. A registered `V1=2` means the base
tablet must provide a continuous path through version 2. The old fixture
returned `V1=2` while leaving the base tablet empty, which is
inconsistent with the production contract and only happened to work
while the capture gate incorrectly used the stale FE request version.
The test oracle remains independent from the fixture repair:
- the mock injects `[0-1]`, `[2-2]`, and `[4-4]`, but never injects the
target `[3-3]`
- `[3-3]` can only be produced by the production `fill_version_holes()`
path
- the test verifies that `[3-3]` is both empty and marked as a hole
rowset
- it also verifies that `capture_consistent_versions([3,4])` returns
`[3-3]` followed by `[4-4]`
- removing the production hole-filling call makes these assertions fail
The zero-row, zero-segment base rowset is intentional and internally
consistent. This test focuses on version-graph finalization rather than
payload transformation, so it uses a valid empty historical rowset
without requiring physical segment files. The local-backed remote
filesystem and `prepare_rowset`/`commit_rowset` callbacks only provide
the infrastructure now required because the registered V1 correctly
enters historical conversion; they do not create the hole under test.
This PR changes only the fixture. It does not change production
behavior. The existing test validates the observable graph repair after
schema change finalization; it is not intended to validate row-content
transformation or add a new concurrency/order oracle.
## Validation
- `sh format_code.sh be/test/cloud/cloud_schema_change_job_test.cpp`
- `sh run-be-ut.sh --run --filter
"CloudSchemaChangeJobTest.FillVersionHolesBeforeNewTabletRunning"`
- `sh run-be-ut.sh --run --filter "CloudSchemaChangeJobTest.*"`
Jira: DORIS-27883
Co-authored-by: gavinchou <[email protected]>
---
be/test/cloud/cloud_schema_change_job_test.cpp | 101 ++++++++++++++++++++++++-
1 file changed, 98 insertions(+), 3 deletions(-)
diff --git a/be/test/cloud/cloud_schema_change_job_test.cpp
b/be/test/cloud/cloud_schema_change_job_test.cpp
index 74b27bed7ad..9afce0bfb5c 100644
--- a/be/test/cloud/cloud_schema_change_job_test.cpp
+++ b/be/test/cloud/cloud_schema_change_job_test.cpp
@@ -30,6 +30,8 @@
#include "cloud/cloud_tablet_mgr.h"
#include "common/status.h"
#include "cpp/sync_point.h"
+#include "io/fs/local_file_system.h"
+#include "io/fs/remote_file_system.h"
#include "json2pb/json_to_pb.h"
#include "storage/rowset/rowset_factory.h"
#include "storage/rowset/rowset_meta.h"
@@ -38,6 +40,78 @@
namespace doris {
+namespace {
+
+class LocalRemoteFileSystem final : public io::RemoteFileSystem {
+public:
+ explicit LocalRemoteFileSystem(std::string root_path)
+ : RemoteFileSystem(std::move(root_path),
"cloud_schema_change_job_test_fs",
+ io::FileSystemType::BROKER) {}
+
+private:
+ Status create_file_impl(const io::Path& file, io::FileWriterPtr* writer,
+ const io::FileWriterOptions* opts) override {
+ return io::global_local_filesystem()->create_file(file, writer, opts);
+ }
+
+ Status create_directory_impl(const io::Path& dir, bool failed_if_exists)
override {
+ return io::global_local_filesystem()->create_directory(dir,
failed_if_exists);
+ }
+
+ Status delete_file_impl(const io::Path& file) override {
+ return io::global_local_filesystem()->delete_file(file);
+ }
+
+ Status batch_delete_impl(const std::vector<io::Path>& files) override {
+ return io::global_local_filesystem()->batch_delete(files);
+ }
+
+ Status delete_directory_impl(const io::Path& dir) override {
+ return io::global_local_filesystem()->delete_directory(dir);
+ }
+
+ Status exists_impl(const io::Path& path, bool* res) const override {
+ return io::global_local_filesystem()->exists(path, res);
+ }
+
+ Status file_size_impl(const io::Path& file, int64_t* file_size) const
override {
+ return io::global_local_filesystem()->file_size(file, file_size);
+ }
+
+ Status list_impl(const io::Path& dir, bool only_file,
std::vector<io::FileInfo>* files,
+ bool* exists) override {
+ return io::global_local_filesystem()->list(dir, only_file, files,
exists);
+ }
+
+ Status rename_impl(const io::Path& orig_name, const io::Path& new_name)
override {
+ return io::global_local_filesystem()->rename(orig_name, new_name);
+ }
+
+ Status upload_impl(const io::Path& local_file, const io::Path&
remote_file) override {
+ return io::global_local_filesystem()->link_file(local_file,
remote_file);
+ }
+
+ Status batch_upload_impl(const std::vector<io::Path>& local_files,
+ const std::vector<io::Path>& remote_files)
override {
+ DCHECK_EQ(local_files.size(), remote_files.size());
+ for (size_t i = 0; i < local_files.size(); ++i) {
+ RETURN_IF_ERROR(upload_impl(local_files[i], remote_files[i]));
+ }
+ return Status::OK();
+ }
+
+ Status download_impl(const io::Path& remote_file, const io::Path&
local_file) override {
+ return io::global_local_filesystem()->link_file(remote_file,
local_file);
+ }
+
+ Status open_file_internal(const io::Path& file, io::FileReaderSPtr* reader,
+ const io::FileReaderOptions& opts) override {
+ return io::global_local_filesystem()->open_file(file, reader, &opts);
+ }
+};
+
+} // namespace
+
class CloudSchemaChangeJobTest : public testing::Test {
public:
CloudSchemaChangeJobTest() : _engine(CloudStorageEngine(EngineOptions {}))
{}
@@ -46,6 +120,7 @@ public:
_cluster_info = std::make_shared<CloudClusterInfo>();
_cluster_info->_is_in_standby = false;
ExecEnv::GetInstance()->_cluster_info = _cluster_info.get();
+ _engine.set_latest_fs(std::make_shared<LocalRemoteFileSystem>("/tmp"));
_json_rowset_meta = R"({
"rowset_id": 540081,
@@ -56,11 +131,11 @@ public:
"rowset_state": "VISIBLE",
"start_version": 2,
"end_version": 2,
- "num_rows": 100,
+ "num_rows": 0,
"total_disk_size": 41,
"data_disk_size": 41,
"index_disk_size": 235,
- "empty": false,
+ "empty": true,
"load_id": {
"hi": -5350970832824939812,
"lo": -6717994719194512122
@@ -87,6 +162,7 @@ protected:
auto rs_meta = std::make_shared<RowsetMeta>();
rs_meta->init_from_pb(pb);
rs_meta->set_tablet_schema(schema);
+ rs_meta->set_newest_write_timestamp(1553765670);
RowsetSharedPtr rowset;
static_cast<void>(RowsetFactory::create_rowset(schema, "", rs_meta,
&rowset));
return rowset;
@@ -112,8 +188,10 @@ TEST_F(CloudSchemaChangeJobTest,
FillVersionHolesBeforeNewTabletRunning) {
auto new_tablet = std::make_shared<CloudTablet>(_engine,
std::move(new_meta));
static_cast<void>(new_tablet->set_tablet_state(TABLET_NOTREADY));
+ auto base_rowset = create_rowset(base_tablet->tablet_schema(),
base_tablet_id, 2, 2);
auto placeholder = create_rowset(new_tablet->tablet_schema(),
new_tablet_id, 0, 1);
auto rowset_after_hole = create_rowset(new_tablet->tablet_schema(),
new_tablet_id, 4, 4);
+ ASSERT_NE(base_rowset, nullptr);
ASSERT_NE(placeholder, nullptr);
ASSERT_NE(rowset_after_hole, nullptr);
@@ -135,7 +213,12 @@ TEST_F(CloudSchemaChangeJobTest,
FillVersionHolesBeforeNewTabletRunning) {
CloudTablet* loaded_new_tablet = nullptr;
sp->set_call_back("CloudMetaMgr::sync_tablet_rowsets", [&](auto&& outcome)
{
auto* tablet = try_any_cast<CloudTablet*>(outcome[0]);
- if (tablet->tablet_id() == new_tablet_id) {
+ if (tablet->tablet_id() == base_tablet_id) {
+ std::unique_lock lock(tablet->get_header_lock());
+ if (!tablet->rowset_map().count(Version(2, 2))) {
+ tablet->add_rowsets({base_rowset}, false, lock, false);
+ }
+ } else if (tablet->tablet_id() == new_tablet_id) {
loaded_new_tablet = tablet;
std::unique_lock lock(tablet->get_header_lock());
std::vector<RowsetSharedPtr> rowsets;
@@ -162,6 +245,18 @@ TEST_F(CloudSchemaChangeJobTest,
FillVersionHolesBeforeNewTabletRunning) {
resp->set_alter_version(2);
});
+ sp->set_call_back("CloudMetaMgr::prepare_rowset", [](auto&& outcome) {
+ auto* pairs = try_any_cast_ret<Status>(outcome);
+ pairs->second = true;
+ pairs->first = Status::OK();
+ });
+
+ sp->set_call_back("CloudMetaMgr::commit_rowset", [](auto&& outcome) {
+ auto* pairs = try_any_cast_ret<Status>(outcome);
+ pairs->second = true;
+ pairs->first = Status::OK();
+ });
+
bool commit_called = false;
sp->set_call_back("CloudMetaMgr::commit_tablet_job", [&](auto&& outcome) {
commit_called = true;
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
