feat(hive): add HmsClient connection lifecycle and URI parsing#796
Open
MisterRaindrop wants to merge 5 commits into
Open
feat(hive): add HmsClient connection lifecycle and URI parsing#796MisterRaindrop wants to merge 5 commits into
MisterRaindrop wants to merge 5 commits into
Conversation
MisterRaindrop
force-pushed
the
feat/hive-hms-client
branch
from
e2817ce to
31299ec
Compare
wgtmac
reviewed
Jul 2, 2026
wgtmac
reviewed
Jul 2, 2026
MisterRaindrop
force-pushed
the
feat/hive-hms-client
branch
from
31299ec to
becbf2b
Compare
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, with whitespace around each
segment stripped. Returns InvalidArgument for empty hosts,
non-numeric / out-of-range / trailing-garbage ports, and empty
list segments.
* HmsClient::Connect(): wires TSocket -> TBufferedTransport /
TFramedTransport (selected by HiveCatalogProperties::ThriftTransport)
-> TBinaryProtocol -> ThriftHiveMetastoreClient. Connect / socket
timeouts are parsed through Result-returning accessors so a malformed
value yields InvalidArgument instead of throwing. Multi-endpoint URIs
are rejected with InvalidArgument until HA failover is implemented.
Connection failures are caught and translated into ErrorKind::kIOError
so callers never see raw Thrift exceptions. The dtor
best-effort-closes the transport.
* HiveCatalogProperties gains ConnectTimeoutMs() / SocketTimeoutMs()
accessors that parse the millisecond values via StringUtils::ParseNumber
and reject negative or malformed input.
* 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 19 GoogleTest cases:
12 covering ParseHmsUris and 7 covering HmsClient::Connect's error
paths (missing / bad URI, invalid transport, multi-endpoint,
malformed connect / socket timeouts, 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.
Part of the iceberg-cpp HiveCatalog port.
Add an Ubuntu CI job that configures with -DICEBERG_BUILD_HIVE=ON (via ICEBERG_EXTRA_CMAKE_ARGS) so the iceberg_hive library and hive_catalog_test are compiled and run, and enable ICEBERG_BUILD_HIVE in the cpp-linter build so clang-tidy resolves hms_client.cc's Thrift include paths from the compilation database. Thrift comes from Arrow's bundled build (ICEBERG_BUNDLE_THRIFT=ON, the default), so no system Thrift install is required.
MisterRaindrop
force-pushed
the
feat/hive-hms-client
branch
from
becbf2b to
dd4f08d
Compare
Contributor
Author
|
@wgtmac It is ready, please review it |
Declare connect-timeout-ms / socket-timeout-ms as Entry<int> read via Get(), matching how every other numeric property in the repo (TableProperties, ReaderProperties, WriterProperties) is defined and consumed. Drop the bespoke string storage, ParseTimeoutMs helper and the Result<int> accessors that made these two properties handle parse errors differently from the rest of the codebase.
Arrow's bundled Thrift download lists only the live Apache mirrors (closer.lua / dlcdn), which drop older releases. Thrift 0.22.0 (the version Arrow 24.0.0 pins) has been removed from them and now 404s, breaking every bundled-Arrow build in CI (Parquet requires Thrift, so this hits non-Hive jobs too). Set ARROW_THRIFT_URL to archive.apache.org, which retains all releases, mirroring the archive fallback already used for the Arrow and nanoarrow tarballs.
hms_client.cc used Result / InvalidArgument / IOError, HiveCatalogProperties / HiveThriftTransport and std::size_t only through transitive includes; the test used ErrorKind the same way. Include iceberg/result.h, the hive properties header and <cstddef> directly so misc-include-cleaner passes.
wgtmac
approved these changes
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ParseHmsUris()— tolerant HMS URI parser:thrift://host:port, barehost:port, default port 9083, and comma-separated HA lists. Rejects empty hosts and invalid ports.HmsClient::Connect()— sets up theTSocket → TTransport → TBinaryProtocol → ThriftHiveMetastoreClientchain (transport and timeouts fromHiveCatalogProperties), translating Thrift exceptions tokIOError. Thrift types stay out of the public header via a pImpl.Ubuntu Hivejob builds with-DICEBERG_BUILD_HIVE=ON(bundled Thrift) and runshive_catalog_test.