HIVE-29429: Support multiple parent classes in HMSHander#6377
Open
dengzhhu653 wants to merge 6 commits intoapache:masterfrom
Open
HIVE-29429: Support multiple parent classes in HMSHander#6377dengzhhu653 wants to merge 6 commits intoapache:masterfrom
dengzhhu653 wants to merge 6 commits intoapache:masterfrom
Conversation
asf-ci-hive
added
tests failed
tests pending
tests unstable
and removed
tests pending
tests failed
labels
There was a problem hiding this comment.
Pull request overview
This PR refactors the Hive Metastore server handler implementation by splitting HMSHandler responsibilities into a layered handler class hierarchy (base/deprecated/transaction/privilege), enabling broader reuse and reducing single-class coupling.
Changes:
- Introduces new handler superclasses (
BaseHandler,DeprecatedHandler,TransactionHandler,PrivilegeHandler) and makesHMSHandlerextend the new chain. - Moves commonly used partition-name parsing helper
getPartValsFromNamefromHMSHandlerintoMetaStoreServerUtils, updating call sites/imports accordingly. - Centralizes thread-local IP address access on
BaseHandlerand adjusts related code/tests; relaxes visibility on a few members needed across packages (MetaStoreInit.getConnectionURL,TransactionalValidationListenerconstructor).
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/IpAddressListener.java | Updates test to use BaseHandler thread-local IP helpers. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreServerUtils.java | Adds getPartValsFromName utility and supporting imports. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java | New shared handler base containing common HMS handler plumbing (init, listeners, metrics, IP thread-local, etc.). |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/DeprecatedHandler.java | New layer to isolate deprecated Thrift APIs. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/TransactionHandler.java | New layer for txn/locking and related listener notifications. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/PrivilegeHandler.java | New layer for role/privilege APIs. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/SetAggrStatsHandler.java | Switches static import of getPartValsFromName to MetaStoreServerUtils. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/AppendPartitionHandler.java | Switches static import of getPartValsFromName to MetaStoreServerUtils. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java | Switches static import of getPartValsFromName to MetaStoreServerUtils. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/DirectSqlUpdatePart.java | Switches static import of getPartValsFromName to MetaStoreServerUtils. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java | Makes constructor public for cross-package instantiation from new handler base. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/TSetIpAddressProcessor.java | Updates to set thread-local IP via BaseHandler. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/RetryingHMSHandler.java | Uses getMethod instead of getDeclaredMethod to find inherited public init(). |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/MetaStoreInit.java | Makes getConnectionURL public for cross-package access. |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandlerContext.java | Stores the handler as BaseHandler in thread-local context (for shutdown notifications). |
| standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java | Refactors HMSHandler to extend PrivilegeHandler and delegates shared logic to new handler layers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...etastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/PrivilegeHandler.java
Show resolved
Hide resolved
...ore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java
Outdated
Show resolved
Hide resolved
...astore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreServerUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...astore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/TransactionHandler.java
Outdated
Show resolved
Hide resolved
...ore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java
Outdated
Show resolved
Hide resolved
...ore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java
Outdated
Show resolved
Hide resolved
...ore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java
Outdated
Show resolved
Hide resolved
...astore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreServerUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...ore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/BaseHandler.java
Show resolved
Hide resolved
|
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?
Why are the changes needed?
Does this PR introduce any user-facing change?
How was this patch tested?