Distributed and remote timeouts #4558
Conversation
code-of-kpp
force-pushed
the
distributred-timeout
branch
from
d322bed to
465aa34
Compare
code-of-kpp
force-pushed
the
distributred-timeout
branch
from
4b6fee9 to
b52f153
Compare
code-of-kpp
force-pushed
the
distributred-timeout
branch
from
b52f153 to
0f80e1c
Compare
code-of-kpp
changed the title
code-of-kpp
force-pushed
the
distributred-timeout
branch
from
b26578d to
4047ad8
Compare
|
? |
|
I've implemented here all the suggestions from developers telegram channel but it doesn't seem to solve the problem. Failing tests are not related to new code. So I can rebease tomorrow morning and see if it is better at least in terms of tests. |
|
One probable issue introduced is client program timeouts - when long operation is running sometimes one can get Timeout exception when it doesn't look like the right thing to happen. |
code-of-kpp
force-pushed
the
distributred-timeout
branch
from
4047ad8 to
b97cee0
Compare
code-of-kpp
force-pushed
the
distributred-timeout
branch
3 times, most recently
from
a1be956 to
3154f07
Compare
maybe it is better to provide some `set_timeouts` method?
…ger than timeouts state
…gs.max_execution_time)` from `Cluster`
code-of-kpp
force-pushed
the
distributred-timeout
branch
2 times, most recently
from
0596276 to
63804d9
Compare
code-of-kpp
force-pushed
the
distributred-timeout
branch
from
63804d9 to
6dbf287
Compare
code-of-kpp
changed the title
| @@ -57,7 +57,6 @@ class Connection : private boost::noncopyable | |||
| Connection(const String & host_, UInt16 port_, | |||
| const String & default_database_, | |||
| const String & user_, const String & password_, | |||
| const ConnectionTimeouts & timeouts_, | |||
There was a problem hiding this comment.
It's just a question. Why you don't want to save old behaviour? There are a lot of places in code, where fixed timeout is OK. Constructor may take default_timeouts, and in each function we can pass non default, otherwise they would use default. Diff would be much smaller, and Connection interface would be more convenient.
There was a problem hiding this comment.
This way it was easier to debug as missing argument or variable is a compilation error while wrong default value requires complicated integration tests. On the other hand I don't think the diff would be much smaller because of additional logic to chose between default value and provided via method argument.
| @@ -55,11 +55,9 @@ class PoolWithFailoverBase : private boost::noncopyable | |||
|
|
|||
| PoolWithFailoverBase( | |||
| NestedPools nested_pools_, | |||
| size_t max_tries_, | |||
Qrator Labs s.r.o. contributes this under the terms of the Apache-2.0 license
related to issue #4211
Category (leave one):
Previously timeouts for clusters and clickhouse-dictionaries were basically immutable after corresponding object creation. This PR tackles this.