You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/10/16 20:06:57 UTC
[GitHub] [hive] jfsii opened a new pull request, #3674: [WIP] HIVE-26633: Make Thrift MaxMessageSize configurable
jfsii opened a new pull request, #3674:
URL: https://github.com/apache/hive/pull/3674
### What changes were proposed in this pull request?
Make thrift max message size configurable in a variety of contexts. The most important being between HS2 and HMS since message sizes can get quite large.
### Why are the changes needed?
Large metadata messages since the upgrade to thrift 0.16 can hit an internal thrift limit of 100 MB. This change allows this limit to be configured.
### Does this PR introduce _any_ user-facing change?
It adds a new configuration option to HiveConf and defaults to 1gb. Additionally also adds the ability to adjust the beeline connection thrift max message size (mostly as a safety valve, since I have not seen it get hit here).
### How was this patch tested?
Manually on a kerberized cluster.
--
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] jfsii commented on a diff in pull request #3674: [WIP] HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r997242648
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,8 +50,21 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
- public static TTransport getSocketTransport(String host, int port, int loginTimeout) throws TTransportException {
Review Comment:
Will do.
--
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] amansinha100 commented on a diff in pull request #3674: [WIP] HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
amansinha100 commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r996527079
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,8 +50,21 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
- public static TTransport getSocketTransport(String host, int port, int loginTimeout) throws TTransportException {
Review Comment:
Since this is a public static method, it is possible it is used by some client programs and may be needed for backward compatibility. Can we make this a wrapper and supply the default -1 value ?
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2913,7 +2913,10 @@ public static enum ConfVars {
HIVE_STATS_MAX_NUM_STATS("hive.stats.max.num.stats", (long) 10000,
"When the number of stats to be updated is huge, this value is used to control the number of \n" +
" stats to be sent to HMS for update."),
-
+ HIVE_THRIFT_MAX_MESSAGE_SIZE("hive.thrift.max.message.size", "1gb",
Review Comment:
Couple of comments:
- there is also a hive.server2.thrift.max.message.size parameter currently set to 100MB. However, that appears under the http over thrift transport settings. The naming can get confusing. I think either we consolidate the 2 settings into one or the previous config should have the 'http' string appear in the name to avoid conflict. If we keep them separate, it would be useful to understand whether they should be consistent with each other. I don't have the full context of http over thrift.
- For the default value, most other size specific settings have the full bytes value specified e.g 100*1024*1024L for 100MB. There are a few settings which specify the units as you have done but it seems a lot fewer such instances.
--
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] amansinha100 commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
amansinha100 commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r997565113
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2913,7 +2913,10 @@ public static enum ConfVars {
HIVE_STATS_MAX_NUM_STATS("hive.stats.max.num.stats", (long) 10000,
"When the number of stats to be updated is huge, this value is used to control the number of \n" +
" stats to be sent to HMS for update."),
-
+ HIVE_THRIFT_MAX_MESSAGE_SIZE("hive.thrift.max.message.size", "1gb",
Review Comment:
Forgot to respond on point 2. Yes, the SizeValidator() way sounds good. Just a couple of nits: use common casing in both places (for HiveConf and MetastoreConf) i.e either gb or Gb or GB. Also, would be good to mention in the description what is the max message size allowed. i.e 2 GB based on the Integer.MAX_VALUE.
--
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 #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1283786693
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3674)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&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] dengzhhu653 merged pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
dengzhhu653 merged PR #3674:
URL: https://github.com/apache/hive/pull/3674
--
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 #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1283341961
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3674)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&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] dengzhhu653 commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998393428
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -881,11 +881,27 @@ private TTransport createUnderlyingTransport() throws TTransportException {
}
} else {
// get non-SSL socket transport
- transport = HiveAuthUtils.getSocketTransport(host, port, loginTimeout);
+ transport = HiveAuthUtils.getSocketTransport(host, port, loginTimeout, maxMessageSize);
}
return transport;
Review Comment:
nit: can we set the `maxMessageSize` by `transport.getConfiguration().setMaxMessageSize()` for both non-SSL and SSL transport?
--
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] jfsii commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998784726
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,45 +49,108 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
+ /**
+ * Configure the provided T transport's max message size.
+ * @param transport Transport to configure maxMessage for
+ * @param maxMessageSize Maximum allowed message size in bytes, les than or equal to 0 means use the Thrift library
Review Comment:
les should be less thanks @scarlin-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] dengzhhu653 commented on pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1284747802
> The CI failure is spurious. It is the flaky oracle metastore test in which it times out waiting for docker. It passed in the prior CI run with the same bits (10 and 11 are on the same patchset).
>
> I think this is committable, but I can re-trigger if you want me to @dengzhhu653
Hi @jfsii, I've re-triggered, I will merge this once it gets a green run. Thank you!
--
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 #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1281564508
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3674)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&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] jfsii commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998863281
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,45 +49,108 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
+ /**
+ * Configure the provided T transport's max message size.
+ * @param transport Transport to configure maxMessage for
+ * @param maxMessageSize Maximum allowed message size in bytes, les than or equal to 0 means use the Thrift library
Review Comment:
Done
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -850,7 +851,7 @@ private String getJWTStringFromSession() {
* @return TTransport
* @throws TTransportException
*/
- private TTransport createUnderlyingTransport() throws TTransportException {
+ private TTransport createUnderlyingTransport(int maxMessageSize) throws TTransportException {
TTransport transport = null;
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] amansinha100 commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
amansinha100 commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998868796
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,45 +49,108 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
+ /**
+ * Configure the provided T transport's max message size.
+ * @param transport Transport to configure maxMessage for
+ * @param maxMessageSize Maximum allowed message size in bytes, less than or equal to 0 means use the Thrift library
+ * default.
+ * @return The passed in T transport configured with desired max message size. The same object passed in is returned.
+ */
+ public static <T extends TTransport> T configureThriftMaxMessageSize(T transport, int maxMessageSize) {
+ if (maxMessageSize > 0) {
+ if (transport.getConfiguration() == null) {
+ LOG.warn("TTransport {} is returning a null Configuration, Thrift max message size is not getting configured",
+ transport.getClass().getName());
+ return transport;
+ }
+ transport.getConfiguration().setMaxMessageSize(maxMessageSize);
+ }
+ return transport;
+ }
+
+ /**
+ * Create a TSocket for the provided host and port with specified loginTimeout. Thrift maxMessageSize
+ * will default to Thrift library default.
+ * @param host Host to connect to.
+ * @param port Port to connect to.
+ * @param loginTimeout Socket timeout (0 means no timeout).
+ * @return TTransport TSocket for host/port.
+ */
public static TTransport getSocketTransport(String host, int port, int loginTimeout) throws TTransportException {
- return new TSocket(new TConfiguration(),host, port, loginTimeout);
+ return getSocketTransport(host, port, loginTimeout, -1);
+ }
+
+ /**
+ * Create a TSocket for the provided host and port with specified loginTimeout and maxMessageSize.
+ * will default to Thrift library default.
+ * @param host Host to connect to.
+ * @param port Port to connect to.
+ * @param loginTimeout Socket timeout (0 means no timeout).
+ * @param maxMessageSize Size in bytes for max allowable Thrift message size, less than or equal to 0
+ * results in using the Thrift library default.
+ * @return TTransport TSocket for host/port
+ */
+ public static TTransport getSocketTransport(String host, int port, int loginTimeout, int maxMessageSize)
+ throws TTransportException {
+ TSocket tSocket = new TSocket(host, port, loginTimeout);
+ return configureThriftMaxMessageSize(tSocket, maxMessageSize);
+ }
+
+ public static TTransport getSSLSocket(String host, int port, int loginTimeout, TSSLTransportParameters params,
+ int maxMessageSize) throws TTransportException {
+ // The underlying SSLSocket object is bound to host:port with the given SO_TIMEOUT and
+ // SSLContext created with the given params
+ TSocket tSSLSocket = null;
+ if (params != null) {
+ tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout, params);
+ } else {
+ tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout);
+ }
+ configureThriftMaxMessageSize(tSSLSocket, maxMessageSize);
Review Comment:
It is little odd that the return value of this is not being used. Is this a spurious call ? Considering that the next statement getSSLSocketWithHttps() does the same thing.
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,45 +49,108 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
+ /**
+ * Configure the provided T transport's max message size.
+ * @param transport Transport to configure maxMessage for
+ * @param maxMessageSize Maximum allowed message size in bytes, less than or equal to 0 means use the Thrift library
+ * default.
+ * @return The passed in T transport configured with desired max message size. The same object passed in is returned.
+ */
+ public static <T extends TTransport> T configureThriftMaxMessageSize(T transport, int maxMessageSize) {
+ if (maxMessageSize > 0) {
+ if (transport.getConfiguration() == null) {
+ LOG.warn("TTransport {} is returning a null Configuration, Thrift max message size is not getting configured",
+ transport.getClass().getName());
+ return transport;
+ }
+ transport.getConfiguration().setMaxMessageSize(maxMessageSize);
+ }
+ return transport;
+ }
+
+ /**
+ * Create a TSocket for the provided host and port with specified loginTimeout. Thrift maxMessageSize
+ * will default to Thrift library default.
+ * @param host Host to connect to.
+ * @param port Port to connect to.
+ * @param loginTimeout Socket timeout (0 means no timeout).
+ * @return TTransport TSocket for host/port.
+ */
public static TTransport getSocketTransport(String host, int port, int loginTimeout) throws TTransportException {
- return new TSocket(new TConfiguration(),host, port, loginTimeout);
+ return getSocketTransport(host, port, loginTimeout, -1);
+ }
+
+ /**
+ * Create a TSocket for the provided host and port with specified loginTimeout and maxMessageSize.
+ * will default to Thrift library default.
+ * @param host Host to connect to.
+ * @param port Port to connect to.
+ * @param loginTimeout Socket timeout (0 means no timeout).
+ * @param maxMessageSize Size in bytes for max allowable Thrift message size, less than or equal to 0
+ * results in using the Thrift library default.
+ * @return TTransport TSocket for host/port
+ */
+ public static TTransport getSocketTransport(String host, int port, int loginTimeout, int maxMessageSize)
+ throws TTransportException {
+ TSocket tSocket = new TSocket(host, port, loginTimeout);
+ return configureThriftMaxMessageSize(tSocket, maxMessageSize);
+ }
+
+ public static TTransport getSSLSocket(String host, int port, int loginTimeout, TSSLTransportParameters params,
+ int maxMessageSize) throws TTransportException {
+ // The underlying SSLSocket object is bound to host:port with the given SO_TIMEOUT and
+ // SSLContext created with the given params
+ TSocket tSSLSocket = null;
+ if (params != null) {
+ tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout, params);
+ } else {
+ tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout);
+ }
+ configureThriftMaxMessageSize(tSSLSocket, maxMessageSize);
+ return getSSLSocketWithHttps(tSSLSocket, maxMessageSize);
+ }
+
+ public static TTransport getSSLSocket(String host, int port, int loginTimeout, String trustStorePath,
+ String trustStorePassWord, String trustStoreType, String trustStoreAlgorithm) throws TTransportException {
+ return getSSLSocket(host, port, loginTimeout, trustStorePath, trustStorePassWord, trustStoreType,
+ trustStoreAlgorithm, -1);
}
- public static TTransport getSSLSocket(String host, int port, int loginTimeout)
- throws TTransportException {
- // The underlying SSLSocket object is bound to host:port with the given SO_TIMEOUT
- TSocket tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout);
- return getSSLSocketWithHttps(tSSLSocket);
+ public static TTransport getSSLSocket(String host, int port, int loginTimeout) throws TTransportException {
+ return getSSLSocket(host, port, loginTimeout, -1);
}
- public static TTransport getSSLSocket(String host, int port, int loginTimeout,
- String trustStorePath, String trustStorePassWord, String trustStoreType,
- String trustStoreAlgorithm) throws TTransportException {
- TSSLTransportFactory.TSSLTransportParameters params =
- new TSSLTransportFactory.TSSLTransportParameters();
+ public static TTransport getSSLSocket(String host, int port, int loginTimeout, int maxMessageSize)
+ throws TTransportException {
+ return getSSLSocket(host, port, loginTimeout, null, maxMessageSize);
Review Comment:
For the default values being passed (e.g null) or -1 etc. could you add a /* param name */
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,45 +49,108 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
+ /**
+ * Configure the provided T transport's max message size.
+ * @param transport Transport to configure maxMessage for
+ * @param maxMessageSize Maximum allowed message size in bytes, les than or equal to 0 means use the Thrift library
+ * default.
+ * @return The passed in T transport configured with desired max message size. The same object passed in is returned.
+ */
+ public static <T extends TTransport> T configureThriftMaxMessageSize(T transport, int maxMessageSize) {
+ if (maxMessageSize > 0) {
+ if (transport.getConfiguration() == null) {
+ LOG.warn("TTransport {} is returning a null Configuration, Thrift max message size is not getting configured",
Review Comment:
makes sense
--
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] jfsii commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998687240
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -881,11 +881,27 @@ private TTransport createUnderlyingTransport() throws TTransportException {
}
} else {
// get non-SSL socket transport
- transport = HiveAuthUtils.getSocketTransport(host, port, loginTimeout);
+ transport = HiveAuthUtils.getSocketTransport(host, port, loginTimeout, maxMessageSize);
}
return transport;
Review Comment:
I went with implementing this solution within the calls HiveAuthUtils calls so that it is handled uniformly.
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,45 +49,108 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
+ /**
+ * Configure the provided T transport's max message size.
+ * @param transport Transport to configure maxMessage for
+ * @param maxMessageSize Maximum allowed message size in bytes, les than or equal to 0 means use the Thrift library
+ * default.
+ * @return The passed in T transport configured with desired max message size. The same object passed in is returned.
+ */
+ public static <T extends TTransport> T configureThriftMaxMessageSize(T transport, int maxMessageSize) {
+ if (maxMessageSize > 0) {
+ if (transport.getConfiguration() == null) {
+ LOG.warn("TTransport {} is returning a null Configuration, Thrift max message size is not getting configured",
Review Comment:
Decided to be paranoid here. My initial test run caught TFilterTransport returning null for getConfiguration() (so I plumbed it through in this patch). I figured it is better to be safe than sorry, since an exception in this path likely makes the tables not accessible.
--
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] jfsii commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r997295230
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,8 +50,21 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
- public static TTransport getSocketTransport(String host, int port, int loginTimeout) throws TTransportException {
Review Comment:
Done. Added some javadoc too.
--
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 #3674: [WIP] HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1280054774
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3674)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&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] jfsii commented on a diff in pull request #3674: [WIP] HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r997240526
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2913,7 +2913,10 @@ public static enum ConfVars {
HIVE_STATS_MAX_NUM_STATS("hive.stats.max.num.stats", (long) 10000,
"When the number of stats to be updated is huge, this value is used to control the number of \n" +
" stats to be sent to HMS for update."),
-
+ HIVE_THRIFT_MAX_MESSAGE_SIZE("hive.thrift.max.message.size", "1gb",
Review Comment:
For point 1 - I looked into the hive.server2.thrift.max.message.size - it has nothing to do with http. I wouldn't trust the grouping comments in HiveConf. It has been edited by so many developers with different styles it tends to be a bit messy imo. The "hive.server2.thrift.max.message.size" setting ends up getting used here:
https://github.com/apache/hive/blob/d17bd5bf12e62563cc35016dbb4445c89333d9a3/service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java#L102
which then gets passed to this constructor in thrift:
https://github.com/apache/thrift/blob/2a93df80f27739ccabb5b885cb12a8dc7595ecdf/lib/java/src/org/apache/thrift/protocol/TBinaryProtocol.java#L103
So it ends up being used to define these two limits for the binary protocol:
```/**
* The maximum number of bytes to read from the transport for
* variable-length fields (such as strings or binary) or {@link #NO_LENGTH_LIMIT} for
* unlimited.
*/
private final long stringLengthLimit_;
/**
* The maximum number of elements to read from the network for
* containers (maps, sets, lists), or {@link #NO_LENGTH_LIMIT} for unlimited.
*/
private final long containerLengthLimit_;
```
I would argue the name "hive.server2.thrift.max.message.size" is itself misnamed, however we are stuck with it since it has been in the wild for so long. I am not sure it would be a good idea to use the same setting for 3 different thrift limits.
I am open to an alternative name for my setting though "hive.thrift.max.message.size" because it is confusing with the other named setting. If you think my above concerns about re-using the same setting for stringLengthLimit, containerLengthLimit and overall thrift client bytes received limit are overblown, I could be swayed.
For point 2 - I'd argue my way is more consistent. I am using a SizeValidator() which from my quick check most of the settings that use a size validator, the default is a size with a unit. I'd also argue this is more user-friendly. The usages of SizeValidator() that do not specify a unit look to be using SizeValidator() incorrectly. For example - "hive.mv.files.thread" is not a size. I could not use SizeValidator and just accept an integer but I think that is removing a user friend way to set settings for something that is more engineer oriented.
Both of the above points I can be swayed, I'm just sharing my thought process.
--
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] amansinha100 commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
amansinha100 commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998908638
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2913,7 +2913,10 @@ public static enum ConfVars {
HIVE_STATS_MAX_NUM_STATS("hive.stats.max.num.stats", (long) 10000,
"When the number of stats to be updated is huge, this value is used to control the number of \n" +
" stats to be sent to HMS for update."),
-
+ HIVE_THRIFT_CLIENT_MAX_MESSAGE_SIZE("hive.thrift.client.max.message.size", "1gb",
+ new SizeValidator(-1L, true, (long) Integer.MAX_VALUE, true),
+ "Thrift client configuration for max message size. 0 or -1 will use the default defined in the Thrift " +
+ "library. The upper limit is 21474836480 bytes (or 2gb)."),
Review Comment:
`21474836480` I missed this earlier. Remove the extra '0' at the end.
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/conf/MetastoreConf.java:
##########
@@ -1405,6 +1405,10 @@ public enum ConfVars {
"If dynamic service discovery mode is set, the URIs are used to connect to the" +
" corresponding service discovery servers e.g. a zookeeper. Otherwise they are " +
"used as URIs for remote metastore."),
+ THRIFT_METASTORE_CLIENT_MAX_MESSAGE_SIZE("metastore.thrift.client.max.message.size",
+ "hive.thrift.client.max.message.size", "1gb", new SizeValidator(-1L, true, (long) Integer.MAX_VALUE, true),
+ "Thrift client configuration for max message size. 0 or -1 will use the default defined in the Thrift " +
+ "library. The upper limit is 21474836480 bytes (or 2gb)."),
Review Comment:
`21474836480` I missed this earlier. Remove the extra '0' at the end.
--
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 #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1284786939
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3674)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&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] dengzhhu653 commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998853885
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -850,7 +851,7 @@ private String getJWTStringFromSession() {
* @return TTransport
* @throws TTransportException
*/
- private TTransport createUnderlyingTransport() throws TTransportException {
+ private TTransport createUnderlyingTransport(int maxMessageSize) throws TTransportException {
TTransport transport = null;
Review Comment:
nit: we can get this `maxMessageSize` through `getMaxMessageSize()` in the method instead by passing an argument.
--
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] jfsii commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998898238
##########
common/src/java/org/apache/hadoop/hive/common/auth/HiveAuthUtils.java:
##########
@@ -50,45 +49,108 @@
public class HiveAuthUtils {
private static final Logger LOG = LoggerFactory.getLogger(HiveAuthUtils.class);
+ /**
+ * Configure the provided T transport's max message size.
+ * @param transport Transport to configure maxMessage for
+ * @param maxMessageSize Maximum allowed message size in bytes, less than or equal to 0 means use the Thrift library
+ * default.
+ * @return The passed in T transport configured with desired max message size. The same object passed in is returned.
+ */
+ public static <T extends TTransport> T configureThriftMaxMessageSize(T transport, int maxMessageSize) {
+ if (maxMessageSize > 0) {
+ if (transport.getConfiguration() == null) {
+ LOG.warn("TTransport {} is returning a null Configuration, Thrift max message size is not getting configured",
+ transport.getClass().getName());
+ return transport;
+ }
+ transport.getConfiguration().setMaxMessageSize(maxMessageSize);
+ }
+ return transport;
+ }
+
+ /**
+ * Create a TSocket for the provided host and port with specified loginTimeout. Thrift maxMessageSize
+ * will default to Thrift library default.
+ * @param host Host to connect to.
+ * @param port Port to connect to.
+ * @param loginTimeout Socket timeout (0 means no timeout).
+ * @return TTransport TSocket for host/port.
+ */
public static TTransport getSocketTransport(String host, int port, int loginTimeout) throws TTransportException {
- return new TSocket(new TConfiguration(),host, port, loginTimeout);
+ return getSocketTransport(host, port, loginTimeout, -1);
+ }
+
+ /**
+ * Create a TSocket for the provided host and port with specified loginTimeout and maxMessageSize.
+ * will default to Thrift library default.
+ * @param host Host to connect to.
+ * @param port Port to connect to.
+ * @param loginTimeout Socket timeout (0 means no timeout).
+ * @param maxMessageSize Size in bytes for max allowable Thrift message size, less than or equal to 0
+ * results in using the Thrift library default.
+ * @return TTransport TSocket for host/port
+ */
+ public static TTransport getSocketTransport(String host, int port, int loginTimeout, int maxMessageSize)
+ throws TTransportException {
+ TSocket tSocket = new TSocket(host, port, loginTimeout);
+ return configureThriftMaxMessageSize(tSocket, maxMessageSize);
+ }
+
+ public static TTransport getSSLSocket(String host, int port, int loginTimeout, TSSLTransportParameters params,
+ int maxMessageSize) throws TTransportException {
+ // The underlying SSLSocket object is bound to host:port with the given SO_TIMEOUT and
+ // SSLContext created with the given params
+ TSocket tSSLSocket = null;
+ if (params != null) {
+ tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout, params);
+ } else {
+ tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout);
+ }
+ configureThriftMaxMessageSize(tSSLSocket, maxMessageSize);
Review Comment:
I was being overly paranoid. Looking closer at getSSLSocketWithHttps, I see it doesn't even end up using the TSocket. It grabs the SSLSocket and re-wraps it with TSocket. When I originally read the code I thought it was a TSocket wrapped by a TSocket which I didn't know which conf would take (but I was wrong).
--
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] jfsii commented on pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1284302958
The CI failure is spurious. It is the flaky oracle metastore test in which it times out waiting for docker. It passed in the prior CI run with the same bits (10 and 11 are on the same patchset).
I think this is committable, but I can re-trigger if you want me to @dengzhhu653
--
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] amansinha100 commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
amansinha100 commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r997487069
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2913,7 +2913,10 @@ public static enum ConfVars {
HIVE_STATS_MAX_NUM_STATS("hive.stats.max.num.stats", (long) 10000,
"When the number of stats to be updated is huge, this value is used to control the number of \n" +
" stats to be sent to HMS for update."),
-
+ HIVE_THRIFT_MAX_MESSAGE_SIZE("hive.thrift.max.message.size", "1gb",
Review Comment:
I see.. I agree that hive.server2.thrift.max.message.size is getting used for a different purpose (for the stringLengthLimit and containerLengthLimit) and it would not be appropriate to overload that for our use (I didn't realize earlier that this setting has been around since 8 years or so).
Regarding the naming for the new setting, currently I don't have a better suggestion. I can foresee fair amount of confusion about these 2 config options. Let me give it some more thought.
--
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 #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1281749029
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3674)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&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] jfsii commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r997598574
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2913,7 +2913,10 @@ public static enum ConfVars {
HIVE_STATS_MAX_NUM_STATS("hive.stats.max.num.stats", (long) 10000,
"When the number of stats to be updated is huge, this value is used to control the number of \n" +
" stats to be sent to HMS for update."),
-
+ HIVE_THRIFT_MAX_MESSAGE_SIZE("hive.thrift.max.message.size", "1gb",
Review Comment:
Just uploaded to fix the nits for point 2.
--
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 #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1284033380
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3674)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&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] sonarcloud[bot] commented on pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3674:
URL: https://github.com/apache/hive/pull/3674#issuecomment-1282553993
Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3674)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=BUG)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=VULNERABILITY)
[](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3674&resolved=false&types=SECURITY_HOTSPOT)
[](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3674&resolved=false&types=CODE_SMELL)
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&metric=coverage&view=list) No Coverage information
[](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3674&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] jfsii commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998333515
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2913,7 +2913,10 @@ public static enum ConfVars {
HIVE_STATS_MAX_NUM_STATS("hive.stats.max.num.stats", (long) 10000,
"When the number of stats to be updated is huge, this value is used to control the number of \n" +
" stats to be sent to HMS for update."),
-
+ HIVE_THRIFT_MAX_MESSAGE_SIZE("hive.thrift.max.message.size", "1gb",
Review Comment:
I think both points are now covered. Went with hive.thrift.client.max.message.size
--
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] jfsii commented on a diff in pull request #3674: HIVE-26633: Make Thrift MaxMessageSize configurable
Posted by GitBox <gi...@apache.org>.
jfsii commented on code in PR #3674:
URL: https://github.com/apache/hive/pull/3674#discussion_r998397658
##########
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java:
##########
@@ -881,11 +881,27 @@ private TTransport createUnderlyingTransport() throws TTransportException {
}
} else {
// get non-SSL socket transport
- transport = HiveAuthUtils.getSocketTransport(host, port, loginTimeout);
+ transport = HiveAuthUtils.getSocketTransport(host, port, loginTimeout, maxMessageSize);
}
return transport;
Review Comment:
That is a great idea. I am not sure how I missed that approach - likely in a rush to fix the issue.
--
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