Populate _tags virtual column for S3Queue/ObjectStorageQueue

[groeneai]

Related: #108625

Changelog category (leave one):

• Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fixed the _tags virtual column for the S3Queue table engine: it was declared but never populated, so SELECT _tags always returned an empty map. It now returns the object tags, like the S3 engine and the s3() table function.

Description

Follow-up to #108625 (_etag), per @ alexey-milovidov's request to check for other unpopulated virtual columns.

_tags is declared for ObjectStorageQueue engines through the shared file-like virtual column list, but the read path in ObjectStorageQueueSource never filled it, so SELECT _tags always returned an empty map.

Unlike _etag (which ListObjectsV2 returns for free during listing), object tags require a separate GetObjectTagging call, so they cannot simply be forwarded from the listing result: the queue iterator lists with with_tags = false. The plain S3 engine fetches tags on demand only when _tags is requested (StorageObjectStorageSource::createReader). This change mirrors that: when _tags is among the requested virtual columns, the queue source fetches the object metadata with tags once per file (right after the reader is created) and then forwards tags into the virtuals struct, the same way _etag is forwarded.

This is S3-only: AzureObjectStorage::getObjectMetadata does not expose blob tags, so AzureQueue's _tags stays empty, the same as the plain Azure path today.

Audit of the remaining declared file-like virtual columns on the queue read path: _path, _file, _size, _time, _etag, _table and _row_number are populated. _data_lake_snapshot_version and _iceberg_metadata_file_path are declared via the shared list but are not applicable to queue engines (they read plain objects with no Iceberg/Delta metadata).

A regression test is added to test_storage_s3_queue test_virtual_column_tags: it tags the object, reads _tags through a materialized view, and asserts the map is populated. The test fails before this change (empty map) and passes after.

Version info

• Merged into: 26.8.1.460 (included in 26.8 and later)

[groeneai]

Pre-PR validation gate (click to expand)

#	Question	Answer
a	Deterministic repro?	Yes. test_storage_s3_queue/test_0.py::test_virtual_column_tags: tag an S3 object, read _tags via a materialized view. Fails on demand without the fix (assert '' == 'ClickHouse\tCore').
b	Root cause explained?	_tags is declared via the shared file-like virtual list, but the queue read path built the virtuals struct without .tags, and the queue iterator lists with with_tags=false, so tags are never fetched. addRequestedFileLikeStorageVirtualsToChunk then fills _tags with an empty map.
c	Fix matches root cause?	Yes. Mirrors the plain S3 path: when _tags is requested, fetch object metadata with tags once per file (getObjectMetadata(path, with_tags=true)) and forward .tags into the virtuals struct. Not a band-aid.
d	Test intent preserved / new tests added?	New regression test added (test_virtual_column_tags). The existing test_virtual_columns (_etag/_path/...) is unchanged and still passes.
e	Both directions demonstrated?	Yes. Without fix: FAILS (empty _tags). With fix: PASSES. Both verified on Build-ID-distinct binaries (b10fe0a4 without, 428019ed with).
f	Fix is general across code paths?	The unpopulated-virtual gap was audited across the whole queue read path. _tags is the only fetchable gap; _data_lake_snapshot_version / _iceberg_metadata_file_path are declared via the shared list but not applicable to queue engines (plain objects, no Iceberg/Delta).
g	Fix generalizes across inputs?	Tags fetched once per file only when _tags is requested (no extra GetObjectTagging otherwise, matching the plain-path contract). S3-only: AzureObjectStorage::getObjectMetadata does not expose blob tags, so AzureQueue _tags stays empty, same as the plain Azure path today. Empty-tag objects yield an empty map (correct).
h	Backward compatible?	Yes. No setting default, on-disk/wire format, or validation change. A previously-empty _tags now returns the real value (the documented intent of the column).
i	Invariants and contracts preserved?	Yes. The on-demand fetch runs once per file in the reader-creation branch (not per chunk), merges into the existing ObjectInfo metadata via setObjectMetadata, and forwards a pointer to that long-lived metadata. No block-structure, ownership, or concurrency contract is altered.

Session id: cron:clickhouse-worker-slot-2:20260627-004000

[groeneai]

cc @kssenii @scanhex12 — could you review this? Follow-up to #108625: _tags is declared for S3Queue/ObjectStorageQueue but the read path never populated it (the queue iterator lists with with_tags=false), so SELECT _tags always returned an empty map. This fetches tags on demand once per file when _tags is requested, mirroring the plain S3 path.

[clickhouse-gh]

