Can some one add force test label please? (it requires updating of fast test image which is not built per-PR AFAIK)

@Mergifyio update

Command update: success

Branch already up to date

But it does not test clickhouse-client.
It's worth doing for debug and sanitizers builds.

@alexey-milovidov this PR converts only service use of clickhouse-client i.e. to:

• run show processlist
• to health check (simple select 1 or select {test_name})
• create database for tests

Every .sql test that was started using clickhouse-client will still use it.

How about adding an option to run tests with clickhouse_driver as this is only useful for developers.

this PR converts only service use of clickhouse-client

Ok.

How about adding an option to run tests with clickhouse_driver as this is only useful for developers.

This will too strange. Then, I would prefer to just leave it as-is (i.e. close this PR).
@amosbird Why do you think that separate code path is required for this?

I've converted it to python driver to make it faster since executing binaries (even from release build) will be always slower then simply connect+send+recv (i.e. native protocol).
My use case, once I've added some test I want to check it, and I'm waiting for clickhouse-test to do some checks that takes +-5 seconds with debug build.

I've converted it to python driver to make it faster since executing binaries (even from release build) will be always slower then simply connect+send+recv (i.e. native protocol).

Ok. I think it's worth doing.
But let's also improve the startup time of clickhouse-client (in separate PR).

for i in {1..10000}; do clickhouse-local --query "SELECT 1"; done

• it's spending time in initializing DateLUT - for what?

Why do you think that separate code path is required for this?

I missed this reply "this PR converts only service use of clickhouse-client" and thought it was to replace clickhouse-client for .sql tests)

Btw, why don't we just use HTTP for this purpose instead?

Btw, why don't we just use HTTP for this purpose instead?

I don't see any benefits in using HTTP instead of TCP.
Plus you need to parse it (since we don't only need to execute queries, and check return values, but also there are things like show processlist)

BTW looks like everything related had been passed!

Fast test — Cannot install or start ClickHouse

Requires image update (@alesapin I guess it will be done automatically after merge?).

Btw, why don't we just use HTTP for this purpose instead?

Ok, I haven't understand that clear enough.

@amosbird is right, HTTP interface can be also used instead of clickhouse-driver (python).

Cons:

• one more extra dependency

Pros:

• has better error handling (i.e. defines lots of exceptions for various kind of errors, and also provides lots of predefined error codes)
• provide better API for reading (streaming and so on), but it is not used right now
• it is already used for performance tests, so the cost of adding this "extra dependency" should be minor (although I guess not every developer runs performance tests by themselves, but they do run stateless tests for sure)

So maybe someone else has some opinion on this?

I don't have stong opinion on that - let's use what will be more convenient.

Integration tests (asan) — fail: 3, passed: 1585, flaky: 4
Integration tests (release) — fail: 3, passed: 1588, flaky: 3
Integration tests (thread) — fail: 5, passed: 1583, flaky: 5

• test_backup_restore - Fix flaky integration tests (test_backup_restore/test_input_format_parallel_parsing_memory_tracking) #29919
• test_input_format_parallel_parsing_memory_tracking/test.py::test_memory_tracking_total - Fix flaky integration tests (test_backup_restore/test_input_format_parallel_parsing_memory_tracking) #29919
• test_cluster_copier - Fix filtering by tuple (some conditions was lost during analyzing) #29956
• test_dictionaries_dependency_xml/test.py::test_get_data - Fix lock-order-inversion between periodic dictionary reload and config reload #29928
• Stress test (thread) — Sanitizer assert (in stderr.log) - Fix lock-order-inversion between DROP TABLE for DatabaseMemory and LiveView #29929

Also note, that this should make integration tests green.

Some numbers:

• fasttest: before 11min/14min, after 9min/12min -- no benefit
• stateless debug: no benefit

SELECT DISTINCT
    check_duration_ms,
    if(report_url LIKE '%268c155b7d4b15524219ea48cf228bf140cf9cdd%', 'python-requests', '') AS mr
FROM checks
WHERE (check_start_time > today()) AND (pull_request_number = 0) AND (report_url LIKE '%stateless_tests_(debug)%')
ORDER BY check_start_time ASC
FORMAT PrettyCompactMonoBlock

Query id: e9252c81-fe1c-4387-b62c-1a4c69e72715

┌─check_duration_ms─┬─mr──────────────┐
│           9168779 │                 │
│          11118219 │                 │
│           7965240 │ python-requests │
│           8240968 │                 │
│           8505889 │                 │
│           8503848 │                 │
│           8318323 │                 │
│           8066711 │                 │
└───────────────────┴─────────────────┘

• under sanitizers stateless tests looks a little bit faster, but no significant difference.

SELECT DISTINCT
    check_duration_ms,
    if(report_url LIKE '%268c155b7d4b15524219ea48cf228bf140cf9cdd%', 'python-requests', '') AS mr
FROM checks
WHERE (check_start_time > today()) AND (pull_request_number = 0) AND (report_url LIKE '%stateless_tests_(memory)%')
ORDER BY check_start_time ASC
FORMAT PrettyCompactMonoBlock

Query id: bc1993e5-413f-4d6a-86e2-66e0c1465efb

┌─check_duration_ms─┬─mr──────────────┐
│           7283462 │                 │
│           7491509 │                 │
│           7040965 │ python-requests │
│           6817031 │                 │
│           6866696 │                 │
│           6504630 │                 │
│           6624146 │                 │
│           6728470 │                 │
└───────────────────┴─────────────────┘

$ ./clickhouse-test  "01921_concurrent|992|993|1154|1593|1079"
Traceback (most recent call last):
  File "./clickhouse-test", line 38, in <module>
    import pandas
ModuleNotFoundError: No module named 'pandas'`

Should we fallback to using clickhouse-test if pandas or clickhouse_driver are not installed (just like we do it for termcolor and jinja)?

Another issue:

$ ./clickhouse-test  "01921_concurrent|992|993|1154|1593|1079"
Using queries from 'queries' directory
Using clickhouse-client as client program (expecting split build)
Connecting to ClickHouse server... OK

Running 17 stateless tests (MainProcess).

01921_concurrent_ttl_and_normal_merges_zookeeper_long:                  Unknown setting log_comment. Skipping
[ OK ]
01593_concurrent_alter_mutations_kill_many_replicas_long:               Unknown setting log_comment. Skipping
[ OK ]

https://github.com/mymarilyn/clickhouse-driver/blob/9ce201c0689ab04bf85660da78c640dec8c15355/clickhouse_driver/settings/writer.py#L24

Should we fallback to using clickhouse-test if pandas or clickhouse_driver are not installed (just like we do it for termcolor and jinja)?

Have a fallback for this code looks error prone and does not worth it to me.

P.S. I also think that fallback for jinja is extra complexity that does not worth it.
It is ok (to me) to have a fallback for termcolor since it is not a major functionality of clickhouse-test, unlike templated tests.

Unknown setting log_comment. Skipping

@tavplubix what version of clickhouse-driver do you have? (It should not produce such warnings after mymarilyn/clickhouse-driver#142)

Actually I've talked with @alesapin and he said his strong objections against using clickhouse-driver for clickhouse-test, so I'm planning to rewrite it to simple python requests (and using HTTP interface).

Actually I've talked with @alesapin and he said his strong objections against using clickhouse-driver for clickhouse-test, so I'm planning to rewrite it to simple python requests (and using HTTP interface).

+1.