This may cause troubles, like forgetting to pass arguments, and there
are few places in the code (see the upcomming patch).
I doubt that this will make any performance changes, since the check
should be compile time.
And anyway Exception is an exceptional situation which should be rare
(there is no such code with single argument for logging, while logging
is more common).
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
The problem with --foreground option is that it send the signal only to
the process that had been spawned by timeout(1), while it can create
lots of children, and when you killing parent you are closing pipes and
childrens will get EPIPE, like in [1].
[1]: https://s3.amazonaws.com/clickhouse-test-reports/0/069f8bbb2f48541cc736903e1da5459fa2c27da0/stateless_tests__debug__%5B2_5%5D.html
Another problem is that now child process will finish correctly, which
may also print some errors like QUERY_WAS_CANCELLED (see [2]).
[2]: https://s3.amazonaws.com/clickhouse-test-reports/0/ef66714bf20042ba9cb5d59b7839befe26110b93/stateless_tests__release__analyzer_.html
In general this is not required actually, since all timeout invocations
uses timeout value less then the default test limit (10min). But it may
leave some processes in case of overriding this limit, i.e.
`clickhouse-test --timeout 1`
So to workaround this at least somehow, let's send SIGTERM and only
after some timeout (here I use 0.1), SIGKILL. This will give at least
some ability to terminate all childrens that had been spawned by
timeout(1).
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
One of such cases is 02479_race_condition_between_insert_and_droppin_mv
[1], yes it can be fixed (by using fixed number of iterations, or with
some bash trickery), but it is better to fix them completelly,
eventually such tests will be submitted and pass review anyway.
By allocating process group for each test we can kill all the processes
in this process group, and this what this patch does.
This will also fix some test hangs (like in [1]) as well as some
possible issues in stress tests.
[1]: https://s3.amazonaws.com/clickhouse-test-reports/0/e2c1230b00386c4d0096a245396ab3be7ce60950/stateless_tests__release__analyzer_/run.log
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
(cherry picked from commit 72fa58e192)
One of such cases is 02479_race_condition_between_insert_and_droppin_mv
[1], yes it can be fixed (by using fixed number of iterations, or with
some bash trickery), but it is better to fix them completelly,
eventually such tests will be submitted and pass review anyway.
By allocating process group for each test we can kill all the processes
in this process group, and this what this patch does.
This will also fix some test hangs (like in [1]) as well as some
possible issues in stress tests.
[1]: https://s3.amazonaws.com/clickhouse-test-reports/0/e2c1230b00386c4d0096a245396ab3be7ce60950/stateless_tests__release__analyzer_/run.log
Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>