Workflow [PR], commit [bca6656]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR fixes the previously declared but always-empty _tags virtual column for S3Queue / ObjectStorageQueue by fetching object tags on demand after the file is claimed, then forwarding them through the existing file-like virtual-column path. The follow-up commit also closes the earlier accounting hole where a tag-fetch exception could orphan a claimed file before it was tracked in processed_files. I did not find any remaining correctness, compatibility, or coverage issue in the current code, and the latest CI report for commit bca66568b5d2897a8c2bb0b90504f3b331ce9fc1 is all green.

Final Verdict

✅ No findings.

[groeneai]

Re-validation for the review fix (commit 1eafa11), addressing the claimed-file accounting finding.

Pre-PR validation gate (click to expand)

#	Question	Answer
a	Deterministic repro?	Yes. New object_storage_queue_fail_tags_fetch ONCE failpoint makes the tag fetch throw on demand; test_virtual_column_tags_fetch_failure_is_accounted triggers it every run.
b	Root cause explained?	The _tags fetch ran between createReader claiming the file as Processing and processed_files.emplace_back. A getObjectMetadata(.., true) throw reached commit(false) with processed_files empty, so prepareCommitRequests returned early: no failed/reset request, no s3queue_log entry. The claimed file was recoverable only via destructor cleanup and could be re-picked outside retry/failure accounting.
c	Fix matches root cause?	Yes. Moved the fetch to after processed_files.emplace_back (reviewer option 1). A throw now hits commit(false) with the file present as FileState::Processing, taking the same prepareFailedRequests path as read errors.
d	Test intent preserved / new tests added?	Existing test_virtual_column_tags (_tags populated) and test_virtual_columns (_etag) still pass. Added test_virtual_column_tags_fetch_failure_is_accounted asserting the claimed file is recorded as Failed when the fetch throws.
e	Both directions demonstrated?	Yes. With the fetch after emplace_back: test PASSES (file Failed). With the fetch before emplace_back (old position): test FAILS (assert failed_seen is False, file orphaned). Both built with verified-distinct Build IDs.
f	Fix is general across code paths?	Yes. There is one tag-fetch site on the queue read path. The returnForRetry commit-limit branch runs before emplace_back and breaks, so a file returned for retry never reaches the fetch (no orphan there either).
g	Fix generalizes across inputs?	Yes. The relocation is independent of object/tag content; any throw type from the fetch (denied, missing key, transient) is routed through the same accounting. _tags map shape (empty/non-empty) is unaffected.
h	Backward compatible?	Yes. No setting, format, or on-disk/wire change. The new failpoint is test-only (disabled by default).
i	Invariants and contracts preserved?	Yes. The processed_files.back() invariant (last entry is the in-flight file in Processing) now holds before the fetch can throw, matching the assumption of the shutdown/cancel/read-error branches and prepareCommitRequests. progress->processed_files accounting is unchanged (the fetch is after the commit-limit check).

Session id: cron:clickhouse-worker-slot-0:20260627-165500

[groeneai]

CI finish ledger — 1eafa11

CI fully finished (Finish Workflow + Mergeable Check pass, 0 checks pending). Every failure below has an owner: a fixing PR (ours or external), or a full-effort fix task whose fixing-PR link will be posted here when it opens. Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
Stress test (amd_tsan) / Hung check failed, possible deadlock found	deadlock — chronic GlobalThreadPool shutdown family (ThreadPoolImpl::wait() at "Start preparing for shutdown"; 355 master hits + broad cross-PR in 30d). Unrelated to this PR's S3Queue _tags read-path diff.	#101680 / #105905 (ours, open)
Stress test (arm_msan) / Hung check failed, possible deadlock found	deadlock — same shutdown family	#101680 / #105905 (ours, open)

Bugfix validation (integration tests, amd64 + aarch64): SUCCESS (the changed test_storage_s3_queue/test_0.py tests correctly fail on the master-HEAD binary, proving they catch the regression). No PR-caused failure.

Session id: cron:our-pr-ci-monitor:20260627-213000

[kssenii]

Why not just modify it to include /*with_tags=*/ info.requested_virtual_columns.contains("_tags")?

[groeneai]

It would populate tags, but not for free: with_tags=true makes the S3 iterator issue a GetObjectTagging per listed object (S3IteratorAsync in S3ObjectStorage.cpp), since ListObjectsV2 itself does not return tags. I kept the fetch separate on purpose:

