[GitHub] [hive] pvary commented on pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on pull request #1221: URL: https://github.com/apache/hive/pull/1221#issuecomment-655300805 One more thing: Do I remember correctly, that all the logs are on debug level? I think it would be good to have 1 or 2 log lines in info level, just t confirm that everything is good. Like "Filtered out 2 tables from 1000", or whatever... 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on pull request #1221: URL: https://github.com/apache/hive/pull/1221#issuecomment-655295871 I have done a first sweeping review. A few asks: * Beware of HMS API changes * Check for memory consumption * Check for performance * Use checkstyle format check As I am not too familiar with Ranger, please find a reviewer on that regard as well please. Thanks for the patch. We really need these checks on HMS side as well. Peter 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451290794
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizer.java
##
@@ -85,38 +97,268 @@ public final void onEvent(PreEventContext preEventContext)
throws MetaException,
LOG.debug("==> HiveMetaStoreAuthorizer.onEvent(): EventType=" +
preEventContext.getEventType());
}
-HiveMetaStoreAuthzInfo authzContext = buildAuthzContext(preEventContext);
+try {
+HiveAuthorizer hiveAuthorizer = createHiveMetaStoreAuthorizer();
+if (!skipAuthorization()) {
+ HiveMetaStoreAuthzInfo authzContext =
buildAuthzContext(preEventContext);
+ checkPrivileges(authzContext, hiveAuthorizer);
+}
+} catch (Exception e) {
+ LOG.error("HiveMetaStoreAuthorizer.onEvent(): failed", e);
+ throw new MetaException(e.getMessage());
+}
-if (!skipAuthorization(authzContext)) {
- try {
-HiveConf hiveConf = new
HiveConf(super.getConf(), HiveConf.class);
-HiveAuthorizerFactory authorizerFactory =
HiveUtils.getAuthorizerFactory(hiveConf,
HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER);
+if (LOG.isDebugEnabled()) {
+ LOG.debug("<== HiveMetaStoreAuthorizer.onEvent(): EventType=" +
preEventContext.getEventType());
+}
+ }
-if (authorizerFactory != null) {
- HiveMetastoreAuthenticationProvider authenticator =
tAuthenticator.get();
+ @Override
+ public final List filterDatabases(List list) throws
MetaException {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("HiveMetaStoreAuthorizer.filterDatabases()");
+}
- authenticator.setConf(hiveConf);
+if (list == null) {
+ return Collections.emptyList();
+}
- HiveAuthzSessionContext.Builder authzContextBuilder = new
HiveAuthzSessionContext.Builder();
+DatabaseFilterContext databaseFilterContext= new
DatabaseFilterContext(list);
+HiveMetaStoreAuthzInfo hiveMetaStoreAuthzInfo =
databaseFilterContext.getAuthzContext();
+ListfilteredDatabases=
filterDatabaseObjects(hiveMetaStoreAuthzInfo);
+if (CollectionUtils.isEmpty(filteredDatabases)) {
+ filteredDatabases = Collections.emptyList();
+}
-
authzContextBuilder.setClientType(HiveAuthzSessionContext.CLIENT_TYPE.HIVEMETASTORE);
- authzContextBuilder.setSessionString("HiveMetaStore");
+if (LOG.isDebugEnabled()) {
+ LOG.debug("HiveMetaStoreAuthorizer.filterDatabases() :" +
filteredDatabases);
+}
+return filteredDatabases ;
+ }
- HiveAuthzSessionContext authzSessionContext =
authzContextBuilder.build();
+ @Override
+ public final Database filterDatabase(Database database) throws
MetaException, NoSuchObjectException {
+if (database != null) {
+ String dbName = database.getName();
+ List databases =
filterDatabases(Collections.singletonList(dbName));
+ if (databases.isEmpty()) {
+throw new NoSuchObjectException(String.format("Database %s does not
exist", dbName));
+ }
+}
+return database;
+ }
+
+ @Override
+ public final List filterTableNames(String s, String s1, List
list) throws MetaException {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("==> HiveMetaStoreAuthorizer.filterTableNames()");
+}
+List filteredTableNames = null;
+if (list != null) {
+ String dbName = getDBName(s1);
+ TableFilterContext tableFilterContext = new
TableFilterContext(dbName, list);
+ HiveMetaStoreAuthzInfo hiveMetaStoreAuthzInfo =
tableFilterContext.getAuthzContext();
+ filteredTableNames = filterTableNames(hiveMetaStoreAuthzInfo, dbName,
list);
+ if (CollectionUtils.isEmpty(filteredTableNames)) {
+filteredTableNames = Collections.emptyList();
+ }
+}
- HiveAuthorizer hiveAuthorizer =
authorizerFactory.createHiveAuthorizer(new HiveMetastoreClientFactoryImpl(),
hiveConf, authenticator, authzSessionContext);
+if (LOG.isDebugEnabled()) {
+ LOG.debug("<== HiveMetaStoreAuthorizer.filterTableNames() : " +
filteredTableNames);
+}
- checkPrivileges(authzContext, hiveAuthorizer);
-}
- } catch (Exception e) {
-LOG.error("HiveMetaStoreAuthorizer.onEvent(): failed", e);
-throw new MetaException(e.getMessage());
+return filteredTableNames;
+ }
+
+ @Override
+ public final Table filterTable(Table table) throws MetaException,
NoSuchObjectException {
+if (table != null) {
+ List tables = filterTables(Collections.singletonList(table));
+ if (tables.isEmpty()) {
+throw new NoSuchObjectException(String.format("Database %s does not
exist", table.getTableName()));
}
}
+return table;
+ }
+ @Override
+ public final List filterTables(List list) throws MetaException
{
if
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451289748
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizer.java
##
@@ -312,5 +578,13 @@ private String getCurrentUser() {
private String getCurrentUser(HiveMetaStoreAuthorizableEvent
authorizableEvent) {
return authorizableEvent.getAuthzContext().getUGI().getShortUserName();
}
+
+ private UserGroupInformation getUGI() {
+try {
+ return UserGroupInformation.getCurrentUser();
+} catch (IOException excp) {
Review comment:
Why do we swallow the exception?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451288969
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizer.java
##
@@ -198,6 +446,29 @@ HiveMetaStoreAuthzInfo buildAuthzContext(PreEventContext
preEventContext) throws
return ret;
}
+ HiveAuthorizer createHiveMetaStoreAuthorizer() throws Exception {
Review comment:
How costly is this method? Is the Authorizer user specific, or can we
have a single instance per HMS thread, or even better on HMS instance (if this
stuff is thread-safe?)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451287139
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizer.java
##
@@ -85,38 +97,268 @@ public final void onEvent(PreEventContext preEventContext)
throws MetaException,
LOG.debug("==> HiveMetaStoreAuthorizer.onEvent(): EventType=" +
preEventContext.getEventType());
}
-HiveMetaStoreAuthzInfo authzContext = buildAuthzContext(preEventContext);
+try {
+HiveAuthorizer hiveAuthorizer = createHiveMetaStoreAuthorizer();
+if (!skipAuthorization()) {
+ HiveMetaStoreAuthzInfo authzContext =
buildAuthzContext(preEventContext);
+ checkPrivileges(authzContext, hiveAuthorizer);
+}
+} catch (Exception e) {
+ LOG.error("HiveMetaStoreAuthorizer.onEvent(): failed", e);
+ throw new MetaException(e.getMessage());
+}
-if (!skipAuthorization(authzContext)) {
- try {
-HiveConf hiveConf = new
HiveConf(super.getConf(), HiveConf.class);
-HiveAuthorizerFactory authorizerFactory =
HiveUtils.getAuthorizerFactory(hiveConf,
HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER);
+if (LOG.isDebugEnabled()) {
+ LOG.debug("<== HiveMetaStoreAuthorizer.onEvent(): EventType=" +
preEventContext.getEventType());
+}
+ }
-if (authorizerFactory != null) {
- HiveMetastoreAuthenticationProvider authenticator =
tAuthenticator.get();
+ @Override
+ public final List filterDatabases(List list) throws
MetaException {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("HiveMetaStoreAuthorizer.filterDatabases()");
+}
- authenticator.setConf(hiveConf);
+if (list == null) {
+ return Collections.emptyList();
+}
- HiveAuthzSessionContext.Builder authzContextBuilder = new
HiveAuthzSessionContext.Builder();
+DatabaseFilterContext databaseFilterContext= new
DatabaseFilterContext(list);
+HiveMetaStoreAuthzInfo hiveMetaStoreAuthzInfo =
databaseFilterContext.getAuthzContext();
+ListfilteredDatabases=
filterDatabaseObjects(hiveMetaStoreAuthzInfo);
+if (CollectionUtils.isEmpty(filteredDatabases)) {
+ filteredDatabases = Collections.emptyList();
+}
-
authzContextBuilder.setClientType(HiveAuthzSessionContext.CLIENT_TYPE.HIVEMETASTORE);
- authzContextBuilder.setSessionString("HiveMetaStore");
+if (LOG.isDebugEnabled()) {
+ LOG.debug("HiveMetaStoreAuthorizer.filterDatabases() :" +
filteredDatabases);
+}
+return filteredDatabases ;
+ }
- HiveAuthzSessionContext authzSessionContext =
authzContextBuilder.build();
+ @Override
+ public final Database filterDatabase(Database database) throws
MetaException, NoSuchObjectException {
+if (database != null) {
+ String dbName = database.getName();
+ List databases =
filterDatabases(Collections.singletonList(dbName));
+ if (databases.isEmpty()) {
+throw new NoSuchObjectException(String.format("Database %s does not
exist", dbName));
+ }
+}
+return database;
+ }
+
+ @Override
+ public final List filterTableNames(String s, String s1, List
list) throws MetaException {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("==> HiveMetaStoreAuthorizer.filterTableNames()");
+}
+List filteredTableNames = null;
+if (list != null) {
+ String dbName = getDBName(s1);
+ TableFilterContext tableFilterContext = new
TableFilterContext(dbName, list);
+ HiveMetaStoreAuthzInfo hiveMetaStoreAuthzInfo =
tableFilterContext.getAuthzContext();
+ filteredTableNames = filterTableNames(hiveMetaStoreAuthzInfo, dbName,
list);
+ if (CollectionUtils.isEmpty(filteredTableNames)) {
+filteredTableNames = Collections.emptyList();
+ }
+}
- HiveAuthorizer hiveAuthorizer =
authorizerFactory.createHiveAuthorizer(new HiveMetastoreClientFactoryImpl(),
hiveConf, authenticator, authzSessionContext);
+if (LOG.isDebugEnabled()) {
+ LOG.debug("<== HiveMetaStoreAuthorizer.filterTableNames() : " +
filteredTableNames);
+}
- checkPrivileges(authzContext, hiveAuthorizer);
-}
- } catch (Exception e) {
-LOG.error("HiveMetaStoreAuthorizer.onEvent(): failed", e);
-throw new MetaException(e.getMessage());
+return filteredTableNames;
+ }
+
+ @Override
+ public final Table filterTable(Table table) throws MetaException,
NoSuchObjectException {
+if (table != null) {
+ List tables = filterTables(Collections.singletonList(table));
+ if (tables.isEmpty()) {
+throw new NoSuchObjectException(String.format("Database %s does not
exist", table.getTableName()));
}
}
+return table;
+ }
+ @Override
+ public final List filterTables(List list) throws MetaException
{
if
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451286260
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizer.java
##
@@ -85,38 +97,268 @@ public final void onEvent(PreEventContext preEventContext)
throws MetaException,
LOG.debug("==> HiveMetaStoreAuthorizer.onEvent(): EventType=" +
preEventContext.getEventType());
}
-HiveMetaStoreAuthzInfo authzContext = buildAuthzContext(preEventContext);
+try {
+HiveAuthorizer hiveAuthorizer = createHiveMetaStoreAuthorizer();
+if (!skipAuthorization()) {
+ HiveMetaStoreAuthzInfo authzContext =
buildAuthzContext(preEventContext);
+ checkPrivileges(authzContext, hiveAuthorizer);
+}
+} catch (Exception e) {
+ LOG.error("HiveMetaStoreAuthorizer.onEvent(): failed", e);
+ throw new MetaException(e.getMessage());
+}
-if (!skipAuthorization(authzContext)) {
- try {
-HiveConf hiveConf = new
HiveConf(super.getConf(), HiveConf.class);
-HiveAuthorizerFactory authorizerFactory =
HiveUtils.getAuthorizerFactory(hiveConf,
HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER);
+if (LOG.isDebugEnabled()) {
+ LOG.debug("<== HiveMetaStoreAuthorizer.onEvent(): EventType=" +
preEventContext.getEventType());
+}
+ }
-if (authorizerFactory != null) {
- HiveMetastoreAuthenticationProvider authenticator =
tAuthenticator.get();
+ @Override
+ public final List filterDatabases(List list) throws
MetaException {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("HiveMetaStoreAuthorizer.filterDatabases()");
+}
- authenticator.setConf(hiveConf);
+if (list == null) {
+ return Collections.emptyList();
+}
- HiveAuthzSessionContext.Builder authzContextBuilder = new
HiveAuthzSessionContext.Builder();
+DatabaseFilterContext databaseFilterContext= new
DatabaseFilterContext(list);
+HiveMetaStoreAuthzInfo hiveMetaStoreAuthzInfo =
databaseFilterContext.getAuthzContext();
+ListfilteredDatabases=
filterDatabaseObjects(hiveMetaStoreAuthzInfo);
+if (CollectionUtils.isEmpty(filteredDatabases)) {
+ filteredDatabases = Collections.emptyList();
+}
-
authzContextBuilder.setClientType(HiveAuthzSessionContext.CLIENT_TYPE.HIVEMETASTORE);
- authzContextBuilder.setSessionString("HiveMetaStore");
+if (LOG.isDebugEnabled()) {
+ LOG.debug("HiveMetaStoreAuthorizer.filterDatabases() :" +
filteredDatabases);
+}
+return filteredDatabases ;
+ }
- HiveAuthzSessionContext authzSessionContext =
authzContextBuilder.build();
+ @Override
+ public final Database filterDatabase(Database database) throws
MetaException, NoSuchObjectException {
+if (database != null) {
+ String dbName = database.getName();
+ List databases =
filterDatabases(Collections.singletonList(dbName));
+ if (databases.isEmpty()) {
+throw new NoSuchObjectException(String.format("Database %s does not
exist", dbName));
+ }
+}
+return database;
+ }
+
+ @Override
+ public final List filterTableNames(String s, String s1, List
list) throws MetaException {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("==> HiveMetaStoreAuthorizer.filterTableNames()");
+}
+List filteredTableNames = null;
+if (list != null) {
+ String dbName = getDBName(s1);
+ TableFilterContext tableFilterContext = new
TableFilterContext(dbName, list);
+ HiveMetaStoreAuthzInfo hiveMetaStoreAuthzInfo =
tableFilterContext.getAuthzContext();
+ filteredTableNames = filterTableNames(hiveMetaStoreAuthzInfo, dbName,
list);
+ if (CollectionUtils.isEmpty(filteredTableNames)) {
+filteredTableNames = Collections.emptyList();
+ }
+}
- HiveAuthorizer hiveAuthorizer =
authorizerFactory.createHiveAuthorizer(new HiveMetastoreClientFactoryImpl(),
hiveConf, authenticator, authzSessionContext);
+if (LOG.isDebugEnabled()) {
+ LOG.debug("<== HiveMetaStoreAuthorizer.filterTableNames() : " +
filteredTableNames);
+}
- checkPrivileges(authzContext, hiveAuthorizer);
-}
- } catch (Exception e) {
-LOG.error("HiveMetaStoreAuthorizer.onEvent(): failed", e);
-throw new MetaException(e.getMessage());
+return filteredTableNames;
+ }
+
+ @Override
+ public final Table filterTable(Table table) throws MetaException,
NoSuchObjectException {
+if (table != null) {
+ List tables = filterTables(Collections.singletonList(table));
+ if (tables.isEmpty()) {
+throw new NoSuchObjectException(String.format("Database %s does not
exist", table.getTableName()));
}
}
+return table;
+ }
+ @Override
+ public final List filterTables(List list) throws MetaException
{
if
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451285302
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/HiveMetaStoreAuthorizer.java
##
@@ -85,38 +97,268 @@ public final void onEvent(PreEventContext preEventContext)
throws MetaException,
LOG.debug("==> HiveMetaStoreAuthorizer.onEvent(): EventType=" +
preEventContext.getEventType());
}
-HiveMetaStoreAuthzInfo authzContext = buildAuthzContext(preEventContext);
+try {
+HiveAuthorizer hiveAuthorizer = createHiveMetaStoreAuthorizer();
+if (!skipAuthorization()) {
+ HiveMetaStoreAuthzInfo authzContext =
buildAuthzContext(preEventContext);
+ checkPrivileges(authzContext, hiveAuthorizer);
+}
+} catch (Exception e) {
+ LOG.error("HiveMetaStoreAuthorizer.onEvent(): failed", e);
+ throw new MetaException(e.getMessage());
+}
-if (!skipAuthorization(authzContext)) {
- try {
-HiveConf hiveConf = new
HiveConf(super.getConf(), HiveConf.class);
-HiveAuthorizerFactory authorizerFactory =
HiveUtils.getAuthorizerFactory(hiveConf,
HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER);
+if (LOG.isDebugEnabled()) {
+ LOG.debug("<== HiveMetaStoreAuthorizer.onEvent(): EventType=" +
preEventContext.getEventType());
+}
+ }
-if (authorizerFactory != null) {
- HiveMetastoreAuthenticationProvider authenticator =
tAuthenticator.get();
+ @Override
+ public final List filterDatabases(List list) throws
MetaException {
+if (LOG.isDebugEnabled()) {
+ LOG.debug("HiveMetaStoreAuthorizer.filterDatabases()");
+}
- authenticator.setConf(hiveConf);
+if (list == null) {
+ return Collections.emptyList();
+}
- HiveAuthzSessionContext.Builder authzContextBuilder = new
HiveAuthzSessionContext.Builder();
+DatabaseFilterContext databaseFilterContext= new
DatabaseFilterContext(list);
+HiveMetaStoreAuthzInfo hiveMetaStoreAuthzInfo =
databaseFilterContext.getAuthzContext();
+ListfilteredDatabases=
filterDatabaseObjects(hiveMetaStoreAuthzInfo);
+if (CollectionUtils.isEmpty(filteredDatabases)) {
+ filteredDatabases = Collections.emptyList();
+}
-
authzContextBuilder.setClientType(HiveAuthzSessionContext.CLIENT_TYPE.HIVEMETASTORE);
- authzContextBuilder.setSessionString("HiveMetaStore");
+if (LOG.isDebugEnabled()) {
+ LOG.debug("HiveMetaStoreAuthorizer.filterDatabases() :" +
filteredDatabases);
+}
+return filteredDatabases ;
+ }
- HiveAuthzSessionContext authzSessionContext =
authzContextBuilder.build();
+ @Override
+ public final Database filterDatabase(Database database) throws
MetaException, NoSuchObjectException {
+if (database != null) {
+ String dbName = database.getName();
+ List databases =
filterDatabases(Collections.singletonList(dbName));
+ if (databases.isEmpty()) {
+throw new NoSuchObjectException(String.format("Database %s does not
exist", dbName));
+ }
+}
+return database;
+ }
+
+ @Override
+ public final List filterTableNames(String s, String s1, List
list) throws MetaException {
+if (LOG.isDebugEnabled()) {
Review comment:
This LOG.isDebugEnabled is unnecessary. I thrown on them even on cases
where we do string concatenation inside the message, but when we have a static
string this is entirely unnecessary, and bloats the code
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451284085
##
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java
##
@@ -427,7 +428,7 @@ public void testGetPartitionsByNamesNullTblName() throws
Exception {
createTable3PartCols1Part(client);
client.getPartitionsByNames(DB_NAME, null,
Lists.newArrayList("=2000/mm=01/dd=02"));
fail("Should have thrown exception");
-} catch (NullPointerException | TTransportException e) {
+} catch (NullPointerException | TTransportException | TProtocolException |
MetaException e ) {
Review comment:
This is again an HMS API change which should be avoided
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451284051
##
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/client/TestGetPartitions.java
##
@@ -416,7 +417,7 @@ public void testGetPartitionsByNamesNullDbName() throws
Exception {
createTable3PartCols1Part(client);
client.getPartitionsByNames(null, TABLE_NAME,
Lists.newArrayList("=2000/mm=01/dd=02"));
fail("Should have thrown exception");
-} catch (NullPointerException | TTransportException e) {
+} catch (NullPointerException | TTransportException | MetaException e) {
Review comment:
This is again an HMS API change which should be avoided
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451283728
##
File path:
ql/src/test/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/TestHiveMetaStoreAuthorizer.java
##
@@ -283,4 +284,50 @@ public void testM_DropCatalog_SuperUser() throws Exception
{
// no Exceptions for superuser as hive is allowed CREATE CATALOG
operation
}
}
+
+ @Test
+ public void testN__ShowDatabase_authorizedUser() throws Exception {
Review comment:
Please use CamelCase method names. We inherited a few places with
different formatting, but new methods should stick to that
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451283245
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/metastore/filtercontext/DatabaseFilterContext.java
##
@@ -0,0 +1,78 @@
+/*
+ * 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.ql.security.authorization.plugin.metastore.filtercontext;
+
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import
org.apache.hadoop.hive.ql.security.authorization.plugin.HiveOperationType;
+import
org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject;
+import
org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject.HivePrivilegeObjectType;
+import
org.apache.hadoop.hive.ql.security.authorization.plugin.HivePrivilegeObject.HivePrivObjectActionType;
+import
org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.HiveMetaStoreAuthorizableEvent;
+import
org.apache.hadoop.hive.ql.security.authorization.plugin.metastore.HiveMetaStoreAuthzInfo;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class DatabaseFilterContext extends HiveMetaStoreAuthorizableEvent {
+
+private static final Log LOG =
LogFactory.getLog(DatabaseFilterContext.class);
+
+List databases = null;
+
+public DatabaseFilterContext(List databases) {
+super(null);
+this.databases = databases;
+getAuthzContext();
+}
+
+@Override
+public HiveMetaStoreAuthzInfo getAuthzContext() {
+HiveMetaStoreAuthzInfo ret = new
HiveMetaStoreAuthzInfo(preEventContext, HiveOperationType.QUERY,
getInputHObjs(), getOutputHObjs(), null);
+return ret;
+}
+
+private List getInputHObjs() {
+if (LOG.isDebugEnabled()) {
+LOG.debug("==> DatabaseFilterContext.getOutputHObjs()");
+}
+
+List ret = new ArrayList<>();
+for(String database: databases) {
+HivePrivilegeObjectType type=
HivePrivilegeObjectType.DATABASE;
Review comment:
In Hive code we usually avoid this type of formatting.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451282859
##
File path:
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestSessionHiveMetastoreClientGetPartitionsTempTable.java
##
@@ -123,13 +123,13 @@ public void testGetPartitionsByNamesEmptyParts() throws
Exception {
getClient().getPartitionsByNames(DB_NAME, TABLE_NAME,
Lists.newArrayList("", ""));
}
- @Test(expected = MetaException.class)
+ @Test
@Override
public void testGetPartitionsByNamesNullDbName() throws Exception {
super.testGetPartitionsByNamesNullDbName();
}
- @Test(expected = MetaException.class)
+ @Test
Review comment:
This is test was introduced to check backward compatibility of the HMS
API.
Changing this behavior indicates change in the API. I think we should keep
the original Exception.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1221: HIVE-23786: HMS server side filter with Ranger
pvary commented on a change in pull request #1221:
URL: https://github.com/apache/hive/pull/1221#discussion_r451282795
##
File path:
ql/src/test/org/apache/hadoop/hive/ql/metadata/TestSessionHiveMetastoreClientGetPartitionsTempTable.java
##
@@ -123,13 +123,13 @@ public void testGetPartitionsByNamesEmptyParts() throws
Exception {
getClient().getPartitionsByNames(DB_NAME, TABLE_NAME,
Lists.newArrayList("", ""));
}
- @Test(expected = MetaException.class)
+ @Test
Review comment:
This is test was introduced to check backward compatibility of the HMS
API.
Changing this behavior indicates change in the API. I think we should keep
the original Exception.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] jcamachor merged pull request #1119: HIVE-23699: Cleanup HIVEQUERYRESULTFILEFORMAT handling
jcamachor merged pull request #1119: URL: https://github.com/apache/hive/pull/1119 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] StefanXiepj commented on a change in pull request #1209: HIVE-22412: StatsUtils throw NPE when explain
StefanXiepj commented on a change in pull request #1209:
URL: https://github.com/apache/hive/pull/1209#discussion_r451260035
##
File path: ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java
##
@@ -1336,6 +1341,9 @@ public static long
getSizeOfPrimitiveTypeArraysFromType(String colType, int leng
*/
public static long getSizeOfMap(StandardConstantMapObjectInspector scmoi) {
Map map = scmoi.getWritableConstantValue();
+if (null == map || map.isEmpty()) {
+ return 0L;
+}
Review comment:
We can ask the original author's opinion, maybe he has a better idea.
Hi @prasanthj , Could you help me review 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] belugabehr closed pull request #1203: HIVE-22674: Replace Base64 in serde Package
belugabehr closed pull request #1203: URL: https://github.com/apache/hive/pull/1203 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] belugabehr commented on a change in pull request #1209: HIVE-22412: StatsUtils throw NPE when explain
belugabehr commented on a change in pull request #1209:
URL: https://github.com/apache/hive/pull/1209#discussion_r451242285
##
File path: ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java
##
@@ -1336,6 +1341,9 @@ public static long
getSizeOfPrimitiveTypeArraysFromType(String colType, int leng
*/
public static long getSizeOfMap(StandardConstantMapObjectInspector scmoi) {
Map map = scmoi.getWritableConstantValue();
+if (null == map || map.isEmpty()) {
+ return 0L;
+}
Review comment:
I'm actually not sure what the correct behavior is here for a 'null'
Map. I'd have to dig in more to see if there is already precedence for that.
I just know that the current proposal is not correct. Maybe someone else can
weigh in on the correct handling of a null 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] StefanXiepj commented on a change in pull request #1209: HIVE-22412: StatsUtils throw NPE when explain
StefanXiepj commented on a change in pull request #1209:
URL: https://github.com/apache/hive/pull/1209#discussion_r451232462
##
File path: ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java
##
@@ -1336,6 +1341,9 @@ public static long
getSizeOfPrimitiveTypeArraysFromType(String colType, int leng
*/
public static long getSizeOfMap(StandardConstantMapObjectInspector scmoi) {
Map map = scmoi.getWritableConstantValue();
+if (null == map || map.isEmpty()) {
+ return 0L;
+}
Review comment:
Thanks for your review, maybe we can return it like this:
```
Map map = scmoi.getWritableConstantValue();
if (null == map || map.isEmpty()) {
return JavaDataModel.get().hashMap(0);
}
```
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] sam-an-cloudera opened a new pull request #1221: HIVE-23786: HMS server side filter with Ranger
sam-an-cloudera opened a new pull request #1221: URL: https://github.com/apache/hive/pull/1221 HIVE-23786 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] belugabehr closed pull request #1199: HIVE-23795: Add Additional Debugging Help for Import SQL
belugabehr closed pull request #1199: URL: https://github.com/apache/hive/pull/1199 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] belugabehr closed pull request #1197: HIVE-23793: Review of QueryInfo Class
belugabehr closed pull request #1197: URL: https://github.com/apache/hive/pull/1197 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kishendas commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client
kishendas commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r451058731
##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3580,7 +3592,19 @@ public boolean dropPartition(String dbName, String
tableName, List parti
List pvals = MetaStoreUtils.getPvals(t.getPartCols(), partSpec);
try {
- names = getMSC().listPartitionNames(dbName, tblName, pvals, max);
+ if (AcidUtils.isTransactionalTable(t)) {
+GetPartitionNamesPsRequest req = new GetPartitionNamesPsRequest();
+req.setTblName(tblName);
+req.setDbName(dbName);
+req.setPartValues(pvals);
+req.setMaxParts(max);
+ValidWriteIdList validWriteIdList = getValidWriteIdList(dbName,
tblName);
+req.setValidWriteIdList(validWriteIdList != null ?
validWriteIdList.toString() : null);
+GetPartitionNamesPsResponse res =
getMSC().listPartitionNamesRequest(req);
+names = res.getNames();
+ } else {
+names = getMSC().listPartitionNames(dbName, tblName, pvals, max);
Review comment:
Makes sense, done.
##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3602,9 +3626,27 @@ public boolean dropPartition(String dbName, String
tableName, List parti
exprBytes = SerializationUtilities.serializeExpressionToKryo(expr);
}
try {
+
String defaultPartitionName = HiveConf.getVar(conf,
ConfVars.DEFAULTPARTITIONNAME);
- names = getMSC().listPartitionNames(tbl.getCatalogName(),
tbl.getDbName(),
- tbl.getTableName(), defaultPartitionName, exprBytes, order,
maxParts);
+
+ if (AcidUtils.isTransactionalTable(tbl)) {
+PartitionsByExprRequest req =
+new PartitionsByExprRequest(tbl.getDbName(), tbl.getTableName(),
ByteBuffer.wrap(exprBytes));
+req.setDefaultPartitionName(defaultPartitionName);
+if (maxParts >= 0) {
+ req.setMaxParts(maxParts);
+}
+req.setOrder(order);
+req.setCatName(tbl.getCatalogName());
+ValidWriteIdList validWriteIdList =
getValidWriteIdList(tbl.getDbName(), tbl.getTableName());
+req.setValidWriteIdList(validWriteIdList != null ?
validWriteIdList.toString() : null);
+names = getMSC().listPartitionNames(req);
+
+ } else {
+names = getMSC()
+.listPartitionNames(tbl.getCatalogName(), tbl.getDbName(),
tbl.getTableName(), defaultPartitionName,
Review comment:
Makes sense, done.
##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3917,12 +3972,26 @@ public boolean dropPartition(String dbName, String
tableName, List parti
public boolean getPartitionsByExpr(Table tbl, ExprNodeGenericFuncDesc expr,
HiveConf conf,
List result) throws HiveException, TException {
assert result != null;
+boolean hasUnknownParts;
byte[] exprBytes = SerializationUtilities.serializeExpressionToKryo(expr);
String defaultPartitionName = HiveConf.getVar(conf,
ConfVars.DEFAULTPARTITIONNAME);
-List msParts =
-new ArrayList<>();
-boolean hasUnknownParts =
getMSC().listPartitionsSpecByExpr(tbl.getDbName(),
-tbl.getTableName(), exprBytes, defaultPartitionName, (short)-1,
msParts);
+List msParts = new
ArrayList<>();
+
+if (AcidUtils.isTransactionalTable(tbl)) {
+ PartitionsByExprRequest req = new PartitionsByExprRequest();
+ req.setDbName(tbl.getDbName());
+ req.setTblName((tbl.getTableName()));
+ req.setDefaultPartitionName(defaultPartitionName);
+ req.setMaxParts((short) -1);
+ req.setExpr(exprBytes);
+ ValidWriteIdList validWriteIdList = getValidWriteIdList(tbl.getDbName(),
tbl.getTableName());
+ req.setValidWriteIdList(validWriteIdList != null ?
validWriteIdList.toString() : null);
+ hasUnknownParts = getMSC().listPartitionsSpecByExpr(req, msParts);
+} else {
+ hasUnknownParts = getMSC()
+ .listPartitionsSpecByExpr(tbl.getDbName(), tbl.getTableName(),
exprBytes, defaultPartitionName, (short) -1,
Review comment:
Makes sense, 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kishendas commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client
kishendas commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r451057776
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableOperation.java
##
@@ -62,7 +62,8 @@ public int execute() throws HiveException {
if (desc.getReplicationSpec().isInReplicationScope()) {
// If in replication scope, we should check if the object we're looking
at exists, and if so,
// trigger replace-mode semantics.
- Table existingTable = context.getDb().getTable(tbl.getDbName(),
tbl.getTableName(), false);
+ Table existingTable = context.getDb().getTable(tbl.getDbName(),
+ tbl.getTableName(), false, true);
Review comment:
Fixed
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] nrg4878 closed pull request #1117: HIVE-23388: CTAS queries should use target's location for staging (Na…
nrg4878 closed pull request #1117: URL: https://github.com/apache/hive/pull/1117 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] nrg4878 commented on pull request #1117: HIVE-23388: CTAS queries should use target's location for staging (Na…
nrg4878 commented on pull request #1117: URL: https://github.com/apache/hive/pull/1117#issuecomment-654941642 Fix has been merged. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] belugabehr closed pull request #1118: HIVE-23363: Upgrade DataNucleus dependency to 5.2
belugabehr closed pull request #1118: URL: https://github.com/apache/hive/pull/1118 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] ramesh0201 commented on pull request #1177: HIVE-23665 Rewrite last_value to first_value to enable streaming results
ramesh0201 commented on pull request #1177: URL: https://github.com/apache/hive/pull/1177#issuecomment-654936523 hi @jcamachor, i have updated the PR to include the changes suggested. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] vineetgarg02 commented on a change in pull request #1212: HIVE-23807 Wrong results with vectorization enabled
vineetgarg02 commented on a change in pull request #1212:
URL: https://github.com/apache/hive/pull/1212#discussion_r450945185
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/exec/vector/expressions/CastStringToDate.java
##
@@ -116,14 +118,16 @@ public void evaluate(VectorizedRowBatch batch) {
private void evaluate(LongColumnVector outV, BytesColumnVector inV, int i) {
String dateString = new String(inV.vector[i], inV.start[i], inV.length[i],
StandardCharsets.UTF_8);
-if (dateParser.parseDate(dateString, sqlDate)) {
+try {
+ Date utilDate = Date.valueOf(dateString);
Review comment:
Yes this vectorized expression is generated for `GenericUDFDate`. I
don't see any way to trigger precommit job manually for branch-2. @kgyrtkirk
Do you know how can I trigger ptest for branch-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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] belugabehr commented on a change in pull request #1209: HIVE-22412: StatsUtils throw NPE when explain
belugabehr commented on a change in pull request #1209:
URL: https://github.com/apache/hive/pull/1209#discussion_r450931946
##
File path: ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java
##
@@ -1336,6 +1341,9 @@ public static long
getSizeOfPrimitiveTypeArraysFromType(String colType, int leng
*/
public static long getSizeOfMap(StandardConstantMapObjectInspector scmoi) {
Map map = scmoi.getWritableConstantValue();
+if (null == map || map.isEmpty()) {
+ return 0L;
+}
Review comment:
This changes the behavior of the existing code.
An empty map may return a value greater than 0 further down in this method:
```
// A result may be > 0, even with an empty map, because of the map's overhead
result += JavaDataModel.get().hashMap(map.entrySet().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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] belugabehr commented on a change in pull request #1209: HIVE-22412: StatsUtils throw NPE when explain
belugabehr commented on a change in pull request #1209:
URL: https://github.com/apache/hive/pull/1209#discussion_r450931946
##
File path: ql/src/java/org/apache/hadoop/hive/ql/stats/StatsUtils.java
##
@@ -1336,6 +1341,9 @@ public static long
getSizeOfPrimitiveTypeArraysFromType(String colType, int leng
*/
public static long getSizeOfMap(StandardConstantMapObjectInspector scmoi) {
Map map = scmoi.getWritableConstantValue();
+if (null == map || map.isEmpty()) {
+ return 0L;
+}
Review comment:
This changes the behavior of the existing code.
An empty map may return a value greater than 0 further down in this method:
```
// A result may be > 0 because of the map's overhead
result += JavaDataModel.get().hashMap(map.entrySet().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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] jcamachor commented on pull request #865: [HIVE-22634]Improperly SemanticException when filter is optimized to False on a partition table
jcamachor commented on pull request #865: URL: https://github.com/apache/hive/pull/865#issuecomment-654920582 @WangGuangxin , can we add the test to the 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] belugabehr closed pull request #1203: HIVE-22674: Replace Base64 in serde Package
belugabehr closed pull request #1203: URL: https://github.com/apache/hive/pull/1203 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] dengzhhu653 edited a comment on pull request #1149: HIVE-23727: Improve SQLOperation log handling when cleanup
dengzhhu653 edited a comment on pull request #1149: URL: https://github.com/apache/hive/pull/1149#issuecomment-648507858 @belugabehr can you please take a look at the changes? thanks 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvary commented on a change in pull request #1219:
URL: https://github.com/apache/hive/pull/1219#discussion_r450844285
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -568,6 +568,7 @@ public OpenTxnsResponse openTxns(OpenTxnRequest rqst)
throws MetaException {
*/
LOG.error("OpenTxnTimeOut exceeded commit duration {}, deleting
transactionIds: {}", elapsedMillis, txnIds);
deleteInvalidOpenTransactions(dbConn, txnIds);
+ dbConn.commit();
Review comment:
Maybe in a follow-up jira...
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvary commented on a change in pull request #1219:
URL: https://github.com/apache/hive/pull/1219#discussion_r450844111
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -568,6 +568,7 @@ public OpenTxnsResponse openTxns(OpenTxnRequest rqst)
throws MetaException {
*/
LOG.error("OpenTxnTimeOut exceeded commit duration {}, deleting
transactionIds: {}", elapsedMillis, txnIds);
deleteInvalidOpenTransactions(dbConn, txnIds);
+ dbConn.commit();
Review comment:
I thought close(null, stmt, dbConn) commits, but you are right, that it
does not commit.
Shall we create a test case for this? :)
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl opened a new pull request #1220: HIVE-23413: Create a new config to skip all locks
pvargacl opened a new pull request #1220: URL: https://github.com/apache/hive/pull/1220 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvargacl commented on pull request #1219: URL: https://github.com/apache/hive/pull/1219#issuecomment-654826153 @pvary It is already running :) http://ci.hive.apache.org/job/hive-flaky-check/64/ 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvargacl commented on a change in pull request #1219:
URL: https://github.com/apache/hive/pull/1219#discussion_r450829187
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -568,6 +568,7 @@ public OpenTxnsResponse openTxns(OpenTxnRequest rqst)
throws MetaException {
*/
LOG.error("OpenTxnTimeOut exceeded commit duration {}, deleting
transactionIds: {}", elapsedMillis, txnIds);
deleteInvalidOpenTransactions(dbConn, txnIds);
+ dbConn.commit();
Review comment:
It was missing, the delete was not persisted. When I was testing
manually, I didn't notice, because I was querying back the data on the same
connection...
It does not cause a big problem, these transactions would be cleaned up
eventually when they time out.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] WangGuangxin closed pull request #865: [HIVE-22634]Improperly SemanticException when filter is optimized to False on a partition table
WangGuangxin closed pull request #865: URL: https://github.com/apache/hive/pull/865 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] WangGuangxin opened a new pull request #865: [HIVE-22634]Improperly SemanticException when filter is optimized to False on a partition table
WangGuangxin opened a new pull request #865: URL: https://github.com/apache/hive/pull/865 When filter is optimized to False on a partition table, it will throw improperly SemanticException reporting that there is no partition predicate found. The step to reproduce is ``` set hive.strict.checks.no.partition.filter=true; CREATE TABLE test(id int, name string)PARTITIONED BY (`date` string); select * from test where `date` = '20191201' and 1<>1; ``` The above sql will throw "Queries against partitioned tables without a partition filter" exception. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvary commented on pull request #1219: URL: https://github.com/apache/hive/pull/1219#issuecomment-654819192 Could you please validate your fix with the flaky test checker jenkins, and add the relevant url? Thanks, Peter 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvary commented on a change in pull request #1219:
URL: https://github.com/apache/hive/pull/1219#discussion_r450823301
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -568,6 +568,7 @@ public OpenTxnsResponse openTxns(OpenTxnRequest rqst)
throws MetaException {
*/
LOG.error("OpenTxnTimeOut exceeded commit duration {}, deleting
transactionIds: {}", elapsedMillis, txnIds);
deleteInvalidOpenTransactions(dbConn, txnIds);
+ dbConn.commit();
Review comment:
Why is this change?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvary commented on a change in pull request #1219:
URL: https://github.com/apache/hive/pull/1219#discussion_r450822761
##
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestAcidTxnCleanerService.java
##
@@ -148,16 +149,16 @@ public void cleansEmptyAbortedBatchTxns() throws
Exception {
TxnStore.TIMED_OUT_TXN_ABORT_BATCH_SIZE + 50);
OpenTxnsResponse resp = txnHandler.openTxns(new OpenTxnRequest(
TxnStore.TIMED_OUT_TXN_ABORT_BATCH_SIZE + 50, "user", "hostname"));
+txnHandler.setOpenTxnTimeOutMillis(1);
txnHandler.abortTxns(new AbortTxnsRequest(resp.getTxn_ids()));
GetOpenTxnsResponse openTxns = txnHandler.getOpenTxns();
Assert.assertEquals(TxnStore.TIMED_OUT_TXN_ABORT_BATCH_SIZE + 50 + 1,
openTxns.getOpen_txnsSize());
-txnHandler.setOpenTxnTimeOutMillis(1);
underTest.run();
openTxns = txnHandler.getOpenTxns();
Assert.assertEquals(2, openTxns.getOpen_txnsSize());
-Assert.assertEquals(TxnStore.TIMED_OUT_TXN_ABORT_BATCH_SIZE + 50 + 1,
getMaxTxnId());
+Assert.assertTrue("The max txnId shoul be at least", getMaxTxnId() >=
TxnStore.TIMED_OUT_TXN_ABORT_BATCH_SIZE + 50 + 1);
Review comment:
nit: should
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvary commented on a change in pull request #1219:
URL: https://github.com/apache/hive/pull/1219#discussion_r450822656
##
File path:
standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/txn/TestAcidTxnCleanerService.java
##
@@ -135,7 +136,7 @@ public void cleansCommittedAndEmptyAbortedOnly() throws
Exception {
// kept only the 5 non-empty aborted ones
Assert.assertEquals(5, getTxnCount());
-Assert.assertEquals(15, getMaxTxnId());
+Assert.assertTrue("The max txnId shoul be at least 15", getMaxTxnId() >=
15);
Review comment:
nit: should
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl opened a new pull request #1219: HIVE-23525: TestAcidTxnCleanerService is unstable
pvargacl opened a new pull request #1219: URL: https://github.com/apache/hive/pull/1219 Fix flaky test 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kgyrtkirk opened a new pull request #1218: [wip] explore HIVE-18138 revert
kgyrtkirk opened a new pull request #1218: URL: https://github.com/apache/hive/pull/1218 ## NOTICE Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull request which starts with the corresponding JIRA issue number. (e.g. HIVE-X: Fix a typo in YYY) For more details, please see https://cwiki.apache.org/confluence/display/Hive/HowToContribute 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450801959
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java
##
@@ -229,102 +239,168 @@ public int repair(MsckInfo msckInfo) {
throw new MetastoreException(e);
}
}
+if (transactionalTable && !MetaStoreServerUtils.isPartitioned(table)) {
+ if (result.getMaxWriteId() > 0) {
+if (txnId < 0) {
+ // We need the txnId to check against even if we didn't do the
locking
+ txnId = getMsc().openTxn(getUserName());
+}
+
+validateAndAddMaxTxnIdAndWriteId(result.getMaxWriteId(),
result.getMaxTxnId(),
+table.getDbName(), table.getTableName(), txnId);
+ }
+}
}
success = true;
} catch (Exception e) {
LOG.warn("Failed to run metacheck: ", e);
success = false;
- ret = 1;
} finally {
- if (msckInfo.getResFile() != null) {
-BufferedWriter resultOut = null;
-try {
- Path resFile = new Path(msckInfo.getResFile());
- FileSystem fs = resFile.getFileSystem(getConf());
- resultOut = new BufferedWriter(new
OutputStreamWriter(fs.create(resFile)));
-
- boolean firstWritten = false;
- firstWritten |= writeMsckResult(result.getTablesNotInMs(),
-"Tables not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getTablesNotOnFs(),
-"Tables missing on filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotInMs(),
-"Partitions not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotOnFs(),
-"Partitions missing from filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getExpiredPartitions(),
-"Expired partitions (retention period: " + partitionExpirySeconds
+ "s) :", resultOut, firstWritten);
- // sorting to stabilize qfile output (msck_repair_drop.q)
- Collections.sort(repairOutput);
- for (String rout : repairOutput) {
-if (firstWritten) {
- resultOut.write(terminator);
-} else {
- firstWritten = true;
-}
-resultOut.write(rout);
- }
-} catch (IOException e) {
- LOG.warn("Failed to save metacheck output: ", e);
- ret = 1;
-} finally {
- if (resultOut != null) {
-try {
- resultOut.close();
-} catch (IOException e) {
- LOG.warn("Failed to close output file: ", e);
- ret = 1;
-}
- }
+ if (result!=null) {
+logResult(result);
+if (msckInfo.getResFile() != null) {
+ success = writeResultToFile(msckInfo, result, repairOutput,
partitionExpirySeconds) && success;
}
}
- LOG.info("Tables not in metastore: {}", result.getTablesNotInMs());
- LOG.info("Tables missing on filesystem: {}", result.getTablesNotOnFs());
- LOG.info("Partitions not in metastore: {}",
result.getPartitionsNotInMs());
- LOG.info("Partitions missing from filesystem: {}",
result.getPartitionsNotOnFs());
- LOG.info("Expired partitions: {}", result.getExpiredPartitions());
- if (acquireLock && txnId > 0) {
- if (success) {
-try {
- LOG.info("txnId: {} succeeded. Committing..", txnId);
- getMsc().commitTxn(txnId);
-} catch (Exception e) {
- LOG.warn("Error while committing txnId: {} for table: {}",
txnId, qualifiedTableName, e);
- ret = 1;
-}
- } else {
-try {
- LOG.info("txnId: {} failed. Aborting..", txnId);
- getMsc().abortTxns(Lists.newArrayList(txnId));
-} catch (Exception e) {
- LOG.warn("Error while aborting txnId: {} for table: {}", txnId,
qualifiedTableName, e);
- ret = 1;
-}
- }
+ if (txnId > 0) {
+success = closeTxn(qualifiedTableName, success, txnId) && success;
}
if (getMsc() != null) {
getMsc().close();
msc = null;
}
}
+return success ? 0 : 1;
+ }
+ private boolean closeTxn(String qualifiedTableName, boolean success, long
txnId) {
+boolean ret = true;
+if (success) {
+ try {
+LOG.info("txnId: {} succeeded. Committing..", txnId);
+getMsc().commitTxn(txnId);
+ } catch (Exception e) {
+LOG.warn("Error while committing txnId: {} for table: {}", txnId,
qualifiedTableName, e);
+ret = false;
+ }
+} else {
+ try {
+LOG.info("txnId: {} failed.
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450793830
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -2032,28 +2077,61 @@ public void
seedWriteIdOnAcidConversion(InitializeTableWriteIdsRequest rqst)
// The initial value for write id should be 1 and hence we add 1 with
number of write ids
// allocated here
String s = "INSERT INTO \"NEXT_WRITE_ID\" (\"NWI_DATABASE\",
\"NWI_TABLE\", \"NWI_NEXT\") VALUES (?, ?, "
-+ Long.toString(rqst.getSeeWriteId() + 1) + ")";
-pst = sqlGenerator.prepareStmtWithParameters(dbConn, s,
Arrays.asList(rqst.getDbName(), rqst.getTblName()));
++ Long.toString(rqst.getSeedWriteId() + 1) + ")";
+pst = sqlGenerator.prepareStmtWithParameters(dbConn, s,
Arrays.asList(rqst.getDbName(), rqst.getTableName()));
LOG.debug("Going to execute insert <" + s.replaceAll("\\?", "{}") +
">",
-quoteString(rqst.getDbName()), quoteString(rqst.getTblName()));
+quoteString(rqst.getDbName()),
quoteString(rqst.getTableName()));
pst.execute();
LOG.debug("Going to commit");
dbConn.commit();
} catch (SQLException e) {
-LOG.debug("Going to rollback");
rollbackDBConn(dbConn);
-checkRetryable(dbConn, e, "seedWriteIdOnAcidConversion(" + rqst + ")");
-throw new MetaException("Unable to update transaction database "
-+ StringUtils.stringifyException(e));
+checkRetryable(dbConn, e, "seedWriteId(" + rqst + ")");
+throw new MetaException("Unable to update transaction database " +
StringUtils.stringifyException(e));
} finally {
close(null, pst, dbConn);
unlockInternal();
}
} catch (RetryException e) {
- seedWriteIdOnAcidConversion(rqst);
+ seedWriteId(rqst);
}
+ }
+
+ @Override
+ public void seedTxnId(SeedTxnIdRequest rqst) throws MetaException {
+try {
+ Connection dbConn = null;
+ Statement stmt = null;
+ try {
+lockInternal();
+dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED);
+stmt = dbConn.createStatement();
+/*
+ * Locking the txnLock an exclusive way, we do not want to set the
txnId backward accidentally
+ * if there are concurrent open transactions
+ */
+acquireTxnLock(stmt, false);
+long highWaterMark = getHighWaterMark(stmt);
+if (highWaterMark >= rqst.getSeedTxnId()) {
Review comment:
This is not about the writeIds, it is about the txnId. If you have a
data from a database where there were high amount of transaction and the
compaction run on the table, you will have some high txnId in the
visibilityTxnId in the name of the compacted folder.
If you then move this data to a cluster with less transaction (ex. a test
cluster) and you run the msck repair, you have to skip the txnId forward so the
next query will read the compacted folder. Here the race condition is, that
somehow the txnId sequence gets ahead of you between the check and the seeding
the value, in that case we throw this exception to not to set the sequence
backward. Anyway, in this case if you run the msck repair again it will
succeed, since the txnid will be high enough.
The writeId race condition won't cause a problem I think, since if some
other transaction allocated the first writeId the seedWriteId will fail on the
unique constraint on the table
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450789075
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -2032,28 +2077,61 @@ public void
seedWriteIdOnAcidConversion(InitializeTableWriteIdsRequest rqst)
// The initial value for write id should be 1 and hence we add 1 with
number of write ids
// allocated here
String s = "INSERT INTO \"NEXT_WRITE_ID\" (\"NWI_DATABASE\",
\"NWI_TABLE\", \"NWI_NEXT\") VALUES (?, ?, "
-+ Long.toString(rqst.getSeeWriteId() + 1) + ")";
-pst = sqlGenerator.prepareStmtWithParameters(dbConn, s,
Arrays.asList(rqst.getDbName(), rqst.getTblName()));
++ Long.toString(rqst.getSeedWriteId() + 1) + ")";
+pst = sqlGenerator.prepareStmtWithParameters(dbConn, s,
Arrays.asList(rqst.getDbName(), rqst.getTableName()));
LOG.debug("Going to execute insert <" + s.replaceAll("\\?", "{}") +
">",
-quoteString(rqst.getDbName()), quoteString(rqst.getTblName()));
+quoteString(rqst.getDbName()),
quoteString(rqst.getTableName()));
pst.execute();
LOG.debug("Going to commit");
dbConn.commit();
} catch (SQLException e) {
-LOG.debug("Going to rollback");
rollbackDBConn(dbConn);
-checkRetryable(dbConn, e, "seedWriteIdOnAcidConversion(" + rqst + ")");
-throw new MetaException("Unable to update transaction database "
-+ StringUtils.stringifyException(e));
+checkRetryable(dbConn, e, "seedWriteId(" + rqst + ")");
+throw new MetaException("Unable to update transaction database " +
StringUtils.stringifyException(e));
} finally {
close(null, pst, dbConn);
unlockInternal();
Review comment:
fixed, the unique key on NEXT_WRITE_ID is enough
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on a change in pull request #1205: HIVE-23800: Make HiveServer2 oom hook interface
dengzhhu653 commented on a change in pull request #1205:
URL: https://github.com/apache/hive/pull/1205#discussion_r450788257
##
File path:
service/src/java/org/apache/hive/service/server/HiveServer2OomHookRunner.java
##
@@ -0,0 +1,102 @@
+/*
+ * 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.hive.service.server;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.common.JavaUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.util.ArrayList;
+import java.util.List;
+
+public class HiveServer2OomHookRunner implements Runnable {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveServer2OomHookRunner.class);
+ private OomHookContext context;
+ private final List hooks = new
ArrayList();
+
+ HiveServer2OomHookRunner(HiveServer2 hiveServer2, HiveConf hiveConf) {
+context = new OomHookContext(hiveServer2);
+// The hs2 has not been initialized yet, hiveServer2.getHiveConf() would
be null
+init(hiveConf);
+ }
+
+ private void init(HiveConf hiveConf) {
+String csHooks = hiveConf.getVar(ConfVars.HIVE_SERVER2_OOM_HOOKS);
+if (!StringUtils.isBlank(csHooks)) {
+ String[] hookClasses = csHooks.split(",");
+ for (String hookClass : hookClasses) {
+try {
+ Class clazz = JavaUtils.loadClass(hookClass.trim());
+ Constructor ctor = clazz.getDeclaredConstructor();
+ ctor.setAccessible(true);
Review comment:
Thanks for the review! Like DefaultOomHook, the hook declared as private
as nobody would access it except the HiveServer2OomHookRunner. In this case,
the default constructor cannot be used directly to create a instance until
calling setAccessible(true). The hooks loaded by HookRunner::loadHooksFromConf
should be declared as public access and I'm willing to follow this principal.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450787844
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -2015,8 +2019,49 @@ public AllocateTableWriteIdsResponse
allocateTableWriteIds(AllocateTableWriteIds
return allocateTableWriteIds(rqst);
}
}
+
+ @Override
+ public MaxAllocatedTableWriteIdResponse
getMaxAllocatedTableWrited(MaxAllocatedTableWriteIdRequest rqst) throws
MetaException {
+String dbName = rqst.getDbName();
+String tableName = rqst.getTableName();
+try {
+ Connection dbConn = null;
+ PreparedStatement pStmt = null;
+ ResultSet rs = null;
+ try {
+lockInternal();
+dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED);
+List params = Arrays.asList(dbName, tableName);
+String query = "SELECT \"NWI_NEXT\" FROM \"NEXT_WRITE_ID\" WHERE
\"NWI_DATABASE\" = ? AND \"NWI_TABLE\" = ?";
+pStmt = sqlGenerator.prepareStmtWithParameters(dbConn, query, params);
Review comment:
fixed
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450786917
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -2015,8 +2019,49 @@ public AllocateTableWriteIdsResponse
allocateTableWriteIds(AllocateTableWriteIds
return allocateTableWriteIds(rqst);
}
}
+
+ @Override
+ public MaxAllocatedTableWriteIdResponse
getMaxAllocatedTableWrited(MaxAllocatedTableWriteIdRequest rqst) throws
MetaException {
+String dbName = rqst.getDbName();
+String tableName = rqst.getTableName();
+try {
+ Connection dbConn = null;
+ PreparedStatement pStmt = null;
+ ResultSet rs = null;
+ try {
+lockInternal();
+dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED);
Review comment:
most of Txnhandler uses this pattern, instead of using three nested
try-with for dbconn, statement and resultset
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450785863
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -2015,8 +2019,49 @@ public AllocateTableWriteIdsResponse
allocateTableWriteIds(AllocateTableWriteIds
return allocateTableWriteIds(rqst);
}
}
+
+ @Override
+ public MaxAllocatedTableWriteIdResponse
getMaxAllocatedTableWrited(MaxAllocatedTableWriteIdRequest rqst) throws
MetaException {
+String dbName = rqst.getDbName();
+String tableName = rqst.getTableName();
+try {
+ Connection dbConn = null;
+ PreparedStatement pStmt = null;
+ ResultSet rs = null;
+ try {
+lockInternal();
Review comment:
fixed
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450785738
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -2015,8 +2019,49 @@ public AllocateTableWriteIdsResponse
allocateTableWriteIds(AllocateTableWriteIds
return allocateTableWriteIds(rqst);
}
}
+
+ @Override
+ public MaxAllocatedTableWriteIdResponse
getMaxAllocatedTableWrited(MaxAllocatedTableWriteIdRequest rqst) throws
MetaException {
+String dbName = rqst.getDbName();
+String tableName = rqst.getTableName();
+try {
+ Connection dbConn = null;
+ PreparedStatement pStmt = null;
+ ResultSet rs = null;
+ try {
+lockInternal();
+dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED);
+List params = Arrays.asList(dbName, tableName);
+String query = "SELECT \"NWI_NEXT\" FROM \"NEXT_WRITE_ID\" WHERE
\"NWI_DATABASE\" = ? AND \"NWI_TABLE\" = ?";
Review comment:
fixed
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnHandler.java
##
@@ -2015,8 +2019,49 @@ public AllocateTableWriteIdsResponse
allocateTableWriteIds(AllocateTableWriteIds
return allocateTableWriteIds(rqst);
}
}
+
+ @Override
+ public MaxAllocatedTableWriteIdResponse
getMaxAllocatedTableWrited(MaxAllocatedTableWriteIdRequest rqst) throws
MetaException {
+String dbName = rqst.getDbName();
+String tableName = rqst.getTableName();
+try {
+ Connection dbConn = null;
+ PreparedStatement pStmt = null;
+ ResultSet rs = null;
+ try {
+lockInternal();
+dbConn = getDbConn(Connection.TRANSACTION_READ_COMMITTED);
+List params = Arrays.asList(dbName, tableName);
+String query = "SELECT \"NWI_NEXT\" FROM \"NEXT_WRITE_ID\" WHERE
\"NWI_DATABASE\" = ? AND \"NWI_TABLE\" = ?";
+pStmt = sqlGenerator.prepareStmtWithParameters(dbConn, query, params);
+LOG.debug("Going to execute query <" + query.replaceAll("\\?", "{}") +
">", quoteString(dbName),
+quoteString(tableName));
+rs = pStmt.executeQuery();
+// If there is no record, we never allocated anything
+long maxWriteId = 0l;
+if (rs.next()) {
+ // The row contains the nextId not the previously allocated
+ maxWriteId = rs.getLong(1) - 1;
+}
+return new MaxAllocatedTableWriteIdResponse(maxWriteId);
+ } catch (SQLException e) {
+LOG.error(
+"Exception during reading the max allocated writeId for dbName={},
tableName={}. Will retry if possible.",
+dbName, tableName, e);
+rollbackDBConn(dbConn);
Review comment:
fixed
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r45058
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/txn/TxnDbUtil.java
##
@@ -313,6 +313,41 @@ private static void resetTxnSequence(Connection conn,
Statement stmt) throws SQL
}
}
+ /**
+ * Restarts the txnId sequence with the given seed value.
+ * It is the responsibility of the caller to not set the sequence backward.
+ * @param conn database connection
+ * @param stmt sql statement
+ * @param seedTxnId the seed value for the sequence
+ * @throws SQLException ex
+ */
+ public static void seedTxnSequence(Connection conn, Statement stmt, long
seedTxnId) throws SQLException {
+String dbProduct = conn.getMetaData().getDatabaseProductName();
+DatabaseProduct databaseProduct = determineDatabaseProduct(dbProduct);
+switch (databaseProduct) {
+
+case DERBY:
Review comment:
fixed
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client
pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r450777027
##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3917,12 +3972,26 @@ public boolean dropPartition(String dbName, String
tableName, List parti
public boolean getPartitionsByExpr(Table tbl, ExprNodeGenericFuncDesc expr,
HiveConf conf,
List result) throws HiveException, TException {
assert result != null;
+boolean hasUnknownParts;
byte[] exprBytes = SerializationUtilities.serializeExpressionToKryo(expr);
String defaultPartitionName = HiveConf.getVar(conf,
ConfVars.DEFAULTPARTITIONNAME);
-List msParts =
-new ArrayList<>();
-boolean hasUnknownParts =
getMSC().listPartitionsSpecByExpr(tbl.getDbName(),
-tbl.getTableName(), exprBytes, defaultPartitionName, (short)-1,
msParts);
+List msParts = new
ArrayList<>();
+
+if (AcidUtils.isTransactionalTable(tbl)) {
+ PartitionsByExprRequest req = new PartitionsByExprRequest();
+ req.setDbName(tbl.getDbName());
+ req.setTblName((tbl.getTableName()));
+ req.setDefaultPartitionName(defaultPartitionName);
+ req.setMaxParts((short) -1);
+ req.setExpr(exprBytes);
+ ValidWriteIdList validWriteIdList = getValidWriteIdList(tbl.getDbName(),
tbl.getTableName());
+ req.setValidWriteIdList(validWriteIdList != null ?
validWriteIdList.toString() : null);
+ hasUnknownParts = getMSC().listPartitionsSpecByExpr(req, msParts);
+} else {
+ hasUnknownParts = getMSC()
+ .listPartitionsSpecByExpr(tbl.getDbName(), tbl.getTableName(),
exprBytes, defaultPartitionName, (short) -1,
Review comment:
Maybe use the same listPartitionsSpecByExpr for both cases?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450765905
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java
##
@@ -383,6 +475,7 @@ public Void execute(int size) throws MetastoreException {
partsToAdd.add(partition);
lastBatch.add(part);
addMsgs.add(String.format(addMsgFormat,
part.getPartitionName()));
+ LOG.debug(String.format(addMsgFormat, part.getPartitionName()));
Review comment:
fixed
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450756822
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java
##
@@ -229,102 +239,168 @@ public int repair(MsckInfo msckInfo) {
throw new MetastoreException(e);
}
}
+if (transactionalTable && !MetaStoreServerUtils.isPartitioned(table)) {
+ if (result.getMaxWriteId() > 0) {
+if (txnId < 0) {
+ // We need the txnId to check against even if we didn't do the
locking
+ txnId = getMsc().openTxn(getUserName());
+}
+
+validateAndAddMaxTxnIdAndWriteId(result.getMaxWriteId(),
result.getMaxTxnId(),
+table.getDbName(), table.getTableName(), txnId);
+ }
+}
}
success = true;
} catch (Exception e) {
LOG.warn("Failed to run metacheck: ", e);
success = false;
- ret = 1;
} finally {
- if (msckInfo.getResFile() != null) {
-BufferedWriter resultOut = null;
-try {
- Path resFile = new Path(msckInfo.getResFile());
- FileSystem fs = resFile.getFileSystem(getConf());
- resultOut = new BufferedWriter(new
OutputStreamWriter(fs.create(resFile)));
-
- boolean firstWritten = false;
- firstWritten |= writeMsckResult(result.getTablesNotInMs(),
-"Tables not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getTablesNotOnFs(),
-"Tables missing on filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotInMs(),
-"Partitions not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotOnFs(),
-"Partitions missing from filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getExpiredPartitions(),
-"Expired partitions (retention period: " + partitionExpirySeconds
+ "s) :", resultOut, firstWritten);
- // sorting to stabilize qfile output (msck_repair_drop.q)
- Collections.sort(repairOutput);
- for (String rout : repairOutput) {
-if (firstWritten) {
- resultOut.write(terminator);
-} else {
- firstWritten = true;
-}
-resultOut.write(rout);
- }
-} catch (IOException e) {
- LOG.warn("Failed to save metacheck output: ", e);
- ret = 1;
-} finally {
- if (resultOut != null) {
-try {
- resultOut.close();
-} catch (IOException e) {
- LOG.warn("Failed to close output file: ", e);
- ret = 1;
-}
- }
+ if (result!=null) {
+logResult(result);
+if (msckInfo.getResFile() != null) {
+ success = writeResultToFile(msckInfo, result, repairOutput,
partitionExpirySeconds) && success;
}
}
- LOG.info("Tables not in metastore: {}", result.getTablesNotInMs());
- LOG.info("Tables missing on filesystem: {}", result.getTablesNotOnFs());
- LOG.info("Partitions not in metastore: {}",
result.getPartitionsNotInMs());
- LOG.info("Partitions missing from filesystem: {}",
result.getPartitionsNotOnFs());
- LOG.info("Expired partitions: {}", result.getExpiredPartitions());
- if (acquireLock && txnId > 0) {
- if (success) {
-try {
- LOG.info("txnId: {} succeeded. Committing..", txnId);
- getMsc().commitTxn(txnId);
-} catch (Exception e) {
- LOG.warn("Error while committing txnId: {} for table: {}",
txnId, qualifiedTableName, e);
- ret = 1;
-}
- } else {
-try {
- LOG.info("txnId: {} failed. Aborting..", txnId);
- getMsc().abortTxns(Lists.newArrayList(txnId));
-} catch (Exception e) {
- LOG.warn("Error while aborting txnId: {} for table: {}", txnId,
qualifiedTableName, e);
- ret = 1;
-}
- }
+ if (txnId > 0) {
+success = closeTxn(qualifiedTableName, success, txnId) && success;
}
if (getMsc() != null) {
getMsc().close();
msc = null;
}
}
+return success ? 0 : 1;
+ }
+ private boolean closeTxn(String qualifiedTableName, boolean success, long
txnId) {
+boolean ret = true;
+if (success) {
+ try {
+LOG.info("txnId: {} succeeded. Committing..", txnId);
+getMsc().commitTxn(txnId);
+ } catch (Exception e) {
+LOG.warn("Error while committing txnId: {} for table: {}", txnId,
qualifiedTableName, e);
+ret = false;
+ }
+} else {
+ try {
+LOG.info("txnId: {} failed.
[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client
pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r450754893
##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3580,7 +3592,19 @@ public boolean dropPartition(String dbName, String
tableName, List parti
List pvals = MetaStoreUtils.getPvals(t.getPartCols(), partSpec);
try {
- names = getMSC().listPartitionNames(dbName, tblName, pvals, max);
+ if (AcidUtils.isTransactionalTable(t)) {
+GetPartitionNamesPsRequest req = new GetPartitionNamesPsRequest();
+req.setTblName(tblName);
+req.setDbName(dbName);
+req.setPartValues(pvals);
+req.setMaxParts(max);
+ValidWriteIdList validWriteIdList = getValidWriteIdList(dbName,
tblName);
+req.setValidWriteIdList(validWriteIdList != null ?
validWriteIdList.toString() : null);
+GetPartitionNamesPsResponse res =
getMSC().listPartitionNamesRequest(req);
+names = res.getNames();
+ } else {
+names = getMSC().listPartitionNames(dbName, tblName, pvals, max);
Review comment:
Maybe use the same listPartitionNamesReques for acid and non-acid tables
as well?
##
File path: ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
##
@@ -3602,9 +3626,27 @@ public boolean dropPartition(String dbName, String
tableName, List parti
exprBytes = SerializationUtilities.serializeExpressionToKryo(expr);
}
try {
+
String defaultPartitionName = HiveConf.getVar(conf,
ConfVars.DEFAULTPARTITIONNAME);
- names = getMSC().listPartitionNames(tbl.getCatalogName(),
tbl.getDbName(),
- tbl.getTableName(), defaultPartitionName, exprBytes, order,
maxParts);
+
+ if (AcidUtils.isTransactionalTable(tbl)) {
+PartitionsByExprRequest req =
+new PartitionsByExprRequest(tbl.getDbName(), tbl.getTableName(),
ByteBuffer.wrap(exprBytes));
+req.setDefaultPartitionName(defaultPartitionName);
+if (maxParts >= 0) {
+ req.setMaxParts(maxParts);
+}
+req.setOrder(order);
+req.setCatName(tbl.getCatalogName());
+ValidWriteIdList validWriteIdList =
getValidWriteIdList(tbl.getDbName(), tbl.getTableName());
+req.setValidWriteIdList(validWriteIdList != null ?
validWriteIdList.toString() : null);
+names = getMSC().listPartitionNames(req);
+
+ } else {
+names = getMSC()
+.listPartitionNames(tbl.getCatalogName(), tbl.getDbName(),
tbl.getTableName(), defaultPartitionName,
Review comment:
Maybe use the same listPartitionNamesReques for acid and non-acid tables
as well?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450754328
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java
##
@@ -229,102 +239,168 @@ public int repair(MsckInfo msckInfo) {
throw new MetastoreException(e);
}
}
+if (transactionalTable && !MetaStoreServerUtils.isPartitioned(table)) {
+ if (result.getMaxWriteId() > 0) {
+if (txnId < 0) {
+ // We need the txnId to check against even if we didn't do the
locking
+ txnId = getMsc().openTxn(getUserName());
+}
+
+validateAndAddMaxTxnIdAndWriteId(result.getMaxWriteId(),
result.getMaxTxnId(),
+table.getDbName(), table.getTableName(), txnId);
+ }
+}
}
success = true;
} catch (Exception e) {
LOG.warn("Failed to run metacheck: ", e);
success = false;
- ret = 1;
} finally {
- if (msckInfo.getResFile() != null) {
-BufferedWriter resultOut = null;
-try {
- Path resFile = new Path(msckInfo.getResFile());
- FileSystem fs = resFile.getFileSystem(getConf());
- resultOut = new BufferedWriter(new
OutputStreamWriter(fs.create(resFile)));
-
- boolean firstWritten = false;
- firstWritten |= writeMsckResult(result.getTablesNotInMs(),
-"Tables not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getTablesNotOnFs(),
-"Tables missing on filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotInMs(),
-"Partitions not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotOnFs(),
-"Partitions missing from filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getExpiredPartitions(),
-"Expired partitions (retention period: " + partitionExpirySeconds
+ "s) :", resultOut, firstWritten);
- // sorting to stabilize qfile output (msck_repair_drop.q)
- Collections.sort(repairOutput);
- for (String rout : repairOutput) {
-if (firstWritten) {
- resultOut.write(terminator);
-} else {
- firstWritten = true;
-}
-resultOut.write(rout);
- }
-} catch (IOException e) {
- LOG.warn("Failed to save metacheck output: ", e);
- ret = 1;
-} finally {
- if (resultOut != null) {
-try {
- resultOut.close();
-} catch (IOException e) {
- LOG.warn("Failed to close output file: ", e);
- ret = 1;
-}
- }
+ if (result!=null) {
+logResult(result);
+if (msckInfo.getResFile() != null) {
+ success = writeResultToFile(msckInfo, result, repairOutput,
partitionExpirySeconds) && success;
}
}
- LOG.info("Tables not in metastore: {}", result.getTablesNotInMs());
- LOG.info("Tables missing on filesystem: {}", result.getTablesNotOnFs());
- LOG.info("Partitions not in metastore: {}",
result.getPartitionsNotInMs());
- LOG.info("Partitions missing from filesystem: {}",
result.getPartitionsNotOnFs());
- LOG.info("Expired partitions: {}", result.getExpiredPartitions());
- if (acquireLock && txnId > 0) {
- if (success) {
-try {
- LOG.info("txnId: {} succeeded. Committing..", txnId);
- getMsc().commitTxn(txnId);
-} catch (Exception e) {
- LOG.warn("Error while committing txnId: {} for table: {}",
txnId, qualifiedTableName, e);
- ret = 1;
-}
- } else {
-try {
- LOG.info("txnId: {} failed. Aborting..", txnId);
- getMsc().abortTxns(Lists.newArrayList(txnId));
-} catch (Exception e) {
- LOG.warn("Error while aborting txnId: {} for table: {}", txnId,
qualifiedTableName, e);
- ret = 1;
-}
- }
+ if (txnId > 0) {
+success = closeTxn(qualifiedTableName, success, txnId) && success;
}
if (getMsc() != null) {
getMsc().close();
msc = null;
}
}
+return success ? 0 : 1;
+ }
+ private boolean closeTxn(String qualifiedTableName, boolean success, long
txnId) {
+boolean ret = true;
+if (success) {
+ try {
+LOG.info("txnId: {} succeeded. Committing..", txnId);
+getMsc().commitTxn(txnId);
+ } catch (Exception e) {
+LOG.warn("Error while committing txnId: {} for table: {}", txnId,
qualifiedTableName, e);
+ret = false;
+ }
+} else {
+ try {
+LOG.info("txnId: {} failed.
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450754040
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java
##
@@ -229,102 +239,168 @@ public int repair(MsckInfo msckInfo) {
throw new MetastoreException(e);
}
}
+if (transactionalTable && !MetaStoreServerUtils.isPartitioned(table)) {
+ if (result.getMaxWriteId() > 0) {
+if (txnId < 0) {
+ // We need the txnId to check against even if we didn't do the
locking
+ txnId = getMsc().openTxn(getUserName());
+}
+
+validateAndAddMaxTxnIdAndWriteId(result.getMaxWriteId(),
result.getMaxTxnId(),
+table.getDbName(), table.getTableName(), txnId);
+ }
+}
}
success = true;
} catch (Exception e) {
LOG.warn("Failed to run metacheck: ", e);
success = false;
- ret = 1;
} finally {
- if (msckInfo.getResFile() != null) {
-BufferedWriter resultOut = null;
-try {
- Path resFile = new Path(msckInfo.getResFile());
- FileSystem fs = resFile.getFileSystem(getConf());
- resultOut = new BufferedWriter(new
OutputStreamWriter(fs.create(resFile)));
-
- boolean firstWritten = false;
- firstWritten |= writeMsckResult(result.getTablesNotInMs(),
-"Tables not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getTablesNotOnFs(),
-"Tables missing on filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotInMs(),
-"Partitions not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotOnFs(),
-"Partitions missing from filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getExpiredPartitions(),
-"Expired partitions (retention period: " + partitionExpirySeconds
+ "s) :", resultOut, firstWritten);
- // sorting to stabilize qfile output (msck_repair_drop.q)
- Collections.sort(repairOutput);
- for (String rout : repairOutput) {
-if (firstWritten) {
- resultOut.write(terminator);
-} else {
- firstWritten = true;
-}
-resultOut.write(rout);
- }
-} catch (IOException e) {
- LOG.warn("Failed to save metacheck output: ", e);
- ret = 1;
-} finally {
- if (resultOut != null) {
-try {
- resultOut.close();
-} catch (IOException e) {
- LOG.warn("Failed to close output file: ", e);
- ret = 1;
-}
- }
+ if (result!=null) {
+logResult(result);
+if (msckInfo.getResFile() != null) {
+ success = writeResultToFile(msckInfo, result, repairOutput,
partitionExpirySeconds) && success;
}
}
- LOG.info("Tables not in metastore: {}", result.getTablesNotInMs());
- LOG.info("Tables missing on filesystem: {}", result.getTablesNotOnFs());
- LOG.info("Partitions not in metastore: {}",
result.getPartitionsNotInMs());
- LOG.info("Partitions missing from filesystem: {}",
result.getPartitionsNotOnFs());
- LOG.info("Expired partitions: {}", result.getExpiredPartitions());
- if (acquireLock && txnId > 0) {
- if (success) {
-try {
- LOG.info("txnId: {} succeeded. Committing..", txnId);
- getMsc().commitTxn(txnId);
-} catch (Exception e) {
- LOG.warn("Error while committing txnId: {} for table: {}",
txnId, qualifiedTableName, e);
- ret = 1;
-}
- } else {
-try {
- LOG.info("txnId: {} failed. Aborting..", txnId);
- getMsc().abortTxns(Lists.newArrayList(txnId));
-} catch (Exception e) {
- LOG.warn("Error while aborting txnId: {} for table: {}", txnId,
qualifiedTableName, e);
- ret = 1;
-}
- }
+ if (txnId > 0) {
+success = closeTxn(qualifiedTableName, success, txnId) && success;
}
if (getMsc() != null) {
getMsc().close();
msc = null;
}
}
+return success ? 0 : 1;
+ }
+ private boolean closeTxn(String qualifiedTableName, boolean success, long
txnId) {
+boolean ret = true;
+if (success) {
+ try {
+LOG.info("txnId: {} succeeded. Committing..", txnId);
+getMsc().commitTxn(txnId);
+ } catch (Exception e) {
+LOG.warn("Error while committing txnId: {} for table: {}", txnId,
qualifiedTableName, e);
+ret = false;
+ }
+} else {
+ try {
+LOG.info("txnId: {} failed.
[GitHub] [hive] pvargacl commented on a change in pull request #1087: HIVE-23671: MSCK repair should handle transactional tables
pvargacl commented on a change in pull request #1087:
URL: https://github.com/apache/hive/pull/1087#discussion_r450753396
##
File path:
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/Msck.java
##
@@ -229,102 +239,168 @@ public int repair(MsckInfo msckInfo) {
throw new MetastoreException(e);
}
}
+if (transactionalTable && !MetaStoreServerUtils.isPartitioned(table)) {
+ if (result.getMaxWriteId() > 0) {
+if (txnId < 0) {
+ // We need the txnId to check against even if we didn't do the
locking
+ txnId = getMsc().openTxn(getUserName());
+}
+
+validateAndAddMaxTxnIdAndWriteId(result.getMaxWriteId(),
result.getMaxTxnId(),
+table.getDbName(), table.getTableName(), txnId);
+ }
+}
}
success = true;
} catch (Exception e) {
LOG.warn("Failed to run metacheck: ", e);
success = false;
- ret = 1;
} finally {
- if (msckInfo.getResFile() != null) {
-BufferedWriter resultOut = null;
-try {
- Path resFile = new Path(msckInfo.getResFile());
- FileSystem fs = resFile.getFileSystem(getConf());
- resultOut = new BufferedWriter(new
OutputStreamWriter(fs.create(resFile)));
-
- boolean firstWritten = false;
- firstWritten |= writeMsckResult(result.getTablesNotInMs(),
-"Tables not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getTablesNotOnFs(),
-"Tables missing on filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotInMs(),
-"Partitions not in metastore:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getPartitionsNotOnFs(),
-"Partitions missing from filesystem:", resultOut, firstWritten);
- firstWritten |= writeMsckResult(result.getExpiredPartitions(),
-"Expired partitions (retention period: " + partitionExpirySeconds
+ "s) :", resultOut, firstWritten);
- // sorting to stabilize qfile output (msck_repair_drop.q)
- Collections.sort(repairOutput);
- for (String rout : repairOutput) {
-if (firstWritten) {
- resultOut.write(terminator);
-} else {
- firstWritten = true;
-}
-resultOut.write(rout);
- }
-} catch (IOException e) {
- LOG.warn("Failed to save metacheck output: ", e);
- ret = 1;
-} finally {
- if (resultOut != null) {
-try {
- resultOut.close();
-} catch (IOException e) {
- LOG.warn("Failed to close output file: ", e);
- ret = 1;
-}
- }
+ if (result!=null) {
+logResult(result);
+if (msckInfo.getResFile() != null) {
+ success = writeResultToFile(msckInfo, result, repairOutput,
partitionExpirySeconds) && success;
}
}
- LOG.info("Tables not in metastore: {}", result.getTablesNotInMs());
- LOG.info("Tables missing on filesystem: {}", result.getTablesNotOnFs());
- LOG.info("Partitions not in metastore: {}",
result.getPartitionsNotInMs());
- LOG.info("Partitions missing from filesystem: {}",
result.getPartitionsNotOnFs());
- LOG.info("Expired partitions: {}", result.getExpiredPartitions());
- if (acquireLock && txnId > 0) {
- if (success) {
-try {
- LOG.info("txnId: {} succeeded. Committing..", txnId);
- getMsc().commitTxn(txnId);
-} catch (Exception e) {
- LOG.warn("Error while committing txnId: {} for table: {}",
txnId, qualifiedTableName, e);
- ret = 1;
-}
- } else {
-try {
- LOG.info("txnId: {} failed. Aborting..", txnId);
- getMsc().abortTxns(Lists.newArrayList(txnId));
-} catch (Exception e) {
- LOG.warn("Error while aborting txnId: {} for table: {}", txnId,
qualifiedTableName, e);
- ret = 1;
-}
- }
+ if (txnId > 0) {
+success = closeTxn(qualifiedTableName, success, txnId) && success;
}
if (getMsc() != null) {
getMsc().close();
msc = null;
}
}
+return success ? 0 : 1;
+ }
+ private boolean closeTxn(String qualifiedTableName, boolean success, long
txnId) {
+boolean ret = true;
+if (success) {
+ try {
+LOG.info("txnId: {} succeeded. Committing..", txnId);
+getMsc().commitTxn(txnId);
+ } catch (Exception e) {
+LOG.warn("Error while committing txnId: {} for table: {}", txnId,
qualifiedTableName, e);
+ret = false;
+ }
+} else {
+ try {
+LOG.info("txnId: {} failed.
[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client
pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r450752554
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableOperation.java
##
@@ -62,7 +62,8 @@ public int execute() throws HiveException {
if (desc.getReplicationSpec().isInReplicationScope()) {
// If in replication scope, we should check if the object we're looking
at exists, and if so,
// trigger replace-mode semantics.
- Table existingTable = context.getDb().getTable(tbl.getDbName(),
tbl.getTableName(), false);
+ Table existingTable = context.getDb().getTable(tbl.getDbName(),
+ tbl.getTableName(), false, true);
Review comment:
nit: Formatting - I think it is below 120, so we can put it into a
single line. If not, then we should indent the second line with 4 space
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] pvary commented on a change in pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client
pvary commented on a change in pull request #1217:
URL: https://github.com/apache/hive/pull/1217#discussion_r450752554
##
File path:
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/create/CreateTableOperation.java
##
@@ -62,7 +62,8 @@ public int execute() throws HiveException {
if (desc.getReplicationSpec().isInReplicationScope()) {
// If in replication scope, we should check if the object we're looking
at exists, and if so,
// trigger replace-mode semantics.
- Table existingTable = context.getDb().getTable(tbl.getDbName(),
tbl.getTableName(), false);
+ Table existingTable = context.getDb().getTable(tbl.getDbName(),
+ tbl.getTableName(), false, true);
Review comment:
nit: Formatting - I think it is below 120, so we can put it into a
single line. If not, then we should indent the second line with 4
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kishendas opened a new pull request #1217: Hive 23767 : Pass ValidWriteIdList in get_* HMS API requests from HMS Client
kishendas opened a new pull request #1217: URL: https://github.com/apache/hive/pull/1217 ## NOTICE Please create an issue in ASF JIRA before opening a pull request, and you need to set the title of the pull request which starts with the corresponding JIRA issue number. (e.g. HIVE-X: Fix a typo in YYY) For more details, please see https://cwiki.apache.org/confluence/display/Hive/HowToContribute 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kgyrtkirk commented on a change in pull request #1205: HIVE-23800: Make HiveServer2 oom hook interface
kgyrtkirk commented on a change in pull request #1205:
URL: https://github.com/apache/hive/pull/1205#discussion_r450699040
##
File path:
service/src/java/org/apache/hive/service/server/HiveServer2OomHookRunner.java
##
@@ -0,0 +1,102 @@
+/*
+ * 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.hive.service.server;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.common.JavaUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.util.ArrayList;
+import java.util.List;
+
+public class HiveServer2OomHookRunner implements Runnable {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveServer2OomHookRunner.class);
+ private OomHookContext context;
+ private final List hooks = new
ArrayList();
+
+ HiveServer2OomHookRunner(HiveServer2 hiveServer2, HiveConf hiveConf) {
+context = new OomHookContext(hiveServer2);
+// The hs2 has not been initialized yet, hiveServer2.getHiveConf() would
be null
+init(hiveConf);
+ }
+
+ private void init(HiveConf hiveConf) {
+String csHooks = hiveConf.getVar(ConfVars.HIVE_SERVER2_OOM_HOOKS);
+if (!StringUtils.isBlank(csHooks)) {
+ String[] hookClasses = csHooks.split(",");
+ for (String hookClass : hookClasses) {
+try {
+ Class clazz = JavaUtils.loadClass(hookClass.trim());
+ Constructor ctor = clazz.getDeclaredConstructor();
+ ctor.setAccessible(true);
Review comment:
please reuse parts of `HookRunner` for loading stuff
and why would you need `setAccessible` ?
##
File path:
service/src/java/org/apache/hive/service/server/HiveServer2OomHookRunner.java
##
@@ -0,0 +1,102 @@
+/*
+ * 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.hive.service.server;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.common.JavaUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
+import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.reflect.Constructor;
+import java.util.ArrayList;
+import java.util.List;
+
+public class HiveServer2OomHookRunner implements Runnable {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(HiveServer2OomHookRunner.class);
+ private OomHookContext context;
+ private final List hooks = new
ArrayList();
+
+ HiveServer2OomHookRunner(HiveServer2 hiveServer2, HiveConf hiveConf) {
+context = new OomHookContext(hiveServer2);
+// The hs2 has not been initialized yet, hiveServer2.getHiveConf() would
be null
+init(hiveConf);
+ }
+
+ private void init(HiveConf hiveConf) {
+String csHooks = hiveConf.getVar(ConfVars.HIVE_SERVER2_OOM_HOOKS);
+if (!StringUtils.isBlank(csHooks)) {
+ String[] hookClasses = csHooks.split(",");
+ for (String hookClass : hookClasses) {
+try {
+ Class clazz = JavaUtils.loadClass(hookClass.trim());
+ Constructor ctor = clazz.getDeclaredConstructor();
+ ctor.setAccessible(true);
+ hooks.add((OomHookWithContext)ctor.newInstance());
+} catch (Exception e) {
+
[GitHub] [hive] dengzhhu653 edited a comment on pull request #1205: HIVE-23800: Make HiveServer2 oom hook interface
dengzhhu653 edited a comment on pull request #1205: URL: https://github.com/apache/hive/pull/1205#issuecomment-654678113 > I don't see the need for thiswhat's wrong with [afterexecute](https://github.com/apache/hive/blob/e4256fc91fe2c123428400f3737883a83208d29e/ql/src/java/org/apache/hadoop/hive/ql/Executor.java#L533) or [failurehooks](https://github.com/apache/hive/blob/e4256fc91fe2c123428400f3737883a83208d29e/ql/src/java/org/apache/hadoop/hive/ql/Executor.java#L521)? > do you have a use case which could not be handled by those? The oom hook holds a hiveserver2 instance, which calls hiveserver2::stop() to end hiveserver2 gracefully, which would cleanup the session directory/scratch(staging) directory/operation log and so on . Although the hooks in the driver can handle oom, he may not be able to stop the hiveserver2 gracefully as the oom hook does. Sometimes we may want to dump the heap for futher analysis when oom happens or alter the devops, so it may be better to make the oom hook here an interface. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] dengzhhu653 commented on pull request #1205: HIVE-23800: Make HiveServer2 oom hook interface
dengzhhu653 commented on pull request #1205: URL: https://github.com/apache/hive/pull/1205#issuecomment-654678113 > I don't see the need for thiswhat's wrong with [afterexecute](https://github.com/apache/hive/blob/e4256fc91fe2c123428400f3737883a83208d29e/ql/src/java/org/apache/hadoop/hive/ql/Executor.java#L533) or [failurehooks](https://github.com/apache/hive/blob/e4256fc91fe2c123428400f3737883a83208d29e/ql/src/java/org/apache/hadoop/hive/ql/Executor.java#L521)? > do you have a use case which could not be handled by those? The oom hook holds a hiveserver2 instance, which calls hiveserver2::stop() to end hiveserver2 gracefully, which would cleanup the session directory/scratch directory/operation log and so on . Although the hooks in the driver can handle oom, he may not be able to stop the hiveserver2 gracefully as the oom hook does. Sometimes we may want to dump the heap for futher analysis when oom happens or alter the devops, so it may be better to make the oom hook here an interface. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] dlavati edited a comment on pull request #1207: HIVE-23483: Remove DynamicSerDe
dlavati edited a comment on pull request #1207: URL: https://github.com/apache/hive/pull/1207#issuecomment-654666554 > > Looks like my new commit messed up the labeling (fyi @kgyrtkirk) > > The checkout step for http://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-1207/2/tests/ shows my second commit already ([0ec9e25](https://github.com/apache/hive/commit/0ec9e25c7497461b9ee8e1bae89e913da60e6871)) > > not really; what happened was that you submitted a commit while it have started processing - so it skipped the run with the first commit I see, thanks. It was just strange that the pending label is left there, even though the test actually ran and passed. But now it shows passed. Maybe I missed this after/before refreshing. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] dlavati commented on pull request #1207: HIVE-23483: Remove DynamicSerDe
dlavati commented on pull request #1207: URL: https://github.com/apache/hive/pull/1207#issuecomment-654666554 > > Looks like my new commit messed up the labeling (fyi @kgyrtkirk) > > The checkout step for http://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-1207/2/tests/ shows my second commit already ([0ec9e25](https://github.com/apache/hive/commit/0ec9e25c7497461b9ee8e1bae89e913da60e6871)) > > not really; what happened was that you submitted a commit while it have started processing - so it skipped the run with the first commit I see, thanks. It was just strange that the pending label is left there, even though the test actually ran and passed. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] klcopp closed pull request #1175: HIVE-23760: Upgrading to Kafka 2.5 Clients
klcopp closed pull request #1175: URL: https://github.com/apache/hive/pull/1175 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] klcopp opened a new pull request #1216: HIVE-23760: Upgrading to Kafka 2.5 Clients
klcopp opened a new pull request #1216: URL: https://github.com/apache/hive/pull/1216 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] klcopp commented on pull request #1175: HIVE-23760: Upgrading to Kafka 2.5 Clients
klcopp commented on pull request #1175: URL: https://github.com/apache/hive/pull/1175#issuecomment-654663659 Moved to https://github.com/apache/hive/pull/1216 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kgyrtkirk commented on pull request #1205: HIVE-23800: Make HiveServer2 oom hook interface
kgyrtkirk commented on pull request #1205: URL: https://github.com/apache/hive/pull/1205#issuecomment-654663722 I don't see the need for thiswhat's wrong with [afterexecute](https://github.com/apache/hive/blob/e4256fc91fe2c123428400f3737883a83208d29e/ql/src/java/org/apache/hadoop/hive/ql/Executor.java#L533) or [failurehooks](https://github.com/apache/hive/blob/e4256fc91fe2c123428400f3737883a83208d29e/ql/src/java/org/apache/hadoop/hive/ql/Executor.java#L521)? do you have a use case which could not be handled by those? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] dlavati commented on pull request #1207: HIVE-23483: Remove DynamicSerDe
dlavati commented on pull request #1207: URL: https://github.com/apache/hive/pull/1207#issuecomment-654658403 I've seen some additional usage, however I'm not sure how these are utilized, thus I left it in so far: ``` ql/src/java/org/apache/hadoop/hive/ql/io/HiveBinaryOutputFormat.java:76: // DynamicSerDe always writes out BytesWritable serde/src/java/org/apache/hadoop/hive/serde2/thrift/TBinarySortableProtocol.java:150: // If the struct is null and level > 1, DynamicSerDe will call serde/src/java/org/apache/hadoop/hive/serde2/thrift/TBinarySortableProtocol.java:418: // slight hack to communicate to DynamicSerDe that the field ids are not serde/src/java/org/apache/hadoop/hive/serde2/thrift/TCTLSeparatedProtocol.java:667: // slight hack to communicate to DynamicSerDe that the field ids are not ``` 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kgyrtkirk commented on pull request #1207: HIVE-23483: Remove DynamicSerDe
kgyrtkirk commented on pull request #1207: URL: https://github.com/apache/hive/pull/1207#issuecomment-654658790 > Looks like my new commit messed up the labeling (fyi @kgyrtkirk) > The checkout step for http://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-1207/2/tests/ shows my second commit already ([0ec9e25](https://github.com/apache/hive/commit/0ec9e25c7497461b9ee8e1bae89e913da60e6871)) not really; what happened was that you submitted a commit while it have started processing - so it skipped the run with the first commit 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] dlavati commented on pull request #1207: HIVE-23483: Remove DynamicSerDe
dlavati commented on pull request #1207: URL: https://github.com/apache/hive/pull/1207#issuecomment-654656802 Looks like my new commit messed up the labeling (fyi @kgyrtkirk) The checkout step for http://ci.hive.apache.org/blue/organizations/jenkins/hive-precommit/detail/PR-1207/2/tests/ shows my second commit already (0ec9e25) 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] dlavati commented on pull request #1207: HIVE-23483: Remove DynamicSerDe
dlavati commented on pull request #1207: URL: https://github.com/apache/hive/pull/1207#issuecomment-654657249 @ashutoshc, @pgaref could you please take a look at this? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kgyrtkirk commented on a change in pull request #1209: HIVE-22412: StatsUtils throw NPE when explain
kgyrtkirk commented on a change in pull request #1209:
URL: https://github.com/apache/hive/pull/1209#discussion_r450655717
##
File path:
serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/StandardConstantMapObjectInspector.java
##
@@ -40,8 +41,10 @@ protected StandardConstantMapObjectInspector() {
*/
protected StandardConstantMapObjectInspector(ObjectInspector
mapKeyObjectInspector,
ObjectInspector mapValueObjectInspector, Map value) {
-super(mapKeyObjectInspector, mapValueObjectInspector);
-this.value = value;
+ super(mapKeyObjectInspector, mapValueObjectInspector);
+ if (null != value) {
Review comment:
please remove this and similar conditionals a map which is `NULL` is
not `EMPTY`!
this should be fixed in `StatsUtils`
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] kgyrtkirk opened a new pull request #1215: HIVE-23806 avoid stat clear
kgyrtkirk opened a new pull request #1215: URL: https://github.com/apache/hive/pull/1215 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
[GitHub] [hive] mustafaiman commented on pull request #1192: HIVE-23780: Fail dropTable if acid cleanup fails
mustafaiman commented on pull request #1192: URL: https://github.com/apache/hive/pull/1192#issuecomment-654621572 @pvary can you review again? The existing tests used a hack to create transactional metastore tables on the spot when they were accessed the first time. When this happened during a dropDatabase call, it would lead to a deadlock. So I had to move the creating of these tables to initialization part of such tests. The hack was purely for testing, therefore it is safe to remove. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org For additional commands, e-mail: gitbox-h...@hive.apache.org
