You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by "wecharyu (via GitHub)" <gi...@apache.org> on 2023/04/21 22:07:52 UTC
[GitHub] [hive] wecharyu opened a new pull request, #4257: HIVE-27284: Make HMSHandler proxy pluggable
wecharyu opened a new pull request, #4257:
URL: https://github.com/apache/hive/pull/4257
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
1. make `HMSHandler` Proxy configurable by new metastore conf **"metastore.hmshandler.proxy"**
2. provide `AbstractHMSHandlerProxy` to extend customized handler
3. clean some hacked test codes in `HMSHandler`
### Why are the changes needed?
1. make the handler of metastore flexible to config and extend
2. improve the code of `HMSHandler`
### Does this PR introduce _any_ user-facing change?
Yes, user can use new conf and api to extend their own handler.
### How was this patch tested?
Passing all existing tests.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1223026996
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -874,6 +876,9 @@ public enum ConfVars {
"testing only."),
HMS_HANDLER_INTERVAL("metastore.hmshandler.retry.interval", "hive.hmshandler.retry.interval",
2000, TimeUnit.MILLISECONDS, "The time between HMSHandler retry attempts on failure."),
+ HMS_HANDLER_PROXY_CLASS("metastore.hmshandler.proxy", "hive.metastore.hmshandler.proxy",
Review Comment:
do we really need a config for that, since METASTORE_RETRYING_HANDLER_CLASS it the only implementation at this moment?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1223031081
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AbstractHMSHandlerProxy.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.MetaStoreInit.MetaStoreInitData;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.metrics.PerfLogger;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractHMSHandlerProxy implements InvocationHandler {
+ protected final MetaStoreInitData metaStoreInitData = new MetaStoreInitData();
+ protected final IHMSHandler baseHandler;
+ protected final Configuration origConf; // base configuration
+ protected final Configuration activeConf; // active configuration
+ protected final boolean reloadConf;
+ protected final long timeout;
+
+ public AbstractHMSHandlerProxy(Configuration conf, IHMSHandler baseHandler, boolean local)
+ throws MetaException {
+ this.origConf = conf;
+ this.baseHandler = baseHandler;
+ if (local) {
+ baseHandler.setConf(origConf); // tests expect configuration changes applied directly to metastore
+ }
+ activeConf = baseHandler.getConf();
+ reloadConf = MetastoreConf.getBoolVar(origConf, ConfVars.HMS_HANDLER_FORCE_RELOAD_CONF);
+ timeout = MetastoreConf.getTimeVar(origConf,
+ ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
+
+ // This has to be called before initializing the instance of HMSHandler
+ // Using the hook on startup ensures that the hook always has priority
+ // over settings in *.xml. The thread local conf needs to be used because at this point
+ // it has already been initialized using hiveConf.
+ MetaStoreInit.updateConnectionURL(origConf, getActiveConf(), null, metaStoreInitData);
+ initBaseHandler();
+ }
+
+ static class Result {
Review Comment:
`private static final`
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1533538693
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4257)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ merged pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ merged PR #4257:
URL: https://github.com/apache/hive/pull/4257
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1518416780
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4257)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1225331918
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -874,6 +876,9 @@ public enum ConfVars {
"testing only."),
HMS_HANDLER_INTERVAL("metastore.hmshandler.retry.interval", "hive.hmshandler.retry.interval",
2000, TimeUnit.MILLISECONDS, "The time between HMSHandler retry attempts on failure."),
+ HMS_HANDLER_PROXY_CLASS("metastore.hmshandler.proxy", "hive.metastore.hmshandler.proxy",
Review Comment:
👍
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1601117859
> > @dengzhhu653, do you have any concerns with the current approach?
>
> I'm +1 on the idea of removing test codes from main routine. Rather than defining a template method, I'd like to introduce a hook for this purpose, unless we would introduce complex strategies around the `method.invoke(proxy, args)` in the future.
I think, regardless if we introduce new custom strategies, the current patch makes the code more readable and extensible/flexible.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1518520796
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4257)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1582560762
@wecharyu would you be able to rebase?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] wecharyu commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1223303101
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -874,6 +876,9 @@ public enum ConfVars {
"testing only."),
HMS_HANDLER_INTERVAL("metastore.hmshandler.retry.interval", "hive.hmshandler.retry.interval",
2000, TimeUnit.MILLISECONDS, "The time between HMSHandler retry attempts on failure."),
+ HMS_HANDLER_PROXY_CLASS("metastore.hmshandler.proxy", "hive.metastore.hmshandler.proxy",
Review Comment:
This config will make Meta Store Server more easy to extend other proxy strategy without modifying the main server code, so I highly recommend this config.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1586558850
Thanks for the contribution, @wecharyu!
For me, the `AbstractHMSHandlerProxy` provides an abstract method for his children to have chances to perform some special work on calling `proxy`, I would prefer a hook for this case, for example, we can introduce a hook like:
```
interface RetryingHMSHandlerHook {
default void preCall(proxy, method, args) throws MetaException {
}
default void postCall(proxy, method, args, result) {
}
// more interfaces if needed....
}
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1518607682
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4257)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [14 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] wecharyu commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1533349412
@deniskuzZ @saihemanth-cloudera @veghlaci05: Could you help review this PR?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1223044780
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AbstractHMSHandlerProxy.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.MetaStoreInit.MetaStoreInitData;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.metrics.PerfLogger;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractHMSHandlerProxy implements InvocationHandler {
+ protected final MetaStoreInitData metaStoreInitData = new MetaStoreInitData();
+ protected final IHMSHandler baseHandler;
+ protected final Configuration origConf; // base configuration
+ protected final Configuration activeConf; // active configuration
+ protected final boolean reloadConf;
+ protected final long timeout;
+
+ public AbstractHMSHandlerProxy(Configuration conf, IHMSHandler baseHandler, boolean local)
+ throws MetaException {
+ this.origConf = conf;
+ this.baseHandler = baseHandler;
+ if (local) {
+ baseHandler.setConf(origConf); // tests expect configuration changes applied directly to metastore
+ }
+ activeConf = baseHandler.getConf();
+ reloadConf = MetastoreConf.getBoolVar(origConf, ConfVars.HMS_HANDLER_FORCE_RELOAD_CONF);
+ timeout = MetastoreConf.getTimeVar(origConf,
+ ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
+
+ // This has to be called before initializing the instance of HMSHandler
+ // Using the hook on startup ensures that the hook always has priority
+ // over settings in *.xml. The thread local conf needs to be used because at this point
+ // it has already been initialized using hiveConf.
+ MetaStoreInit.updateConnectionURL(origConf, getActiveConf(), null, metaStoreInitData);
+ initBaseHandler();
+ }
+
+ static class Result {
+ static final Result ERROR_RESULT = new Result(null, "error=true");
Review Comment:
that would be creating a new instance, should we extract it?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1223031081
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AbstractHMSHandlerProxy.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.MetaStoreInit.MetaStoreInitData;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.metrics.PerfLogger;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractHMSHandlerProxy implements InvocationHandler {
+ protected final MetaStoreInitData metaStoreInitData = new MetaStoreInitData();
+ protected final IHMSHandler baseHandler;
+ protected final Configuration origConf; // base configuration
+ protected final Configuration activeConf; // active configuration
+ protected final boolean reloadConf;
+ protected final long timeout;
+
+ public AbstractHMSHandlerProxy(Configuration conf, IHMSHandler baseHandler, boolean local)
+ throws MetaException {
+ this.origConf = conf;
+ this.baseHandler = baseHandler;
+ if (local) {
+ baseHandler.setConf(origConf); // tests expect configuration changes applied directly to metastore
+ }
+ activeConf = baseHandler.getConf();
+ reloadConf = MetastoreConf.getBoolVar(origConf, ConfVars.HMS_HANDLER_FORCE_RELOAD_CONF);
+ timeout = MetastoreConf.getTimeVar(origConf,
+ ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
+
+ // This has to be called before initializing the instance of HMSHandler
+ // Using the hook on startup ensures that the hook always has priority
+ // over settings in *.xml. The thread local conf needs to be used because at this point
+ // it has already been initialized using hiveConf.
+ MetaStoreInit.updateConnectionURL(origConf, getActiveConf(), null, metaStoreInitData);
+ initBaseHandler();
+ }
+
+ static class Result {
Review Comment:
`static final`
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] wecharyu commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1588669444
Yeah, retrying strategy is highly effective in HMS. Actually we just want to refactor the HMSHandler proxy code to be more clear and improve code extensibility. For example we can combine different proxy strategies into one like:
```java
class TimerAndRetryingHMSHandler extends AbstractHMSHandlerProxy {
private final IHMSHandler retryingHandler; // replace baseHandler
}
```
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1598263837
> @dengzhhu653, do you have any concerns with the current approach?
I'm +1 on the idea of removing test codes from main routine.
Rather than defining a template method, I'd like to introduce a hook for this purpose, unless we would introduce complex strategies around the `method.invoke(proxy, args)` in the future.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1223026996
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -874,6 +876,9 @@ public enum ConfVars {
"testing only."),
HMS_HANDLER_INTERVAL("metastore.hmshandler.retry.interval", "hive.hmshandler.retry.interval",
2000, TimeUnit.MILLISECONDS, "The time between HMSHandler retry attempts on failure."),
+ HMS_HANDLER_PROXY_CLASS("metastore.hmshandler.proxy", "hive.metastore.hmshandler.proxy",
Review Comment:
do we really need a config for that, since `RetryingHMSHandler` it the only implementation at this moment?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] sonarcloud[bot] commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1583182900
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=4257)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=4257&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL) [23 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=4257&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=4257&metric=duplicated_lines_density&view=list) No Duplication information
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] wecharyu commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1223303312
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AbstractHMSHandlerProxy.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.MetaStoreInit.MetaStoreInitData;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.metrics.PerfLogger;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractHMSHandlerProxy implements InvocationHandler {
+ protected final MetaStoreInitData metaStoreInitData = new MetaStoreInitData();
+ protected final IHMSHandler baseHandler;
+ protected final Configuration origConf; // base configuration
+ protected final Configuration activeConf; // active configuration
+ protected final boolean reloadConf;
+ protected final long timeout;
+
+ public AbstractHMSHandlerProxy(Configuration conf, IHMSHandler baseHandler, boolean local)
+ throws MetaException {
+ this.origConf = conf;
+ this.baseHandler = baseHandler;
+ if (local) {
+ baseHandler.setConf(origConf); // tests expect configuration changes applied directly to metastore
+ }
+ activeConf = baseHandler.getConf();
+ reloadConf = MetastoreConf.getBoolVar(origConf, ConfVars.HMS_HANDLER_FORCE_RELOAD_CONF);
+ timeout = MetastoreConf.getTimeVar(origConf,
+ ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
+
+ // This has to be called before initializing the instance of HMSHandler
+ // Using the hook on startup ensures that the hook always has priority
+ // over settings in *.xml. The thread local conf needs to be used because at this point
+ // it has already been initialized using hiveConf.
+ MetaStoreInit.updateConnectionURL(origConf, getActiveConf(), null, metaStoreInitData);
+ initBaseHandler();
+ }
+
+ static class Result {
Review Comment:
Done.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] wecharyu commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "wecharyu (via GitHub)" <gi...@apache.org>.
wecharyu commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1587241635
@dengzhhu653 Thanks for your review. In this patch we want `AbstractHMSHandlerProxy` become an interface easy to expand other proxy strategies beyond just "retrying", like timing or something else. It may be different from `RetryingHMSHandlerHook`, WDYT?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1588344974
> @dengzhhu653 Thanks for your review. In this patch we want `AbstractHMSHandlerProxy` become an interface easy to expand other proxy strategies beyond just "retrying", like timing or something else. It may be different from `RetryingHMSHandlerHook`, WDYT?
Retrying on transient exception is a good way to reduce the network roundtrip client retries, besides there has been a property `hive.hmshandler.retry.attempts` to control the number of retries, e.g, 0 for disabling the retry mechanism.
Given the template method `AbstractHMSHandlerProxy` provided:
```
protected abstract Result invokeInternal(Object proxy, Method method, Object[] args)
throws Throwable;
```
Whatever the strategies is, method should is called on the `proxy` in the child's `invokeInternal`, that's what `RetryingHMSHandler` does currently.
So in my view, if there is no special need to introduce complex strategies on `method.invoke(proxy, args)`, then let's keep the codes simple.
Thanks,
Zhihua
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on a diff in pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on code in PR #4257:
URL: https://github.com/apache/hive/pull/4257#discussion_r1223044780
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/AbstractHMSHandlerProxy.java:
##########
@@ -0,0 +1,96 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hive.metastore.MetaStoreInit.MetaStoreInitData;
+import org.apache.hadoop.hive.metastore.api.MetaException;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf;
+import org.apache.hadoop.hive.metastore.conf.MetastoreConf.ConfVars;
+import org.apache.hadoop.hive.metastore.metrics.PerfLogger;
+
+import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.Method;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractHMSHandlerProxy implements InvocationHandler {
+ protected final MetaStoreInitData metaStoreInitData = new MetaStoreInitData();
+ protected final IHMSHandler baseHandler;
+ protected final Configuration origConf; // base configuration
+ protected final Configuration activeConf; // active configuration
+ protected final boolean reloadConf;
+ protected final long timeout;
+
+ public AbstractHMSHandlerProxy(Configuration conf, IHMSHandler baseHandler, boolean local)
+ throws MetaException {
+ this.origConf = conf;
+ this.baseHandler = baseHandler;
+ if (local) {
+ baseHandler.setConf(origConf); // tests expect configuration changes applied directly to metastore
+ }
+ activeConf = baseHandler.getConf();
+ reloadConf = MetastoreConf.getBoolVar(origConf, ConfVars.HMS_HANDLER_FORCE_RELOAD_CONF);
+ timeout = MetastoreConf.getTimeVar(origConf,
+ ConfVars.CLIENT_SOCKET_TIMEOUT, TimeUnit.MILLISECONDS);
+
+ // This has to be called before initializing the instance of HMSHandler
+ // Using the hook on startup ensures that the hook always has priority
+ // over settings in *.xml. The thread local conf needs to be used because at this point
+ // it has already been initialized using hiveConf.
+ MetaStoreInit.updateConnectionURL(origConf, getActiveConf(), null, metaStoreInitData);
+ initBaseHandler();
+ }
+
+ static class Result {
+ static final Result ERROR_RESULT = new Result(null, "error=true");
Review Comment:
that would be creating a new instance, should we extract it?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1585646956
@wecharyu, thank you for the contribution! let's wait approx 1 week for the feedback from @nrg4878 and @saihemanth-cloudera.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] deniskuzZ commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1596935672
@dengzhhu653, do you have any concerns with the current approach?
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on pull request #4257: HIVE-27284: Make HMSHandler proxy pluggable
Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #4257:
URL: https://github.com/apache/hive/pull/4257#issuecomment-1602576178
> > > @dengzhhu653, do you have any concerns with the current approach?
> >
> >
> > I'm +1 on the idea of removing test codes from main routine. Rather than defining a template method, I'd like to introduce a hook for this purpose, unless we would introduce complex strategies around the `method.invoke(proxy, args)` in the future.
>
> I think, regardless if we introduce new custom strategies, the current patch makes the code more readable and extensible/flexible.
The code itself looks fine to me, though I'm +0 on the factory approach, if we think the benefit wins the complexity and maintenance, I'm fine with the changes.
--
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: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org