Skip to content

Commit 8e08b6e

Browse files
feat(hive): HmsClient connection lifecycle and URI parsing
Add the first iceberg_hive code that actually talks to a Hive Metastore over Thrift, mirroring the structure of iceberg-rust's HmsCatalog::new while keeping every Thrift type out of the public header via a pImpl. * HmsEndpoint + ParseHmsUris(): tolerant URI parser that accepts `thrift://host:port`, bare `host:port`, missing-port (defaults to 9083), and comma-separated lists for HA failover, with whitespace around each segment stripped. Returns InvalidArgument for empty hosts, non-numeric or out-of-range ports, and empty list segments. * HmsClient::Connect(): wires TSocket -> TBufferedTransport / TFramedTransport (selected by HiveCatalogProperties::ThriftTransport) -> TBinaryProtocol -> ThriftHiveMetastoreClient. Connect / socket timeouts come from the properties. Connection failures are caught and translated into ErrorKind::kIOError so callers never see raw Thrift exceptions. The dtor best-effort-closes the transport. * HmsClient::Impl holds the Thrift state in destruction-order (client -> protocol -> transport -> socket) so teardown is clean. * src/iceberg/test/hms_client_test.cc adds 15 GoogleTest cases: 11 covering ParseHmsUris (single, multi-HA, default port, scheme prefix, whitespace, empty/bad host/port edges) and 4 covering HmsClient::Connect's error paths (missing URI, bad URI, invalid transport mode, unreachable HMS). * src/iceberg/test/CMakeLists.txt gains an add_hive_iceberg_test() helper (mirroring add_rest_iceberg_test) and the hive_catalog_test target gated on ICEBERG_BUILD_HIVE. `ctest --test-dir build --output-on-failure` now reports 17/17 passing (16 previous + 1 new hive_catalog_test with 15 cases inside). Part of the iceberg-cpp HiveCatalog port (C06).
1 parent d1df6e8 commit 8e08b6e

5 files changed

Lines changed: 452 additions & 1 deletion

File tree

src/iceberg/catalog/hive/CMakeLists.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ if(NOT TARGET thrift::thrift)
5454
"-DICEBERG_BUNDLE_THRIFT=OFF against a system Thrift install.")
5555
endif()
5656

