Skip to content

tests: Add unittest about tagging under disagg arch #10759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
JaySon-Huang wants to merge 2 commits into
base: master
Choose a base branch
from
Open

tests: Add unittest about tagging under disagg arch #10759

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8d2b951
tests: guard deleteBucket calls behind mocked S3 check
JaySon-Huang Mar 20, 2026
f7da65f
tests: Add simple test about set/get tagging
JaySon-Huang Mar 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion dbms/src/IO/Encryption/tests/gtest_keyspaces_key_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ class KeyspacesKeyManagerTest : public DB::base::TiFlashStorageTestBasic
}

protected:
void deleteBucket() { ::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client); }
void deleteBucket()
{
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
}

protected:
FileProviderPtr file_provider;
Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/DeltaMerge/File/tests/gtest_dm_meta_version.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,8 @@ class S3DMFile
auto & global_context = db_context->getGlobalContext();
global_context.getSharedContextDisagg()->remote_data_store = nullptr;
auto s3_client = S3::ClientFactory::instance().sharedTiFlashClient();
DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
DB::tests::TiFlashTestEnv::disableS3Config();
}

Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/DeltaMerge/Index/InvertedIndex/tests/gtest_dm_inverted_index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,8 @@ class InvertedIndexColumnFileTinyTest : public DB::base::TiFlashStorageTestBasic
global_context.setPageStorageRunMode(orig_mode);

auto s3_client = S3::ClientFactory::instance().sharedTiFlashClient();
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
DB::tests::TiFlashTestEnv::disableS3Config();
}

Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/DeltaMerge/Index/VectorIndex/tests/gtest_dm_vector_index.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1961,7 +1961,8 @@ class VectorIndexSegmentOnS3Test
global_context.setPageStorageRunMode(orig_mode);

auto s3_client = S3::ClientFactory::instance().sharedTiFlashClient();
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
DB::tests::TiFlashTestEnv::disableS3Config();
}

Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/DeltaMerge/tests/gtest_dm_delta_merge_store_fast_add_peer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ class DeltaMergeStoreTestFastAddPeer
global_context.setPageStorageRunMode(orig_mode);
}
auto s3_client = S3::ClientFactory::instance().sharedTiFlashClient();
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
DB::tests::TiFlashTestEnv::disableS3Config();
setStorageFormat(current_format_version);
}
Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/DeltaMerge/tests/gtest_dm_segment_s3.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@ class SegmentTestS3 : public DB::base::TiFlashStorageTestBasic
global_context.setPageStorageRunMode(orig_mode);
}
auto s3_client = S3::ClientFactory::instance().sharedTiFlashClient();
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
DB::tests::TiFlashTestEnv::disableS3Config();
}

Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/DeltaMerge/tests/gtest_segment_replace_stable_data.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,8 @@ class SegmentReplaceStableDataDisaggregated
global_context.setPageStorageRunMode(orig_mode);

auto s3_client = S3::ClientFactory::instance().sharedTiFlashClient();
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
DB::tests::TiFlashTestEnv::disableS3Config();

STORAGE_FORMAT_CURRENT = storage_version;
Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ class RegionKVStoreTestFAP : public KVStoreTestBase
global_context.setPageStorageRunMode(orig_mode);
}
auto s3_client = S3::ClientFactory::instance().sharedTiFlashClient();
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
DB::tests::TiFlashTestEnv::disableS3Config();
}

Expand Down
3 changes: 2 additions & 1 deletion dbms/src/Storages/Page/V3/Universal/tests/gtest_lock_local_mgr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ class S3LockLocalManagerTest : public testing::Test

void SetUp() override
{
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
::DB::tests::TiFlashTestEnv::createBucketIfNotExist(*s3_client);
}

Expand Down
6 changes: 5 additions & 1 deletion dbms/src/Storages/Page/V3/Universal/tests/gtest_remote_read.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,11 @@ class UniPageStorageRemoteReadTest
}

protected:
void deleteBucket() { ::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client); }
void deleteBucket()
{
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
::DB::tests::TiFlashTestEnv::deleteBucket(*s3_client);
}

protected:
StoreID test_store_id = 1234;
Expand Down
18 changes: 18 additions & 0 deletions dbms/src/Storages/S3/S3Common.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
#include <aws/s3/model/DeleteObjectRequest.h>
#include <aws/s3/model/ExpirationStatus.h>
#include <aws/s3/model/GetObjectRequest.h>
#include <aws/s3/model/GetObjectTaggingRequest.h>
#include <aws/s3/model/HeadObjectRequest.h>
#include <aws/s3/model/ListObjectsRequest.h>
#include <aws/s3/model/ListObjectsV2Request.h>
Expand Down Expand Up @@ -825,6 +826,23 @@ void rewriteObjectWithTagging(const TiFlashS3Client & client, const String & key
LOG_DEBUG(client.log, "rewrite object key={} cost={:.2f}s", key, elapsed_seconds);
}

Aws::S3::Model::GetObjectTaggingResult getObjectTagging(const TiFlashS3Client & client, const String & key)
{
Aws::S3::Model::GetObjectTaggingRequest req;
client.setBucketAndKeyWithRoot(req, key);
auto outcome = client.GetObjectTagging(req);
if (!outcome.IsSuccess())
{
throw fromS3Error(
outcome.GetError(),
"S3 GetObjectTagging failed, bucket={} root={} key={}",
client.bucket(),
client.root(),
key);
}
return outcome.GetResult();
}

