WIP: HIVE-29697: Add support for Persistable Sessions - #6687
Open
ayushtkn wants to merge 4 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an initial implementation of “Persistable Sessions” for HiveServer2, allowing HS2 to recover session state (configs, jars, temp tables) from an external shared store (ZooKeeper/Redis) after failover.
Changes:
- Introduces a new
service-session-storemodule withSessionStateStoreplus ZooKeeper and Redis implementations and a JSON-serializableHiveSessionSnapshot. - Integrates snapshot save/delete + session recovery logic into HS2
SessionManager, with a configurable fetch strategy. - Adds utilities + hooks (
PersistableSessionUtils,HiveSessionImplnotifications) and new unit/integration tests covering recovery and strategies.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| service/src/java/org/apache/hive/service/cli/session/SessionManager.java | Initializes the store, saves/deletes snapshots, and attempts session recovery/sync from the remote store. |
| service/src/java/org/apache/hive/service/cli/session/PersistableSessionUtils.java | Snapshot capture/hydration helpers and store save/delete helpers. |
| service/src/java/org/apache/hive/service/cli/session/HiveSessionProxy.java | Exposes the base session for unwrapping during snapshot capture. |
| service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java | Calls snapshot save notifications on detected “state-changing” commands and exposes captureSnapshot(). |
| service/pom.xml | Adds dependency on the new session-store module. |
| service-session-store/src/main/java/org/apache/hive/service/cli/session/store/SessionStateStore.java | Defines the SPI for snapshot persistence. |
| service-session-store/src/main/java/org/apache/hive/service/cli/session/store/HiveSessionSnapshot.java | DTO for persisted session state (Jackson-serializable). |
| service-session-store/src/main/java/org/apache/hive/service/cli/session/store/ZooKeeperSessionStateStore.java | ZooKeeper-backed persistence implementation. |
| service-session-store/src/main/java/org/apache/hive/service/cli/session/store/RedisSessionStateStore.java | Redis-backed persistence implementation (TTL-based). |
| service-session-store/src/test/java/org/apache/hive/service/cli/session/store/TestSessionStateStoreBase.java | Base unit tests for store save/get/delete behavior. |
| service-session-store/src/test/java/org/apache/hive/service/cli/session/store/TestZooKeeperSessionStateStore.java | ZooKeeper store unit tests using Curator TestingServer. |
| service-session-store/src/test/java/org/apache/hive/service/cli/session/store/TestRedisSessionStateStore.java | Redis store unit tests using Testcontainers. |
| service-session-store/pom.xml | New module POM and dependencies (Jackson/Curator/Jedis). |
| pom.xml | Adds module + jedis.version property. |
| itests/hive-unit/src/test/java/org/apache/hive/service/cli/session/TestPersistableSessionBase.java | Integration tests that validate failover recovery + fetch strategies. |
| itests/hive-unit/src/test/java/org/apache/hive/service/cli/session/TestPersistableSessionWithZooKeeper.java | ZooKeeper-backed integration test wiring. |
| itests/hive-unit/src/test/java/org/apache/hive/service/cli/session/TestPersistableSessionWithRedis.java | Redis-backed integration test wiring. |
| itests/hive-unit/pom.xml | Adds test dependencies for the new module and Jedis. |
| common/src/java/org/apache/hadoop/hive/conf/HiveConf.java | Adds new ConfVars for store class, strategy, and TTL. |
Suppressed comments (1)
service/src/java/org/apache/hive/service/cli/session/PersistableSessionUtils.java:190
- When restoring temp tables during hydration, the warn log drops the exception stack trace (only logs e.getMessage()). That makes restore failures hard to debug, especially when a DDL fails due to missing jars/serde/permissions. Logging the exception object preserves useful context.
for (Map.Entry<String, String> entry : snapshot.getTempTableDefinitions().entrySet()) {
try {
session.executeStatement(entry.getValue(), null);
} catch (Exception e) {
LOG.warn("Failed to restore temporary table {}: {}", entry.getKey(), e.getMessage());
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
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 changes were proposed in this pull request?
Add support for Persistable Sessions in Hive
Why are the changes needed?
In case of HS2 failures, the Session should be preserved for better HS2 HA
Does this PR introduce any user-facing change?
In case there is a HS2 crash, the other HS2 can reload the session, so session is preserved and the query retry doesn't fail with
Invalid SessionHandlelikeHow was this patch tested?
UT, Manually for REDIS via K8s Operator
Killed the HS2 pod which had the session with
Checked running a query and if the temp table and configs set survived