• Listing tags every object before filtering. FileIterator::next lists, then drops files by glob/predicate, filterProcessableFiles (already-processed/failed, plus hash-ring to this replica in Unordered mode), and finally the ZK set-processing claim. Setting with_tags on iterate would issue GetObjectTagging for files this replica never processes, and re-tag already-processed files on every re-list pass. The on-demand fetch is exactly one GetObjectTagging per file we actually process.
• Listing runs on a shared background thread. A tagging throw there aborts the whole batch for all processors, with no claimed file to attribute it to. Fetching after the file is claimed (processed_files.emplace_back, Processing state) routes a throw through commit -> prepareFailedRequests, the same Failed / s3queue_log path read errors use (this is the accounting fix in 1eafa11).
• It mirrors the plain object storage read path, which keeps its read iterator at with_tags=false and fetches tags per file in StorageObjectStorageSource::next gated on requested_virtual_columns.contains("_tags").

Happy to switch to the iterator flag if you prefer the simpler form, with the caveat of extra GetObjectTagging calls on filtered-out and already-processed files.

[alexey-milovidov]

Yes, this is reasonable. Thank you!

[groeneai]

Hi @kssenii, gentle ping: do you prefer I switch to threading with_tags through the iterator, or keep the separate on-demand fetch (which avoids a GetObjectTagging on every listed object before filtering)? Happy to do either.

[groeneai]

CI finish ledger — ca7d8c5

