ClickHouse / ClickHouse/ClickHouse

`SYSTEM SYNC MERGES` hangs forever when scheduled parts don't exist or are dropped

Open
#104,883 0 comments 0 reactions 1 assignee Claimed by @tiandiwonder View on GitHub
bug comp-background-ops comp-mergetree
Dominant language
C++
Stars
49.9k
Forks
9k
Avg merge
21h 32m
Merged PRs (30d)
515

Description

_Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment._

### Describe what's wrong

`SYSTEM SYNC MERGES table` enters an infinite loop if any scheduled part names don't correspond to existing active parts, causing the query to hang until manually killed or connection timeout.

**Root cause:** ManualMergeSelector.cpp:125-128: The erase predicate `!containing.empty() && containing != part_info.getPartNameV1()` only erases when a covering part exists. When `getContainingPart` returns empty (part doesn't exist), the condition evaluates to false, so the part stays in `scheduled_part_infos` forever.

**Why we believe this is a bug:** InterpreterSystemQuery.cpp:2107-2120 → syncMerges() polls in `while(true)` loop → ManualMergeSelector::isAllScheduledPartsCovered() (ManualMergeSelector.cpp:121-131) → ActiveDataPartSet::getContainingPart() returns empty string for non-existent parts → erase condition fails → scheduled_part_infos never becomes empty → loop never terminates

**Affected locations:**
- `src/Interpreters/InterpreterSystemQuery.cpp:2107` — syncMerges while(true) loop
- `src/Storages/MergeTree/Compaction/MergeSelectors/ManualMergeSelector.cpp:125` — isAllScheduledPartsCovered erase predicate

**Impact:** Any test using SYSTEM SYNC MERGES with invalid part names (typos, parts dropped before merge completes, old-format part names on new-format tables) will hang indefinitely. This defeats the testing purpose and wastes CI resources.

### Does it reproduce on most recent release?

Likely yes — see testability note in additional context.

### How to reproduce

```sql
-- Cannot test: binary does not include PR changes (MANUAL merge selector not recognized)
-- Test would be:
DROP TABLE IF EXISTS t_sync_hang;
CREATE TABLE t_sync_hang (x UInt64) ENGINE = MergeTree ORDER BY x SETTINGS merge_selector_algorithm = 'Manual';
INSERT INTO t_sync_hang VALUES (1);
SYSTEM SCHEDULE MERGE t_sync_hang PARTS 'all_1_1_0', 'all_999_999_0'; -- non-existent part
SYSTEM SYNC MERGES t_sync_hang; -- HANGS FOREVER
SELECT 'unreachable';
DROP TABLE t_sync_hang;
```

### Expected behavior

_The code should not exhibit the behavior described in the root cause above._

### Error message and/or stacktrace

_See root cause description above._

### Additional context

**Open risks:**
- Parts scheduled in the queue block subsequent merges if lookupRange fails - the queue is never drained for invalid entries

**Suggested fix:** Add handling for non-existent parts: (1) In `isAllScheduledPartsCovered`, also erase parts where `containing.empty()` and no matching part exists anywhere in the active set (the part was dropped/never existed); (2) Add a timeout parameter to `SYNC MERGES` or use `receive_timeout` setting like `SYNC REPLICA` does; (3) In `scheduleMerge`, validate that all specified parts exist before adding to queue.

**Analysis details:** Confidence HIGH | Severity P2 | Testability: `THEORETICAL`

Found during automated review of [PR #104274](https://github.com/ClickHouse/ClickHouse/pull/104274).

---
_ClickGapAI · Confidence: HIGH · Severity: P2 · Finding: `h_pr104274_001`_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.