Replace lost parts with empty parts instead of hacking replication queue#25820
Merged
Replace lost parts with empty parts instead of hacking replication queue#25820
Conversation
Member
Author
|
Will add only basic integration tests. Our stress/trash tests will catch other errors. |
Member
Author
|
@Mergifyio update |
Contributor
|
Command
|
Member
Author
|
One more time |
tavplubix
reviewed
Jun 30, 2021
alesapin
commented
Jun 30, 2021
Member
Author
|
@Mergifyio update |
Contributor
|
Command
|
tavplubix
reviewed
Jul 1, 2021
tavplubix
approved these changes
Jul 2, 2021
Member
Author
|
Got it: |
Member
Author
|
|
Member
Author
|
@Mergifyio update |
Contributor
|
Command
|
Member
Author
|
|
nvartolomei
added a commit
to nvartolomei/ClickHouse
that referenced
this pull request
Jul 22, 2021
This was introduced in ClickHouse#8602. The idea was to avoid data re-appearing in ClickHouse after DROP/DETACH PARTITION. This problem was only present in MergeTree engine and I don't understand why we need to do the same in ReplicatedMergeTree. For ReplicatedMergeTree the state of truth is stored in ZK, deleting things from filesystem just introduces inconsistencies and this is the main source for errors like "No active replica has part X or covering part". The resulting problem is fixed by ClickHouse#25820, but in my opinion we would better avoid introducing the ZK/FS inconsistency in the first place. When does this inconsistency appear? Often the sequence is like this: 0. Write 2 parts to ZK [all_0_0_0, all_1_1_0] 1. A merge gets scheduled 2. New part replaces old parts [new: all_0_1_1, old: all_0_0_0, all_1_1_0] 3. Replica gets shutdown and old parts are removed from filesystem 4. Replica comes back online, metadata about all parts is still stored in ZK for this new replica. 5. Other replica after cleanup thread runs will have only [all_0_1_1] in ZK 5. User triggers a DROP_RANGE after a while (drop range is for all_0_1_9999*) 6. Each replica deletes from ZK only [all_0_1_1]. The replica that got restarted uses its in-memory state to choose nodes to delete from ZK. 7. Restart the replica again. It will now think that there are 2 parts that it lost and needs to fetch them [all_0_0_0, all_1_1_0]. `clearOldPartsAndRemoveFromZK` which is triggered from cleanup thread runs cleanup sequence correctly, it first removes things from ZK and then from filesystem. I don't see much benefit of triggering it on shutdown and would rather have it called only from a single place. --- This is a very, very edge case situation but it proves that the current "fix" (ClickHouse#25820) isn't complete. ``` create table test( v UInt64 ) engine=ReplicatedMergeTree('/clickhouse/test', 'one') order by v settings old_parts_lifetime = 30; create table test2( v UInt64 ) engine=ReplicatedMergeTree('/clickhouse/test', 'two') order by v settings old_parts_lifetime = 30; create table test3( v UInt64 ) engine=ReplicatedMergeTree('/clickhouse/test', 'three') order by v settings old_parts_lifetime = 30; insert into table test values (1), (2), (3); insert into table test values (4); optimize table test final; detach table test; detach table test2; alter table test3 drop partition tuple(); attach table test; attach table test2; ``` ``` (CONNECTED [localhost:9181]) /> ls /clickhouse/test/replicas/one/parts all_0_0_0 all_1_1_0 (CONNECTED [localhost:9181]) /> ls /clickhouse/test/replicas/two/parts all_0_0_0 all_1_1_0 (CONNECTED [localhost:9181]) /> ls /clickhouse/test/replicas/three/parts ``` ``` detach table test; attach table test; ``` `test` will now figure out that parts exist only in ZK and will issue `GET_PART` after first removing parts from ZK. `test2` will receive fetch for unknown parts and will trigger part checks itself. Because `test` doesn't have the parts anymore in ZK `test2` will mark them as LostForever. It will also not insert empty parts, because the partition is empty. `test` is left with `GET_PART` in the queue and stuck. ``` SELECT table, type, replica_name, new_part_name, last_exception FROM system.replication_queue Query id: 74c5aa00-048d-4bc1-a2ea-6f69501c11a0 Row 1: ────── table: test type: GET_PART replica_name: one new_part_name: all_0_0_0 last_exception: Code: 234. DB::Exception: No active replica has part all_0_0_0 or covering part. (NO_REPLICA_HAS_PART) (version 21.9.1.1) Row 2: ────── table: test type: GET_PART replica_name: one new_part_name: all_1_1_0 last_exception: Code: 234. DB::Exception: No active replica has part all_1_1_0 or covering part. (NO_REPLICA_HAS_PART) (version 21.9.1.1) ```
azat
added a commit
to azat/ClickHouse
that referenced
this pull request
Aug 30, 2021
…y part AFAICS the problem is that some parts may be replaced with empty parts (after ClickHouse#25820), and removed by the cleanup thread, due to it is empty [1] (while it should not be deleted since it can download source part): <details> ``` 2021.08.18 20:11:22.687933 [ 341 ] {} <Trace> test_dpefxp.alter_table_1 (0758ca24-90e7-452c-8758-ca2490e7252c): Created log entry for mutation -1_115_115_0_146 ... 2021.08.18 20:11:22.707609 [ 22825 ] {} <Trace> test_dpefxp.alter_table_6 (766ae414-e113-4965-b66a-e414e1137965): Executing log entry to mutate part -1_115_115_0 to -1_115_115_0_146 2021.08.18 20:11:22.707643 [ 22825 ] {} <Debug> test_dpefxp.alter_table_6 (766ae414-e113-4965-b66a-e414e1137965): Source part -1_115_115_0 for -1_115_115_0_146 is not ready; will try to fetch it instead ... 2021.08.18 20:11:22.709397 [ 333 ] {} <Trace> test_dpefxp.alter_table_6 (ReplicatedMergeTreeQueue): Not executing log entry queue-0000001579 for part -1_115_115_0 because it is covered by part -1_115_115_0_146 that is currently executing. 2021.08.18 20:11:22.718861 [ 22825 ] {} <Information> test_dpefxp.alter_table_6 (766ae414-e113-4965-b66a-e414e1137965): DB::Exception: No active replica has part -1_115_115_0_146 or covering part ... 2021.08.18 20:11:27.936829 [ 295 ] {} <Information> test_dpefxp.alter_table_6 (766ae414-e113-4965-b66a-e414e1137965): Going to replace lost part -1_115_115_0_146 with empty part 2021.08.18 20:11:27.957839 [ 295 ] {} <Information> test_dpefxp.alter_table_6 (766ae414-e113-4965-b66a-e414e1137965): Created empty part -1_115_115_0_146 instead of lost part ... 2021.08.18 20:11:28.731635 [ 257 ] {} <Trace> test_dpefxp.alter_table_6 (ReplicatedMergeTreeCleanupThread): Cleared 190 old blocks from ZooKeeper ... 2021.08.18 20:11:28.734507 [ 257 ] {} <Trace> test_dpefxp.alter_table_6 (766ae414-e113-4965-b66a-e414e1137965): Will try to insert a log entry to DROP_RANGE for part: -1_115_115_0_146 2021.08.18 20:11:28.779373 [ 22837 ] {} <Debug> test_dpefxp.alter_table_6 (766ae414-e113-4965-b66a-e414e1137965): Removed 1 parts inside -1_115_115_0_146. ... 2021.08.18 20:11:28.792600 [ 273 ] {} <Trace> test_dpefxp.alter_table_6 (766ae414-e113-4965-b66a-e414e1137965): Created log entry /clickhouse/tables/00993_system_parts_race_condition_drop_zookeeper_test_dpefxp/alter_table/log/log-0000003459 for merge -1_111_118_1 ... 2021.08.18 20:11:28.910988 [ 354 ] {} <Error> test_dpefxp.alter_table_7 (ReplicatedMergeTreeQueue): Code: 49. DB::Exception: Part -1_111_118_1 intersects next part -1_115_115_0_146. It is a bug. (LOGICAL_ERROR), Stack trace (when copying this message, always include the lines below): 2021.08.18 20:11:31.282160 [ 305 ] {} <Error> test_dpefxp.alter_table_2 (ReplicatedMergeTreeQueue): Code: 49. DB::Exception: Part -1_111_118_1 intersects next part -1_115_115_0_146. It is a bug. (LOGICAL_ERROR), Stack trace (when copying this message, always include the lines below): ``` </details> [1]: https://clickhouse-test-reports.s3.yandex.net/27752/59e3cb18f4e53c453951267b5599afeb664290d8/functional_stateless_tests_(release,_wide_parts_enabled).html
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Better handling of lost parts for ReplicatedMergeTree tables. Fixes rare inconsistencies in ReplicationQueue. Nothing should be visible to the user. Fixes #10368.