Every failure below has an owner: a fixing PR (ours or external), or a full-effort fix task whose fixing-PR link will be posted here when it opens. Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
AST fuzzer (amd_debug, targeted, old_compatibility) / Unknown error, "Lost connection to server"	slow-query server hang (fuzzed numbers_mt(2147483646) with max_threads=65535 runs >5min on debug build → health-check SOCKET_TIMEOUT → watchdog kill, exit 137; not a crash/LOGICAL_ERROR/OOM; unrelated to this PR's S3Queue-only diff)	a fix task is created (investigating at full effort — fixing-PR link to follow on this PR)

Session id: cron:our-pr-ci-monitor:20260703-043000

[groeneai]

Fixing PR for the AST fuzzer slow-query / Lost connection to server hang (seen here on AST fuzzer (amd_debug, targeted, old_compatibility), sha ca7d8c5) created: #109256. Root cause was a fuzzed max_threads=65535 saturating the global thread pool and starving the liveness probe; #109256 caps max_threads in the fuzzer profile. Not caused by this PR (S3Queue-only diff).

[clickhouse-gh]

Dear @kssenii, you haven't been active on this PR for 30 days. You will be unassigned. Will you continue working on it? If so, please feel free to reassign yourself.

[alexey-milovidov]

The code LGTM

[alexey-milovidov]

@groeneai, update the branch to fix the "unknown error" in fuzzer, which is likely fixed in master.

[groeneai]

Done, new head ee4431b822a6851c2c362f7fd114f274802620b4. Your hypothesis was right, and the fix is specifically identifiable.

The Unknown error / Lost connection to server on the old head was not a crash. The fuzzer reached step 31 with SELECT sum(pointInPolygon(...)) FROM numbers_mt(2147483646) SETTINGS max_threads = 65535; a fuzzed max_threads above the global pool size (max_thread_pool_size = 10000) saturates the pool, so the harness's SELECT 1 liveness probe never gets a thread and blocks for the full 300 s socket timeout instead of the intended 5 s. run-fuzzer.sh counted 12 such probe timeouts over ~62 minutes, declared the server hung, and the gdb watchdog killed it (server_exit_code=137, server_died=1, no core, no fatal, no OOM in dmesg).

That is fixed on master by the max_threads <max>128</max> constraint I added in #109256 and moved into the shared ci/jobs/scripts/fuzzer/limit-recursion-settings.xml in 9648eed9cd6c9d5928f87adcb210550e08ecba12 (merged 2026-07-08). A fuzzed max_threads = 65535 now returns a clean SETTING_CONSTRAINT_VIOLATION, which the fuzzer tolerates and continues past. Neither commit was an ancestor of the old head (2026-06-28); both are ancestors of the new one, so this run should not reproduce the hang. Since master's AST fuzzer (..., old_compatibility) has had zero failures of this signature since that merge, I expect it green here too.

The merge changed nothing in the fix itself: the PR delta before and after is content-identical apart from hunk offsets, and no submodule pin moved. The merged tree builds clean.

[groeneai]

CI finish ledger — ee4431b

Every failure below has an owner: a fixing PR (mine or external), or a full-effort fix task
whose fixing-PR link will be posted here when it opens. Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
Stateless tests (amd_tsan, parallel) / Server died	mislabelled harness leaf: the test runner was SIGTERMed externally, the server did not die	a fix task is created (investigating at full effort — fixing-PR link to follow here)

All 174 checks completed; this was the only failing job, and 173 of them are green.

The Server died label is provably wrong on this run, so the fix belongs in the harness rather
than in this diff:

• The job node reports Failed: 0, Passed: 10564, Skipped: 171. No test failed.
• Tests were completing normally at the instant of the kill: [10734/11981], [10735] and
  [10736] all [ OK ] within the final two seconds, then Terminated with 15 signal.
• The server answered queries after that signal: system flush logs and
  SYSTEM FLUSH ASYNC INSERT QUEUE both ran, and clickhouse stop --do-not-kill then completed
  cleanly. A dead or hung server does neither.
• No crash: zero <Fatal>, zero Received signal, zero Sanitizer: lines, no core dump, and
  dmesg.log contains no OOM record.
• Not a job timeout either: the budget for this job is 150 minutes and the signal arrived at 81,
  with none of the praktika timeout warning lines present.

ci/jobs/scripts/functional_tests_results.py puts 128 + SIGTERM in ABORTED_RUN_EXIT_CODES and
that branch appends a Server died leaf unconditionally, so any external signal to the runner is
reported as a server death. The 30-day spread of this leaf on this check is 35 hits across 25
distinct PRs with 0 on master.

This PR only populates the _tags virtual column for S3Queue
(ObjectStorageQueueSource.cpp, FailPoint.cpp, one integration test), so it cannot terminate a
test runner.

The AST fuzzer (amd_debug, targeted, old_compatibility) red that this branch was updated to clear
is green on this head, as predicted.

Session id: cron:our-pr-ci-monitor:20260729-010000

[alexey-milovidov]

Updated the branch to current master, new head bca66568b5da.... The merge was conflict-free, the PR delta is content-identical (3 files, +161: FailPoint.cpp, ObjectStorageQueueSource.cpp, test_storage_s3_queue/test_0.py), no submodule pin moved, and the merged tree compiles both touched translation units clean.

The reason for the refresh is the single red on the previous head, Stateless tests (amd_tsan, parallel) / Server died, which is a mislabelled harness leaf and not a server death: the job node reported Failed: 0, Passed: 10564, tests [10734]–[10736] all [ OK ] in the final two seconds, then Terminated with 15 signal, after which the server still answered SYSTEM FLUSH LOGS and stopped cleanly — no <Fatal>, no Received signal, no sanitizer report, no core, no OOM in dmesg, and no praktika timeout warning (the signal arrived at 81 of 150 budget minutes). ci/jobs/scripts/functional_tests_results.py treats 128 + SIGTERM as an aborted run and appends a Server died leaf unconditionally, so any external signal to the runner is reported this way. This PR only populates the _tags virtual column on the S3Queue read path and cannot terminate a test runner.

Related: #112265 — the same external-kill family on Stateless tests, there in its no-results job-level error form.

Also confirmed on the current code: no unresolved review threads remain (the with_tags iterator-flag question from @kssenii was answered and accepted), and the latest AI review reports no findings.

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.50%	86.50%	+0.00%
Functions	91.90%	91.90%	+0.00%
Branches	78.60%	78.60%	+0.00%

Changed lines: Changed C/C++ lines covered: 29/29 (100.00%) · Uncovered code

Full report · Diff report

[groeneai]

CI finish ledger - bca6656

Every failure below has an owner: a fixing PR (mine or external), or a full-effort fix task
whose fixing-PR link will be posted here when it opens. Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
(none)	174/174 check-runs completed, 0 failed	-

All 174 check-runs on this head are completed with no failure, timed_out or
action_required conclusion, and the praktika Finish Workflow aggregator is green.

One clarification so the CIDB rows are not misread: Bugfix validation (integration tests, amd64/aarch64) both report conclusion success, and the test_storage_s3_queue/test_0.py
FAIL rows recorded against this commit are the master-binary side of that job, which is
required to fail for the validation to pass. On the PR side the same tests are green on every
regular integration job (amd_tsan, amd_msan, amd_asan_ubsan db-disk old-analyzer,
amd_llvm_coverage, arm_binary distributed-plan, amd_binary_excluded_from_llvm, and the flaky
job): 168/168, 168/168, 168/168, 139/139, 170/170, 39/39, 41/41, zero failures.

The previous head's single red, Stateless tests (amd_tsan, parallel) / Server died, is not
present on this head.

Session id: cron:our-pr-ci-monitor:20260730-180000

[groeneai]

Fixing PR for the mislabelled Server died leaf on the CI finish ledger: #112952