-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Client fault tolerant connection #12
base: managed-feature/automatic-server-choosing
Are you sure you want to change the base?
Client fault tolerant connection #12
Conversation
src/Client/ClientBase.h
Outdated
String host; | ||
std::optional<int> port{}; | ||
friend std::istream & operator>>(std::istream & in, HostPort & hostPort) { | ||
String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use more common way:
String host_with_port;
String delimiter = ":";
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/Client/ClientBase.h
Outdated
in >> host_with_port; | ||
size_t delimiter_pos = host_with_port.find(delimiter); | ||
hostPort.host = host_with_port.substr(0, delimiter_pos); | ||
if (delimiter_pos < host_with_port.length()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
delimiter_pos
!= std::string::npos
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
src/Client/ClientBase.h
Outdated
delimiter = ":"; | ||
in >> host_with_port; | ||
size_t delimiter_pos = host_with_port.find(delimiter); | ||
hostPort.host = host_with_port.substr(0, delimiter_pos); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can move it also inside if
and separately assign hostPort.host
to host_with_port
in else
branch so that not to mess with host_with_port.substr(0, std::string::npos)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
return in; | ||
} | ||
}; | ||
std::vector<HostPort> hosts_ports{}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's better to delete the old host
field
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no host
field in ClientBase
. The reason why I add this field in ClientBase
, you can read our discussion in old PR: #4 (comment)
programs/client/Client.cpp
Outdated
config().setString("host", options["host"].as<std::string>()); | ||
{ | ||
hosts_ports = options["host"].as<std::vector<HostPort>>(); | ||
config().setString("host", hosts_ports[0].host); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it better to set all the hosts and ports instead of just one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
config
can't store std::vector
, only String
, Int
and other simple types. I wrote about this in this PR: #4 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
programs/client/Client.cpp
Outdated
String server_name; | ||
UInt64 server_version_major = 0; | ||
UInt64 server_version_minor = 0; | ||
UInt64 server_version_patch = 0; | ||
|
||
try | ||
String tcp_port_config_key = is_secure ? "tcp_port_secure" : "tcp_port"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be easier to read with if
instead of ternary operator
@@ -512,48 +512,73 @@ catch (...) | |||
|
|||
void Client::connect() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also need rebase on new master and tests
94e48b0
to
962d851
Compare
fix the run command and add example
Before this patch current_user/current_address will be preserved from the previous query. Signed-off-by: Azat Khuzhin <[email protected]>
Co-authored-by: Azat Khuzhin <[email protected]>
Update list-versions.sh, update version_date.tsv
…-for-show-grants Fix checking grants for SHOW GRANTS
Cmake leftovers cleanup
…ssword Add 'clickhouse-client --password' comment to the scripts used in Qui…
Disable data skipping indexes by default for queries with FINAL
3253ae1
to
c1df291
Compare
…rays Fix consecutive backward seeks in seekable read buffers
…database-memory Fix wrong engine in SHOW CREATE DATABASE with engine Memory ClickHouse#34225
Add table function format(format_name, data)
Add options for clickhouse-format.
Add composability to casting and index operators
Fix clang tidy
…-fault-tolerant-connection
Support UUID in MsgPack format
…temp-tables-via-grpc Fix inserting to temporary tables via gRPC.
Update clickhouse-keeper.md
…metadata Better local metadata comparison with ZooKeeper metadata
Fix segfault in schema inference from url
add function addressToLineWithInlines
Fix various issues when projection is enabled by default
Method called on already moved
Use UTC in docker images
Revert "Merge pull request ClickHouse#34373 from ClickHouse/docker-tz"
Add submodule minizip
…-fault-tolerant-connection
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
...
Parameter
host
can accept multiple hosts. In case of unavailability of one of them, the client will try to connect to the next one.Detailed description / Documentation draft:
...
When multiple addresses are passed to the
host
argument and the first one is unavailable, the console client will try to connect to the next one.