Use subshell to:
- avoid change/restore of TIMEFORMAT
- grepping out trace output, by using set +x in a subshell
- use array for options to avoid extra backslashes
killall requires strict match, i.e. "clickhouse-server" not
"clickhouse":
2021-12-03 05:24:56 + env kill -- -21700
2021-12-03 05:24:56 kill: (-21700): No such process
2021-12-03 05:24:56 + killall clickhouse
2021-12-03 05:24:56 clickhouse: no process found
2021-12-03 05:24:56 + echo Servers stopped.
2021-12-03 05:24:56 Servers stopped.
2021-12-03 05:24:56 + analyze_queries
$ tail -n1 *-server-log.log
==> left-server-log.log <==
2021.12.03 05:26:59.530647 [ 450 ] {} <Trace> SystemLog (system.asynchronous_metric_log): Flushed system log up to offset 1668052
==> right-server-log.log <==
2021.12.03 05:27:20.873136 [ 466 ] {} <Trace> SystemLog (system.metric_log): Flushed system log up to offset 9605
==> setup-server-log.log <==
2021.12.03 02:47:14.844395 [ 96 ] {} <Information> Application: Child process exited normally with code 0.
As you can see killall instantly fails with no such process, while this
cannot be true since it was there, and also according to logs there were
messages after running analyze_queries() from compare.sh
This should fix problems like in [1].
[1]: https://clickhouse-test-reports.s3.yandex.net/32080/344298f4037f88b114b8e798bb30036b24be8f16/performance_comparison/report.html#fail1
Before this patch:
- upstream/master and PR's *with* perf tests or pef scripts changes:
--runs=13 --max-queries=0
- PRs *without* perf changes:
--runs=7 --max-queries=20
- PRs w/ only perf tests changes:
--runs-13 --max-queries=0 <list of perf tests>
After:
- upstream/master and PR's *with* perf tests changes:
--runs=13 --max-queries=0
- PRs *without* perf changes:
--runs=7 --max-queries=10
- PRs w/ only perf tests changes:
--runs-13 --max-queries=0 <list of perf tests>
So to underline, now we will not look at perf scripts changes anymore,
and we will also decrease number of random queries to run to 10.
If there is top_level_domains list in the upstream/master, use from the
patched version (this is required to run all performance tests for
upstream/master in the PR).
In #18113 the top_level_domains list copying was moved into
docker/packager/binary/build.sh, this was done to avoid symlinks (since
Dockerfile cannot dereference them).
But the patch was wrong, since it copied into the root (/), which is not
included into performance.tgz and also compare.sh was not modified.
This wasn't showed up with CI checks since the docker image wasn't
updated and it still included that top_level_domains, once it was
modified the image was updated and it became broken.
Cc: @akuzm
