Properly support materialized views restoration #192
Conversation
Khatskevich
force-pushed
the
khatskevich-materialized-views
branch
3 times, most recently
from
0a84969 to
278b323
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #192 +/- ##
==========================================
- Coverage 89.58% 89.10% -0.48%
==========================================
Files 143 144 +1
Lines 10126 10216 +90
==========================================
+ Hits 9071 9103 +32
- Misses 1055 1113 +58 ☔ View full report in Codecov by Sentry. |
Khatskevich
force-pushed
the
khatskevich-materialized-views
branch
from
278b323 to
3156fc6
Compare
| @@ -188,7 +188,6 @@ async def setup_cluster_content(clients: Sequence[HttpClickHouseClient], use_nam | |||
| await clients[0].execute( | |||
| b"CREATE TABLE default.mysql (a Int) ENGINE = MySQL('https://host:1234', 'database', 'table', 'user', 'password')" | |||
| ) | |||
| await clients[0].execute(b"CREATE TABLE default.s3 (a Int) ENGINE = S3('http://bucket.s3.amazonaws.com/key.json')") | |||
There was a problem hiding this comment.
Can you make that dependent on the presence of the engine instead of disabling it ?
(By using SELECT name FROM system.table_engines)
There was a problem hiding this comment.
I have fixed this.
But I have added incomplete set of table engines and did not used it in Table. Is it fine? (not sure if enum should not be used in Table because some new engines can be easily added to Clickhouse)
During test run several Clickhouse clusters are created. Each Clickhouse instance preallocates many thread pools with many threads each, especially on machines with many available cores. This commit decreases the number of threads each Clickhouse instance spawns during tests. `setting` helper was necessary to avoid too long lines, because Clickhouse does not support `\n` in variable declaration.
Khatskevich
force-pushed
the
khatskevich-materialized-views
branch
from
3156fc6 to
cedc04f
Compare
tests/integration/coordinator/plugins/clickhouse/test_plugin.py
Outdated
Show resolved
Hide resolved
| await restored_cluster[0].execute( | ||
| b"CREATE TABLE default.source_table_for_view_deleted (thekey UInt32, thedata String) " | ||
| b"ENGINE = ReplicatedMergeTree ORDER BY (thekey)" | ||
| ) | ||
| await restored_cluster[0].execute(b"INSERT INTO default.source_table_for_view_deleted VALUES (11, '11')") | ||
| await restored_cluster[2].execute(b"INSERT INTO default.source_table_for_view_deleted VALUES (17, '17')") |
There was a problem hiding this comment.
Sorry I didn't notice earlier.
This test will mutate the restored_cluster that has a module scope, this will affect other tests.
This needs a separate function_restored_cluster fixture which does the same as the module fixture but function-scoped and without the steps parametrization. (We don't want to make all tests function scoped, since the restore is slow and this test doesn't need to check the partial-restore-and-retry that we use for other tests)
There was a problem hiding this comment.
Sorry, I did it intentionally, I thought it is fine. But probably it is better to not touch state which looks immutable.
The fix looks a bit ugly, but fine. I am not exactly sure what this test is testing, but let it better be there, as someone will want to undelete some table one day.
Khatskevich
force-pushed
the
khatskevich-materialized-views
branch
3 times, most recently
from
4a78dd3 to
72574ec
Compare
Khatskevich
force-pushed
the
khatskevich-materialized-views
branch
from
72574ec to
8226a4a
Compare
Khatskevich
force-pushed
the
khatskevich-materialized-views
branch
from
536bf2e to
fa1e44e
Compare
tests/integration/coordinator/plugins/clickhouse/test_plugin.py
Outdated
Show resolved
Hide resolved
This commit enables the restoration of databases with materialized views when: 1. Source table was deleted before backup. 2. Source table was not backed up by Astacus. This is achieved by using `ATTACH` instead of `CREATE`, which performs fewer checks at the moment of creation. Misc: add a test for basic views to make sure they are restored properly in similar case. [DDB-890]
1. Deduplicate sorting logic. 2. Extend type checks.
Astacus can be tested against different builds and versions of Clickhouse. This commit skips tests with table engines which are likely to be disabled while supported by Astacus.
Khatskevich
force-pushed
the
khatskevich-materialized-views
branch
from
fa1e44e to
587ecf4
Compare
This commit makes replicas in Clickhouse tests synchronize before checking if expected data is present. It eliminates situation when one replica did not pass a fresh data to another one. [DDB-906]
Khatskevich
force-pushed
the
khatskevich-materialized-views
branch
from
587ecf4 to
a8ebc86
Compare
This pr enables the restoration of databases with materialized views when:
This is achieved by using
ATTACHinstead ofCREATE, which performsfewer checks at the moment of creation.
Misc: add a test for basic views to make sure they are restored properly
in similar case.
[DDB-890]