feat: make Hive2Namespace and Hive3Namespace Closeable#137
Merged
Conversation
f1e968d to
61a8bd9
Compare
Both Hive2Namespace and Hive3Namespace own a Hive*ClientPool, which implements Closeable and holds a pool of live Hive Metastore client connections. Neither namespace exposed any way to release that pool. Every other client-backed implementation in this repo already implements Closeable and closes its underlying client/pool in close() (Iceberg, Glue, Unity, Polaris); the two Hive namespaces were the only exceptions, so a caller holding a Hive*Namespace has no way to release its metastore connections and leaks them when the instance is discarded. Implement Closeable on both classes with a close() that closes the client pool. close() is null-safe (a no-op when initialize() was never called) and idempotent (ClientPoolImpl.close() already guards with a closed flag). Add unit tests for both behaviors in TestHive2Namespace and TestHive3Namespace. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
61a8bd9 to
9a2d75e
Compare
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
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.
What
Make
Hive2NamespaceandHive3Namespaceimplementjava.io.Closeableand add aclose()that closes the underlyingHive*ClientPool.Why
Both Hive namespaces own a
Hive*ClientPool— whichimplements Closeableand holds a pool of live Hive Metastore client connections — but neither exposed any way to release it. So a consumer holding aHive*Namespacecannot release its metastore connections and leaks them when the instance is discarded.Every other client-backed implementation in this repo already handles this:
implements Closeable?close()?IcebergNamespacerestClient)GlueNamespaceglueClient)UnityNamespacerestClient)PolarisNamespacerestClient)Hive2NamespaceclientPool)Hive3NamespaceclientPool)The two Hive namespaces were the only exceptions; this brings them in line with the rest.
Changes
Hive2Namespace/Hive3Namespace:implements LanceNamespace, Closeable; addclose()that closesclientPool.close()is null-safe (no-op ifinitialize()was never called) and idempotent (ClientPoolImpl.close()already guards with aclosedflag).TestHive2NamespaceandTestHive3Namespace:testCloseReleasesClientPool(initialize → use → close twice) andtestCloseBeforeInitializeIsSafe.Testing
./mvnw spotless:check -pl lance-namespace-hive2and-pl lance-namespace-hive3— pass../mvnw test -pl lance-namespace-hive2 -Dtest='!*Integration'— 17 tests, 0 failures (incl. the 2 new tests)../mvnw test -pl lance-namespace-hive3 -Dtest='!*Integration'— 25 tests, 0 failures (incl. the 2 new tests).