Restore database replica#73100
Conversation
|
This is an automated comment for commit de99f65 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page
Successful checks
|
Yes, we should determine the first replica for restoring tables metadata. All other replicas would check their local table metadata with zookeeper metadata that was restored from However, it works well only for the So if we use
|
|
Sorry for the confusion. I mean the replica that receives the query is the So that if |
But we don’t have any guarantees about the execution order of |
|
@k-morozov I think we don't need to support restoring metadata from a specific node with |
|
|
||
| assert ["1"] == node_1.query( | ||
| f"SELECT count(*) FROM system.tables WHERE database='{exclusive_database_name}'" | ||
| ).split() |
There was a problem hiding this comment.
We can assert with TSV:
To assert that two TSV files must be equal, wrap them in the TSV class and use the regular assert statement. Example: assert TSV(result) == TSV(reference). In case the assertion fails, pytest will automagically detect the types of variables and only the small diff of two files is printed.
https://github.com/ClickHouse/ClickHouse/blob/master/tests/integration/README.md
Or strip() to remove `\n'.
There was a problem hiding this comment.
Do you mean using TSV file for expected value?
There was a problem hiding this comment.
Please refer to this test:
ClickHouse/tests/integration/test_backup_restore_on_cluster/test_slow_rmt.py
Lines 115 to 118 in a66a6a5
| ) | ||
| def test_query_after_restore_db_replica( | ||
| start_cluster, | ||
| exclusive_database_name, |
There was a problem hiding this comment.
We can use the same DB name. At the beginning of the test, we can drop the DB if it exists with SYNC to avoid flakiness.
It may help identify some potential issues.
There was a problem hiding this comment.
I suggest using unique database name for test because we would have unique database name for each test in the log and we could grep all usefull information about this database by its unique name.
| look_behind_lines=1000, | ||
| ) | ||
|
|
||
| assert [f"{count_test_table_1}"] == node_1.query( |
There was a problem hiding this comment.
It may take some time for the tables to be restored. We can use query_with_retry to make the test to pass consistently.
@tuanpach hi. I am confused. You suggest using There are 2 nodes
How it works in my mind:
As a result we have broken |
Sorry for the confusion. My idea was that we use the host that receives the query as the initial host. And we restore tables' metadata from that host. However, it might not be necessary. If users want to restore tables' metadata from a specific host, they can run the restore query without In the current implementation with |
So we would use current implementation, wouldn't we? Another comments were resolved. |
|
Yes, we can use the current implementation. |
cbab61c
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
This PR introduces the restore database replica functionality for replicated databases, similar to the existing functionality for restore in ReplicatedMergeTree.
Documentation entry for user-facing changes
CI Settings (Only check the boxes if you know what you are doing):