Skip to content

Relax covering part check on mutation#22291

Merged
alesapin merged 2 commits intomasterfrom
no_logical_error_on_mutation
Mar 30, 2021
Merged

Relax covering part check on mutation#22291
alesapin merged 2 commits intomasterfrom
no_logical_error_on_mutation

Conversation

@alesapin
Copy link
Copy Markdown
Member

@alesapin alesapin commented Mar 29, 2021

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • Bug Fix

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Now clickhouse will not throw LOGICAL_ERROR exception when we try to mutate the already covered part. Fixes #22013.

@robot-clickhouse robot-clickhouse added the pr-bugfix Pull request with bugfix, not backported by default label Mar 29, 2021
@CurtizJ CurtizJ self-assigned this Mar 29, 2021
@alesapin
Copy link
Copy Markdown
Member Author

alesapin commented Mar 30, 2021

01526_initial_query_id not related but looks suspicious.

@alesapin
Copy link
Copy Markdown
Member Author

alesapin commented Mar 30, 2021

Coordination::Exception: No node, path: /clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/replicas/replica/metadata: Cannot attach table `test_31`.`replica_01108_1_tmp` from metadata file /var/lib/clickhouse/metadata/test_31/replica_01108_1_tmp.sql from query ATTACH TABLE test_31.replica_01108_1_tmp (`n` Int32) ENGINE = ReplicatedMergeTree('/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1', 'replica')

@alesapin
Copy link
Copy Markdown
Member Author

alesapin commented Mar 30, 2021 •
edited
Loading

/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/replicas/

Really looks like partially dropped table:

$ ~/code/cpp/BuildCH/utils/keeper-data-dumper/keeper-data-dumper snapshots/ logs/ 2>/dev/null | grep '/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31'
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/metadata
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/nonincrement_block_numbers
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/temp
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/quorum
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/mutations
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/dropped
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/quorum/parallel
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/quorum/last_part
/clickhouse/tables/01108_restart_replicas_rename_deadlock_zookeeper_test_31/replica_01108_1/quorum/failed_parts

@alesapin alesapin closed this Mar 30, 2021
@alesapin alesapin reopened this Mar 30, 2021
@alesapin
Copy link
Copy Markdown
Member Author

alesapin commented Mar 30, 2021

Need to investigate.

@alesapin
Copy link
Copy Markdown
Member Author

alesapin commented Mar 30, 2021 •
edited
Loading

Known behavior

ClickHouse/src/Storages/StorageReplicatedMergeTree.cpp

Lines 801 to 803 in 1512ff8

zookeeper->tryRemoveRecursive(remote_replica_path);
if (zookeeper->exists(remote_replica_path))
LOG_ERROR(logger, "Replica was not completely removed from ZooKeeper, {} still exists and may contain some garbage.", remote_replica_path);

@alesapin alesapin merged commit 2f89152 into master Mar 30, 2021
@alesapin alesapin deleted the no_logical_error_on_mutation branch March 30, 2021 10:25
This was referenced Mar 30, 2021
Merged
Merged
Merged
Merged
Merged
robot-clickhouse pushed a commit that referenced this pull request Mar 30, 2021
Backport #22291 to 21.3: Relax covering part check on mutation
d5b1151
@robot-clickhouse robot-clickhouse mentioned this pull request Mar 30, 2021
Merged
robot-clickhouse pushed a commit that referenced this pull request Mar 30, 2021
Backport #22291 to 20.12: Relax covering part check on mutation
d98ee44
@robot-clickhouse robot-clickhouse mentioned this pull request Mar 30, 2021
Closed
robot-clickhouse pushed a commit that referenced this pull request Mar 30, 2021
Backport #22291 to 21.1: Relax covering part check on mutation
463956a
@robot-clickhouse robot-clickhouse mentioned this pull request Mar 30, 2021
Merged
@robot-clickhouse robot-clickhouse mentioned this pull request Mar 30, 2021
Merged
robot-clickhouse pushed a commit that referenced this pull request Mar 30, 2021
Backport #22291 to 21.2: Relax covering part check on mutation
74466a9
robot-clickhouse pushed a commit that referenced this pull request Mar 30, 2021
Backport #22291 to 20.8: Relax covering part check on mutation
77090dd
@robot-clickhouse robot-clickhouse mentioned this pull request Mar 30, 2021
Merged
alesapin added a commit that referenced this pull request Mar 31, 2021
@alesapin
Merge pull request #22366 from ClickHouse/backport/21.3/22291
85e6446 
Backport #22291 to 21.3: Relax covering part check on mutation
alesapin added a commit that referenced this pull request Mar 31, 2021
@alesapin
Merge pull request #22369 from ClickHouse/backport/21.2/22291
6e64e4c 
Backport #22291 to 21.2: Relax covering part check on mutation
alesapin added a commit that referenced this pull request Mar 31, 2021
@alesapin
Merge pull request #22368 from ClickHouse/backport/21.1/22291
7a9ec00 
Backport #22291 to 21.1: Relax covering part check on mutation
alesapin added a commit that referenced this pull request Mar 31, 2021
@alesapin
Merge pull request #22370 from ClickHouse/backport/20.8/22291
8c29528 
Backport #22291 to 20.8: Relax covering part check on mutation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CurtizJ CurtizJ CurtizJ approved these changes

Assignees

@CurtizJ CurtizJ

Labels

pr-bugfix Pull request with bugfix, not backported by default

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Stress test: StorageReplicatedMergeTree: part is covered but should be mutated

3 participants

@alesapin @CurtizJ @robot-clickhouse