Skip to content

Commit

Permalink
Fix changed IP for https session
Browse files Browse the repository at this point in the history
  • Loading branch information
aalexfvk committed May 26, 2023
1 parent 09d7512 commit 2a2c35e
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 8 deletions.
3 changes: 3 additions & 0 deletions base/poco/Net/include/Poco/Net/HTTPClientSession.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,9 @@ namespace Net

void setResolvedHost(std::string resolved_host) { _resolved_host.swap(resolved_host); }

std::string getResolvedHost() const { return _resolved_host; }
/// Returns the resolved IP address of the target HTTP server.

Poco::UInt16 getPort() const;
/// Returns the port number of the target HTTP server.

Expand Down
29 changes: 21 additions & 8 deletions src/IO/HTTPCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ namespace
if (https)
{
#if USE_SSL
String resolved_host = resolve_host ? DNSResolver::instance().resolveHost(host).toString() : host;
/// Cannot resolve host in advance, otherwise SNI won't work in Poco.
/// For more information about SNI, see the https://en.wikipedia.org/wiki/Server_Name_Indication
auto https_session = std::make_shared<Poco::Net::HTTPSClientSession>(host, port);
if (resolve_host)
https_session->setResolvedHost(DNSResolver::instance().resolveHost(host).toString());
Expand Down Expand Up @@ -184,6 +185,24 @@ namespace
std::mutex mutex;
std::unordered_map<Key, PoolPtr, Hasher> endpoints_pool;

void updateHostIfIpChanged(Entry & session, const String & new_ip)
{
const auto old_ip = session->getResolvedHost().empty() ? session->getHost() : session->getResolvedHost();

if (new_ip != old_ip)
{
session->reset();
if (session->getResolvedHost().empty())
{
session->setHost(new_ip);
}
else
{
session->setResolvedHost(new_ip);
}
}
}

protected:
HTTPSessionPool() = default;

Expand Down Expand Up @@ -238,13 +257,7 @@ namespace

if (resolve_host)
{
/// Host can change IP
const auto ip = DNSResolver::instance().resolveHost(host).toString();
if (ip != session->getHost())
{
session->reset();
session->setHost(ip);
}
updateHostIfIpChanged(session, DNSResolver::instance().resolveHost(host).toString());
}
}
/// Reset the message, once it has been printed,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<clickhouse>
<listen_host>::</listen_host>
</clickhouse>
96 changes: 96 additions & 0 deletions tests/integration/test_https_replication/test_change_ip.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import pytest
from helpers.cluster import ClickHouseCluster
from helpers.test_tools import assert_eq_with_retry

"""
Both ssl_conf.xml and no_ssl_conf.xml have the same port
"""


def _fill_nodes(nodes, shard):
for node in nodes:
node.query(
"""
CREATE DATABASE test;
CREATE TABLE test_table(date Date, id UInt32)
ENGINE = ReplicatedMergeTree('/clickhouse/tables/test{shard}/replicated', '{replica}') PARTITION BY toYYYYMM(date) ORDER BY id;
""".format(
shard=shard, replica=node.name
)
)


cluster = ClickHouseCluster(__file__)
node1 = cluster.add_instance(
"node1",
main_configs=[
"configs/remote_servers.xml",
"configs/listen_host.xml",
"configs/ssl_conf.xml",
"configs/server.crt",
"configs/server.key",
"configs/dhparam.pem",
],
with_zookeeper=True,
ipv6_address="2001:3984:3989::1:1111",
)
node2 = cluster.add_instance(
"node2",
main_configs=[
"configs/remote_servers.xml",
"configs/listen_host.xml",
"configs/ssl_conf.xml",
"configs/server.crt",
"configs/server.key",
"configs/dhparam.pem",
],
with_zookeeper=True,
ipv6_address="2001:3984:3989::1:1112",
)


@pytest.fixture(scope="module")
def both_https_cluster():
try:
cluster.start()

_fill_nodes([node1, node2], 1)

yield cluster

finally:
cluster.shutdown()


def test_replication_when_node_ip_changed(both_https_cluster):
"""
Test for a bug when replication over HTTPS stops working when the IP of the source replica was changed.
node1 is a source node
node2 fethes data from node1
"""
node1.query("truncate table test_table")
node2.query("truncate table test_table")

# First we check, that normal replication works
node1.query(
"INSERT INTO test_table VALUES ('2022-10-01', 1), ('2022-10-02', 2), ('2022-10-03', 3)"
)
assert node1.query("SELECT count(*) from test_table") == "3\n"
assert_eq_with_retry(node2, "SELECT count(*) from test_table", "3")

# We change source node ip
cluster.restart_instance_with_ip_change(node1, "2001:3984:3989::1:7777")

# Put some data to source node1
node1.query(
"INSERT INTO test_table VALUES ('2018-10-01', 4), ('2018-10-02', 4), ('2018-10-03', 6)"
)
# Check that data is placed on node1
assert node1.query("SELECT count(*) from test_table") == "6\n"

# drop DNS cache
node2.query("SYSTEM DROP DNS CACHE")
# Data is fetched
assert_eq_with_retry(node2, "SELECT count(*) from test_table", "6")

0 comments on commit 2a2c35e

Please sign in to comment.