PDGGK opened a new issue, #39710:
URL: https://github.com/apache/beam/issues/39710
### What happened?
`HBaseIO` releases its resources as consecutive, unguarded statements in
three places. If an earlier `close()` throws, everything after it is skipped.
All line numbers are against `master` today.
**1. `HBaseIO.java:550-560` — `HBaseReader.close()`**
```java
if (scanner != null) { scanner.close(); scanner = null; }
if (connection != null) { connection.close(); connection = null; }
```
A throwing `scanner.close()` leaks the `Connection`.
**2. `HBaseIO.java:766-776` — `HBaseWriterFn.tearDown()`**
```java
if (mutator != null) { mutator.close(); mutator = null; }
if (connection != null) { connection.close(); connection = null; }
```
This is the sharpest of the three, because `mutator.close()` is *expected*
to be able to throw. `BufferedMutator.close()` is documented as:
> Performs a `flush()` and releases any resources held.
> `@throws IOException` if a remote or network exception occurs.
So any bundle whose final buffered flush fails — a down region server, a
network blip — skips `connection.close()` and leaks the whole `Connection`. The
connection here is created per-`@Setup` via
`ConnectionFactory.createConnection` (`:744`), so this is a real per-worker
leak, not a shared handle.
**3. `HBaseIO.java:931-940` — `HBaseRowMutationWriterFn.tearDown()` — worst
consequence**
```java
if (table != null) { table.close(); table = null; }
HBaseSharedConnection.close(configuration);
```
Here the skipped call is not an ordinary `close()`, it is a
**reference-count decrement**. `HBaseSharedConnection` keeps a `static
HashMap<String, Pair<Connection, Integer>> connectionPool`
(`HBaseSharedConnection.java:46`); `getOrCreate` increments the count (`:80`)
and `close(Configuration)` decrements it, closing the underlying `Connection`
only once it reaches zero (`:119`, `:128-129`).
So a throwing `table.close()` means the count is never decremented. Because
the map is `static`, that entry — and its ZooKeeper session and RPC threads —
then survives for the lifetime of the JVM, and every later `getOrCreate` hands
back the same permanently unreleasable connection. A leak here is not bounded
by the bundle or even the worker's use of the sink.
### What I'd propose
Make each teardown run every step unconditionally while preserving the
*first* failure, attaching later ones as suppressed, so the caller still sees
the error that actually broke the job rather than a teardown symptom. A plain
nested `try/finally` would guarantee the calls happen but would silently swap
which exception propagates, so it is only half a fix.
I'm happy to submit the PR if this looks right.
### Issue Priority
Priority: 2 (default / most bugs should be filed as P2)
### Issue Components
- [x] Component: Java SDK
- [x] Component: IO connector
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
