skywalker0618 opened a new pull request, #19812:
URL: https://github.com/apache/hudi/pull/19812
### Describe the issue this Pull Request addresses
`HiveQueryDDLExecutor` starts a `SessionState` in its constructor and never
closes it. Hive derives
a session's four scratch directory roots from `hive.session.id` and reclaims
them only in
`SessionState.close()`, so every sync leaves a directory set behind, along
with the session's
function registry and the class loaders it created. A `HiveSyncTool`, and
therefore an executor and
its session, is built per sync, so on a long-running streaming job this
accumulates for the life of
the JVM: `close()` released the metastore client and the Driver, never the
session.
Observed on a Flink job syncing a 5,000-partition table in HiveQL mode: one
unclosed session per
sync cycle, 63 of 63 and 76 of 76 across two runs, each leaving its scratch
directory set on local
disk.
### Summary and Changelog
Users running `hive_sync.mode=hiveql` on a long-lived job stop accumulating
scratch directories and
per-session objects; the session is now released with the rest of the
executor's resources.
- `close()` closes the `SessionState`. It runs after the Driver teardown,
because
`SessionState.close()` detaches the session from the calling thread and
`Driver.destroy()` can
reach `SessionState.get()` while releasing locks, and it runs in a
`finally` so a Driver `close()`
that throws cannot skip it.
- The constructor's error path now uses the same `closeQuietly` helper
instead of its own inline
block. Besides removing the duplication, it widens the caught type from
`IOException` to
`Exception`, so a `RuntimeException` during teardown can no longer mask
the construction failure
that is about to be thrown.
- `updateHiveSQLs` re-asserts its own session before running statements.
This is required by the
change above: `SessionState.close()` calls `detachSession()`, which clears
the thread local for
whichever session is attached, so an executor can no longer assume the
session it started in its
constructor is still current when a second executor on the same thread is
constructed or closed.
`Driver.compile()` dereferences `SessionState.get()` unconditionally, and
Hive documents that a
thread running several sessions must set the current session when
switching between them.
- New `TestHiveQueryDDLExecutorSession` covers the session lifecycle: closed
after the Driver and in
that order, closed even when the Driver `close()` throws, a failing
session close swallowed so it
does not become the caller's problem, and SQL running under the session
the executor started.
### Impact
No public API or config change. Behaviour change is that the HiveQL sync
path now releases its Hive
session when the sync tool is closed, so its scratch directories are
reclaimed instead of being left
until the JVM exits.
### Risk Level
low
Verification: `TestHiveSyncTool` passes in full, 275 tests, which exercises
the HiveQL path
end-to-end against the embedded metastore, plus `TestHiveDriverPool`,
`TestHiveQueryDDLExecutorFailures`, `TestHoodieHiveSyncClientClose` and the
new tests. The session
re-assert was checked by removing it and confirming the new test fails with
`SessionState.get()`
returning null, which is the same NPE in `Driver.compile()` that the full
suite surfaced in teardown
before the re-assert was added.
### Documentation Update
none
### Contributor's checklist
- [x] Read through [contributor's
guide](https://hudi.apache.org/contribute/how-to-contribute)
- [x] Enough context is provided in the sections above
- [x] Adequate tests were added if applicable
--
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]
