tests: Add unittest about tagging under disagg arch

[JaySon-Huang]

What problem does this PR solve?

Issue Number: ref #6233

Problem Summary:

What is changed and how it works?

  * Added object tagging retrieval capability to S3 and add related testing
  * Improved S3 test suite to conditionally manage test bucket cleanup based on client mock status, reducing unnecessary operations in test environments.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

None

Summary by CodeRabbit

Release Notes

• New Features

  • Added object tagging retrieval capability to S3 utilities for enhanced metadata operations.

• Tests

  • Improved S3 test suite to conditionally manage test bucket cleanup based on client mock status, reducing unnecessary operations in test environments.

[pantheon-ai]

Review Complete

Findings: 1 issues
Posted: 1
Duplicates/Skipped: 0

_{ℹ️ Learn more details on Pantheon AI.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Multiple test files now conditionally delete S3 buckets only when a mocked S3 client is present. A new S3 object tagging utility function is introduced alongside a corresponding test case.

Changes

Cohort / File(s)	Summary
Conditional S3 Bucket Cleanup in Tests dbms/src/Storages/DeltaMerge/File/tests/gtest_dm_meta_version.cpp, dbms/src/Storages/DeltaMerge/Index/InvertedIndex/tests/gtest_dm_inverted_index.cpp, dbms/src/Storages/DeltaMerge/Index/VectorIndex/tests/gtest_dm_vector_index.cpp, dbms/src/Storages/DeltaMerge/tests/gtest_dm_delta_merge_store_fast_add_peer.cpp, dbms/src/Storages/DeltaMerge/tests/gtest_dm_segment_s3.cpp, dbms/src/Storages/DeltaMerge/tests/gtest_segment_replace_stable_data.cpp, dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp, dbms/src/Storages/Page/V3/Universal/tests/gtest_lock_local_mgr.cpp, dbms/src/Storages/S3/tests/gtest_s3gcmanager.cpp	Updated TearDown/SetUp methods to conditionally invoke bucket deletion only when isMockedS3Client() returns true; previously unconditional.
Test Cleanup Helpers dbms/src/IO/Encryption/tests/gtest_keyspaces_key_manager.cpp, dbms/src/Storages/Page/V3/Universal/tests/gtest_remote_read.cpp	Modified helper functions to guard bucket deletion behind isMockedS3Client() check instead of unconditional invocation.
S3 Object Tagging Utility dbms/src/Storages/S3/S3Common.h, dbms/src/Storages/S3/S3Common.cpp	Added new public function getObjectTagging() that wraps the S3 GetObjectTagging API with error handling.
S3 Client Tests dbms/src/Storages/S3/tests/gtest_s3client.cpp	Conditionally delete/recreate bucket in SetUp only for mocked clients; added new SetGetTagging test case validating object tagging retrieval workflow.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

size/L

Suggested reviewers

• CalvinNeo
• JinheLin

Poem

🐰 A rabbit hops through S3 clouds,
Conditionally cleaning, not too loud!
Tagging objects, storing with care,
Mocked buckets deleted only when they're there. ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	PR description is mostly complete with checked unit test box and release note, but Problem Summary section is empty and commit message details are missing.	Fill in the Problem Summary explaining the motivation and issue context; provide detailed commit message in the commit-message block to clearly document the changes.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title accurately describes the main change: adding a unittest about tagging under disaggregated architecture, which is evidenced by new tagging test functionality and supporting infrastructure changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

You can disable the changed files summary in the walkthrough.

Disable the reviews.changed_files_summary setting to disable the changed files summary in the walkthrough.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign searise for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

🧹 Nitpick comments (1)

dbms/src/Storages/S3/S3Common.cpp (1)

829-844: Implementation is correct.

The function properly follows the error handling pattern used by other S3 helper functions.

For consistency with other S3 operations in this file (e.g., rewriteObjectWithTagging, headObject), consider adding metrics/observability if this function will be used in production paths. Currently, since it's only used in tests, this is optional.

♻️ Optional: Add observability for production use

 Aws::S3::Model::GetObjectTaggingResult getObjectTagging(const TiFlashS3Client & client, const String & key)
 {
+    Stopwatch sw;
     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);
     }
+    GET_METRIC(tiflash_storage_s3_request_seconds, type_get_object_tagging).Observe(sw.elapsedSeconds());
     return outcome.GetResult();
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dbms/src/Storages/S3/S3Common.cpp` around lines 829 - 844, getObjectTagging
is implemented correctly but lacks observability; wrap the
Aws::S3::Model::GetObjectTaggingRequest call in the same metrics pattern used by
other S3 helpers (e.g., record latency and increment success/failure counters),
instrument around client.GetObjectTagging in getObjectTagging, label metrics
with client.bucket(), client.root() and key, and ensure failures still throw via
fromS3Error while emitting the failure metric.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@dbms/src/Storages/S3/S3Common.cpp`:
- Around line 829-844: getObjectTagging is implemented correctly but lacks
observability; wrap the Aws::S3::Model::GetObjectTaggingRequest call in the same
metrics pattern used by other S3 helpers (e.g., record latency and increment
success/failure counters), instrument around client.GetObjectTagging in
getObjectTagging, label metrics with client.bucket(), client.root() and key, and
ensure failures still throw via fromS3Error while emitting the failure metric.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e33f4aac-a190-4a79-9b47-c6e5ededd60b

📥 Commits

Reviewing files that changed from the base of the PR and between 7fe4b3b and f7da65f.

📒 Files selected for processing (14)

• dbms/src/IO/Encryption/tests/gtest_keyspaces_key_manager.cpp
• dbms/src/Storages/DeltaMerge/File/tests/gtest_dm_meta_version.cpp
• dbms/src/Storages/DeltaMerge/Index/InvertedIndex/tests/gtest_dm_inverted_index.cpp
• dbms/src/Storages/DeltaMerge/Index/VectorIndex/tests/gtest_dm_vector_index.cpp
• dbms/src/Storages/DeltaMerge/tests/gtest_dm_delta_merge_store_fast_add_peer.cpp
• dbms/src/Storages/DeltaMerge/tests/gtest_dm_segment_s3.cpp
• dbms/src/Storages/DeltaMerge/tests/gtest_segment_replace_stable_data.cpp
• dbms/src/Storages/KVStore/tests/gtest_kvstore_fast_add_peer.cpp
• dbms/src/Storages/Page/V3/Universal/tests/gtest_lock_local_mgr.cpp
• dbms/src/Storages/Page/V3/Universal/tests/gtest_remote_read.cpp
• dbms/src/Storages/S3/S3Common.cpp
• dbms/src/Storages/S3/S3Common.h
• dbms/src/Storages/S3/tests/gtest_s3client.cpp
• dbms/src/Storages/S3/tests/gtest_s3gcmanager.cpp

[pantheon-ai]

[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.