57-
set(ICEBERG_HIVE_SOURCES hive_catalog.cc hive_catalog_properties.cc
57+
set(ICEBERG_HIVE_SOURCES hive_catalog.cc hive_catalog_properties.cc hms_client.cc
5858
${ICEBERG_HIVE_THRIFT_GEN_SOURCES})
5959

6060
set(ICEBERG_HIVE_STATIC_BUILD_INTERFACE_LIBS)
Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#include "iceberg/catalog/hive/hms_client.h"
21+
22+
#include <cctype>
23+
#include <charconv>
24+
#include <memory>
25+
#include <string>
26+
#include <string_view>
27+
#include <system_error>
28+
#include <utility>
29+
#include <vector>
30+
31+
#include <thrift/Thrift.h>
32+
#include <thrift/protocol/TBinaryProtocol.h>
33+
#include <thrift/transport/TBufferTransports.h>
34+
#include <thrift/transport/TSocket.h>
35+
#include <thrift/transport/TTransportException.h>
36+
37+
#include "ThriftHiveMetastore.h"
38+
#include "iceberg/util/macros.h"
39+
40+
namespace iceberg::hive {
41+
42+
namespace {
43+
44+
constexpr std::string_view kThriftPrefix = "thrift://";
45+
46+
std::string_view StripScheme(std::string_view spec) {
47+
if (spec.starts_with(kThriftPrefix)) {
48+
return spec.substr(kThriftPrefix.size());
49+
}
50+
return spec;
51+
}
52+
53+
std::string_view Trim(std::string_view spec) {
54+
while (!spec.empty() && (std::isspace(static_cast<unsigned char>(spec.front())) != 0)) {
55+
spec.remove_prefix(1);
56+
}
57+
while (!spec.empty() && (std::isspace(static_cast<unsigned char>(spec.back())) != 0)) {
58+
spec.remove_suffix(1);
59+
}
60+
return spec;
61+
}
62+
63+
Result<HmsEndpoint> ParseSingleEndpoint(std::string_view spec) {
64+
spec = Trim(spec);
65+
spec = StripScheme(spec);
66+
spec = Trim(spec);
67+
if (spec.empty()) {
68+
return InvalidArgument("Empty HMS endpoint in URI list.");
69+
}
70+
71+
HmsEndpoint endpoint;
72+
const auto colon = spec.rfind(':');
73+
if (colon == std::string_view::npos) {
74+
endpoint.host = std::string(spec);
75+
endpoint.port = kDefaultHmsPort;
76+
return endpoint;
77+
}
78+
79+
endpoint.host = std::string(spec.substr(0, colon));
80+
if (endpoint.host.empty()) {
81+
return InvalidArgument("HMS endpoint has empty host: '{}'.", spec);
82+
}
83+
84+
const auto port_str = spec.substr(colon + 1);
85+
if (port_str.empty()) {
86+
endpoint.port = kDefaultHmsPort;
87+
return endpoint;
88+
}
89+
90+
int port = 0;
91+
const auto* const port_end = port_str.data() + port_str.size();
92+
const auto [ptr, ec] = std::from_chars(port_str.data(), port_end, port);
93+
if (ec != std::errc() || ptr != port_end || port <= 0 || port > 65535) {
94+
return InvalidArgument("Invalid HMS port in endpoint '{}'.", spec);
95+
}
96+
endpoint.port = port;
97+
return endpoint;
98+
}
99+
100+
} // namespace
101+
102+
Result<std::vector<HmsEndpoint>> ParseHmsUris(std::string_view uri) {
103+
std::vector<HmsEndpoint> endpoints;
104+
if (Trim(uri).empty()) {
105+
return InvalidArgument("HMS URI is empty.");
106+
}
107+
108+
std::size_t pos = 0;
109+
while (pos <= uri.size()) {
110+
const auto comma = uri.find(',', pos);
111+
const auto piece = uri.substr(
112+
pos, comma == std::string_view::npos ? std::string_view::npos : comma - pos);
113+
ICEBERG_ASSIGN_OR_RAISE(auto endpoint, ParseSingleEndpoint(piece));
114+
endpoints.push_back(std::move(endpoint));
115+
if (comma == std::string_view::npos) {
116+
break;
117+
}
118+
pos = comma + 1;
119+
}
120+
return endpoints;
121+
}
122+
123+
// Fields are declared in dependency order (socket <- transport <- protocol
124+
// <- client) so the client tears down before the transport it borrows.
125+
class HmsClient::Impl {
126+
public:
127+
std::shared_ptr<apache::thrift::transport::TSocket> socket;
128+
std::shared_ptr<apache::thrift::transport::TTransport> transport;
129+
std::shared_ptr<apache::thrift::protocol::TProtocol> protocol;
130+
std::unique_ptr<Apache::Hadoop::Hive::ThriftHiveMetastoreClient> client;
131+
};
132+
133+
HmsClient::HmsClient(std::unique_ptr<Impl> impl) : impl_(std::move(impl)) {}
134+
135+
HmsClient::~HmsClient() {
136+
if (impl_ && impl_->transport && impl_->transport->isOpen()) {
137+
try {
138+
impl_->transport->close();
139+
} catch (const apache::thrift::TException&) {
140+
// Best-effort close on teardown; ignore exceptions.
141+
}
142+
}
143+
}
144+
145+
Result<std::unique_ptr<HmsClient>> HmsClient::Connect(
146+
const HiveCatalogProperties& config) {
147+
ICEBERG_ASSIGN_OR_RAISE(auto uri, config.Uri());
148+
ICEBERG_ASSIGN_OR_RAISE(auto endpoints, ParseHmsUris(uri));
149+
ICEBERG_ASSIGN_OR_RAISE(auto transport_mode, config.ThriftTransport());
150+
151+
// HA failover beyond the first endpoint is left for a later commit.
152+
const HmsEndpoint& endpoint = endpoints.front();
153+
const int connect_timeout_ms = config.Get(HiveCatalogProperties::kConnectTimeoutMs);
154+
const int socket_timeout_ms = config.Get(HiveCatalogProperties::kSocketTimeoutMs);
155+
156+
auto socket =
157+
std::make_shared<apache::thrift::transport::TSocket>(endpoint.host, endpoint.port);
158+
socket->setConnTimeout(connect_timeout_ms);
159+
socket->setRecvTimeout(socket_timeout_ms);
160+
socket->setSendTimeout(socket_timeout_ms);
161+
162+
std::shared_ptr<apache::thrift::transport::TTransport> transport;
163+
switch (transport_mode) {
164+
case HiveThriftTransport::kBuffered:
165+
transport = std::make_shared<apache::thrift::transport::TBufferedTransport>(socket);
166+
break;
167+
case HiveThriftTransport::kFramed:
168+
transport = std::make_shared<apache::thrift::transport::TFramedTransport>(socket);
169+
break;
170+
}
171+
172+
auto protocol = std::make_shared<apache::thrift::protocol::TBinaryProtocol>(transport);
173+
auto client =
174+
std::make_unique<Apache::Hadoop::Hive::ThriftHiveMetastoreClient>(protocol);
175+
176+
try {
177+
transport->open();
178+
} catch (const apache::thrift::transport::TTransportException& e) {
179+
return IOError("Failed to connect to HMS at {}:{} : {}", endpoint.host, endpoint.port,
180+
e.what());
181+
} catch (const apache::thrift::TException& e) {
182+
return IOError("Thrift error contacting HMS at {}:{} : {}", endpoint.host,
183+
endpoint.port, e.what());
184+
}
185+
186+
auto impl = std::make_unique<HmsClient::Impl>();
187+
impl->socket = std::move(socket);
188+
impl->transport = std::move(transport);
189+
impl->protocol = std::move(protocol);
190+
impl->client = std::move(client);
191+
return std::unique_ptr<HmsClient>(new HmsClient(std::move(impl)));
192+
}
193+
194+
} // namespace iceberg::hive
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
#pragma once
21+
22+
#include <memory>
23+
#include <string>
24+
#include <string_view>
25+
#include <vector>
26+
27+
#include "iceberg/catalog/hive/hive_catalog_properties.h"
28+
#include "iceberg/catalog/hive/iceberg_hive_export.h"
29+
#include "iceberg/result.h"
30+
31+
/// \file iceberg/catalog/hive/hms_client.h
32+
/// \brief Thin wrapper around the generated Hive Metastore Thrift client.
33+
///
34+
/// Thrift types are kept out of the public interface via a pImpl.
35+
36+
namespace iceberg::hive {
37+
38+
/// \brief Hive's well-known metastore port, used when an HMS URI omits one.
39+
inline constexpr int kDefaultHmsPort = 9083;
40+
41+
/// \brief A single host:port pair parsed from an HMS URI.
42+
///
43+
/// HMS URIs commonly take one of the forms:
44+
/// * `thrift://host:port` (Java HiveCatalog convention)
45+
/// * `host:port` (iceberg-rust HmsCatalog convention)
46+
/// * comma-separated list of either form for HA failover
47+
///
48+
/// `port` defaults to 9083 (Hive's well-known metastore port) when the
49+
/// caller omits an explicit port.
50+
struct ICEBERG_HIVE_EXPORT HmsEndpoint {
51+
std::string host;
52+
int port = kDefaultHmsPort;
53+
};
54+
55+
/// \brief Parse an HMS URI string into one or more endpoints.
56+
///
57+
/// Each comma-separated segment is treated as an independent endpoint;
58+
/// surrounding whitespace and an optional `thrift://` scheme prefix are
59+
/// stripped. Returns an InvalidArgument error if any segment fails to
60+
/// produce a non-empty host and a port within (0, 65535].
61+
ICEBERG_HIVE_EXPORT Result<std::vector<HmsEndpoint>> ParseHmsUris(std::string_view uri);
62+
63+
/// \brief A live connection to a Hive Metastore over Thrift.
64+
///
65+
/// Construction goes through `Connect`, which parses the URI, selects the
66+
/// transport and opens it so that configuration errors surface as
67+
/// `iceberg::Error` rather than C++ exceptions.
68+
class ICEBERG_HIVE_EXPORT HmsClient {
69+
public:
70+
~HmsClient();
71+
72+
HmsClient(const HmsClient&) = delete;
73+
HmsClient& operator=(const HmsClient&) = delete;
74+
HmsClient(HmsClient&&) = delete;
75+
HmsClient& operator=(HmsClient&&) = delete;
76+
77+
/// \brief Connect to the Hive Metastore described by `config`.
78+
///
79+
/// The first endpoint listed in `config.Uri()` is used; HA failover
80+
/// to subsequent endpoints is left to a future commit.
81+
static Result<std::unique_ptr<HmsClient>> Connect(const HiveCatalogProperties& config);
82+
83+
private:
84+
class Impl;
85+
std::unique_ptr<Impl> impl_;
86+
87+
explicit HmsClient(std::unique_ptr<Impl> impl);
88+
};
89+
90+
} // namespace iceberg::hive

src/iceberg/test/CMakeLists.txt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -316,3 +316,27 @@ if(ICEBERG_BUILD_REST)
316316
util/docker_compose_util.cc)
317317
endif()
318318
endif()
319+
320+
if(ICEBERG_BUILD_HIVE)
321+
function(add_hive_iceberg_test test_name)
322+
set(options)
323+
set(oneValueArgs)
324+
set(multiValueArgs SOURCES)
325+
cmake_parse_arguments(ARG
326+
"${options}"
327+
"${oneValueArgs}"
328+
"${multiValueArgs}"
329+
${ARGN})
330+
331+
add_executable(${test_name})
332+
target_include_directories(${test_name} PRIVATE "${CMAKE_BINARY_DIR}/iceberg/test/")
333+
target_sources(${test_name} PRIVATE ${ARG_SOURCES})
334+
target_link_libraries(${test_name} PRIVATE GTest::gmock_main iceberg_hive_static)
335+
if(MSVC_TOOLCHAIN)
336+
target_compile_options(${test_name} PRIVATE /bigobj)
337+
endif()
338+
add_test(NAME ${test_name} COMMAND ${test_name})
339+
endfunction()
340+
341+
add_hive_iceberg_test(hive_catalog_test SOURCES hms_client_test.cc)
342+
endif()

0 commit comments

Comments
 (0)