void listPrefix(
const TiFlashS3Client & client,
const String & prefix,
Expand Down
2 changes: 2 additions & 0 deletions dbms/src/Storages/S3/S3Common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,8 @@ void downloadFileByS3RandomAccessFile(

void rewriteObjectWithTagging(const TiFlashS3Client & client, const String & key, const String & tagging);

Aws::S3::Model::GetObjectTaggingResult getObjectTagging(const TiFlashS3Client & client, const String & key);

struct PageResult
{
size_t num_keys;
Expand Down
21 changes: 19 additions & 2 deletions dbms/src/Storages/S3/tests/gtest_s3client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ class S3ClientTest : public ::testing::Test
void SetUp() override
{
client = ClientFactory::instance().sharedTiFlashClient();
::DB::tests::TiFlashTestEnv::deleteBucket(*client);
::DB::tests::TiFlashTestEnv::createBucketIfNotExist(*client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
{
::DB::tests::TiFlashTestEnv::deleteBucket(*client);
::DB::tests::TiFlashTestEnv::createBucketIfNotExist(*client);
}
}

std::shared_ptr<TiFlashS3Client> client;
Expand Down Expand Up @@ -202,6 +205,20 @@ try
}
CATCH

TEST_F(S3ClientTest, SetGetTagging)
try
{
uploadEmptyFile(*client, "s999/abcd");

rewriteObjectWithTagging(*client, "s999/abcd", "jjj=abcd");
auto tagging_res = getObjectTagging(*client, "s999/abcd");
auto tag_set = tagging_res.GetTagSet();
ASSERT_EQ(tag_set.size(), 1) << tag_set.size();
EXPECT_EQ(tag_set[0].GetKey(), "jjj") << tag_set[0].GetKey() << "=" << tag_set[0].GetValue();
EXPECT_EQ(tag_set[0].GetValue(), "abcd") << tag_set[0].GetKey() << "=" << tag_set[0].GetValue();
}
CATCH

TEST_F(S3ClientTest, UploadRead)
try
{
Expand Down
29 changes: 22 additions & 7 deletions dbms/src/Storages/S3/tests/gtest_s3gcmanager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,13 @@ class S3GCManagerTest : public DB::base::TiFlashStorageTestBasic
createIfNotExist(tmp_dir);
}

void TearDown() override { ::DB::tests::TiFlashTestEnv::deleteBucket(*mock_s3_client); }
void TearDown() override
{
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
{
::DB::tests::TiFlashTestEnv::deleteBucket(*mock_s3_client);
}
}

protected:
String tmp_dir;
Expand Down Expand Up @@ -314,8 +320,11 @@ try

auto timepoint = Aws::Utils::DateTime("2023-02-01T08:00:00Z", Aws::Utils::DateFormat::ISO_8601);
auto clear_bucket = [&] {
Copy link
Copy Markdown

@pantheon-ai pantheon-ai bot Mar 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Test becomes incorrect when running with real S3 (MOCK_S3=false)

Why: The introduced change makes clear_bucket() a no-op in non-mocked mode, so test subcases deterministically reuse prior S3 state and can fail/flake.

Evidence: At line 322, clear_bucket only deletes/recreates the bucket when isMockedS3Client() is true. Later, the same test has subcases that assert delmark created (line 342) then assert delmark not created (line 357) using the same key, causing the second assertion to fail when state carries over from the first subcase.

Fix: Either skip these multi-scenario tests when !isMockedS3Client() using GTEST_SKIP(), or implement non-destructive per-key/prefix cleanup so clear_bucket() resets state without requiring DeleteBucket.

DB::tests::TiFlashTestEnv::deleteBucket(*mock_s3_client);
DB::tests::TiFlashTestEnv::createBucketIfNotExist(*mock_s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
{
DB::tests::TiFlashTestEnv::deleteBucket(*mock_s3_client);
DB::tests::TiFlashTestEnv::createBucketIfNotExist(*mock_s3_client);
}
};

{
Expand Down Expand Up @@ -400,8 +409,11 @@ try

auto timepoint = Aws::Utils::DateTime("2023-02-01T08:00:00Z", Aws::Utils::DateFormat::ISO_8601);
auto clear_bucket = [&] {
DB::tests::TiFlashTestEnv::deleteBucket(*mock_s3_client);
DB::tests::TiFlashTestEnv::createBucketIfNotExist(*mock_s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
{
DB::tests::TiFlashTestEnv::deleteBucket(*mock_s3_client);
DB::tests::TiFlashTestEnv::createBucketIfNotExist(*mock_s3_client);
}
};

{
Expand Down Expand Up @@ -514,8 +526,11 @@ try

auto timepoint = Aws::Utils::DateTime("2023-02-01T08:00:00Z", Aws::Utils::DateFormat::ISO_8601);
auto clear_bucket = [&] {
DB::tests::TiFlashTestEnv::deleteBucket(*mock_s3_client);
DB::tests::TiFlashTestEnv::createBucketIfNotExist(*mock_s3_client);
if (DB::tests::TiFlashTestEnv::isMockedS3Client())
{
DB::tests::TiFlashTestEnv::deleteBucket(*mock_s3_client);
DB::tests::TiFlashTestEnv::createBucketIfNotExist(*mock_s3_client);
}
};

{
Expand Down