[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16761064#comment-16761064 ] Karthik Manamcheri commented on HIVE-20977: --- Can we merge this [~pvary]? > Lazy evaluate the table object in PreReadTableEvent to improve get_partition > performance > > > Key: HIVE-20977 > URL: https://issues.apache.org/jira/browse/HIVE-20977 > Project: Hive > Issue Type: Improvement >Reporter: Karthik Manamcheri >Assignee: Karthik Manamcheri >Priority: Minor > Fix For: 4.0.0 > > Attachments: HIVE-20977.1.patch, HIVE-20977.2.patch, > HIVE-20977.3.patch, HIVE-20977.4.patch > > > The PreReadTableEvent is generated for non-table operations (such as > get_partitions), but only if there is an event listener attached. However, > this is also not necessary if the event listener is not interested in the > read table event. > For example, the TransactionalValidationListener's onEvent looks like this > {code:java} > @Override > public void onEvent(PreEventContext context) throws MetaException, > NoSuchObjectException, > InvalidOperationException { > switch (context.getEventType()) { > case CREATE_TABLE: > handle((PreCreateTableEvent) context); > break; > case ALTER_TABLE: > handle((PreAlterTableEvent) context); > break; > default: > //no validation required.. > } > }{code} > > Note that for read table events it is a no-op. The problem is that the > get_table is evaluated when creating the PreReadTableEvent finally to be just > ignored! > Look at the code below.. {{getMS().getTable(..)}} is evaluated irrespective > of if the listener uses it or not. > {code:java} > private void fireReadTablePreEvent(String catName, String dbName, String > tblName) > throws MetaException, NoSuchObjectException { > if(preListeners.size() > 0) { > // do this only if there is a pre event listener registered (avoid > unnecessary > // metastore api call) > Table t = getMS().getTable(catName, dbName, tblName); > if (t == null) { > throw new NoSuchObjectException(TableName.getQualified(catName, dbName, > tblName) > + " table not found"); > } > firePreEvent(new PreReadTableEvent(t, this)); > } > } > {code} > This can be improved by using a {{Supplier}} and lazily evaluating the table > when needed (once when the first time it is called, memorized after that). > *Motivation* > Whenever a partition call occurs (get_partition, etc.), we fire the > PreReadTableEvent. This affects performance since it fetches the table even > if it is not being used. This change will improve performance on the > get_partition calls. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16769272#comment-16769272 ] Peter Vary commented on HIVE-20977: --- [~karthik.manamcheri]: The change could not be applied to master :( Could you please rebase? Thanks, Peter > Lazy evaluate the table object in PreReadTableEvent to improve get_partition > performance > > > Key: HIVE-20977 > URL: https://issues.apache.org/jira/browse/HIVE-20977 > Project: Hive > Issue Type: Improvement >Reporter: Karthik Manamcheri >Assignee: Karthik Manamcheri >Priority: Minor > Fix For: 4.0.0 > > Attachments: HIVE-20977.1.patch, HIVE-20977.2.patch, > HIVE-20977.3.patch, HIVE-20977.4.patch > > > The PreReadTableEvent is generated for non-table operations (such as > get_partitions), but only if there is an event listener attached. However, > this is also not necessary if the event listener is not interested in the > read table event. > For example, the TransactionalValidationListener's onEvent looks like this > {code:java} > @Override > public void onEvent(PreEventContext context) throws MetaException, > NoSuchObjectException, > InvalidOperationException { > switch (context.getEventType()) { > case CREATE_TABLE: > handle((PreCreateTableEvent) context); > break; > case ALTER_TABLE: > handle((PreAlterTableEvent) context); > break; > default: > //no validation required.. > } > }{code} > > Note that for read table events it is a no-op. The problem is that the > get_table is evaluated when creating the PreReadTableEvent finally to be just > ignored! > Look at the code below.. {{getMS().getTable(..)}} is evaluated irrespective > of if the listener uses it or not. > {code:java} > private void fireReadTablePreEvent(String catName, String dbName, String > tblName) > throws MetaException, NoSuchObjectException { > if(preListeners.size() > 0) { > // do this only if there is a pre event listener registered (avoid > unnecessary > // metastore api call) > Table t = getMS().getTable(catName, dbName, tblName); > if (t == null) { > throw new NoSuchObjectException(TableName.getQualified(catName, dbName, > tblName) > + " table not found"); > } > firePreEvent(new PreReadTableEvent(t, this)); > } > } > {code} > This can be improved by using a {{Supplier}} and lazily evaluating the table > when needed (once when the first time it is called, memorized after that). > *Motivation* > Whenever a partition call occurs (get_partition, etc.), we fire the > PreReadTableEvent. This affects performance since it fetches the table even > if it is not being used. This change will improve performance on the > get_partition calls. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16730523#comment-16730523 ] Hive QA commented on HIVE-20977: | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | || || || || {color:brown} master Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m 34s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 23s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 6s{color} | {color:green} master passed {color} | | {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 1m 4s{color} | {color:blue} standalone-metastore/metastore-server in master has 188 extant Findbugs warnings. {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 17s{color} | {color:green} master passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 27s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 24s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 24s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 6s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 12s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 18s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:red}-1{color} | {color:red} asflicense {color} | {color:red} 0m 13s{color} | {color:red} The patch generated 1 ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 13m 21s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Optional Tests | asflicense javac javadoc findbugs checkstyle compile | | uname | Linux hiveptest-server-upstream 3.16.0-4-amd64 #1 SMP Debian 3.16.36-1+deb8u1 (2016-09-03) x86_64 GNU/Linux | | Build tool | maven | | Personality | /data/hiveptest/working/yetus_PreCommit-HIVE-Build-15443/dev-support/hive-personality.sh | | git revision | master / 926c1e8 | | Default Java | 1.8.0_111 | | findbugs | v3.0.0 | | asflicense | http://104.198.109.242/logs//PreCommit-HIVE-Build-15443/yetus/patch-asflicense-problems.txt | | modules | C: standalone-metastore/metastore-server U: standalone-metastore/metastore-server | | Console output | http://104.198.109.242/logs//PreCommit-HIVE-Build-15443/yetus.txt | | Powered by | Apache Yetushttp://yetus.apache.org | This message was automatically generated. > Lazy evaluate the table object in PreReadTableEvent to improve get_partition > performance > > > Key: HIVE-20977 > URL: https://issues.apache.org/jira/browse/HIVE-20977 > Project: Hive > Issue Type: Improvement >Reporter: Karthik Manamcheri >Assignee: Karthik Manamcheri >Priority: Minor > Attachments: HIVE-20977.1.patch > > > The PreReadTableEvent is generated for non-table operations (such as > get_partitions), but only if there is an event listener attached. However, > this is also not necessary if the event listener is not interested in the > read table event. > For example, the TransactionalValidationListener's onEvent looks like this > {code:java} > @Override > public void onEvent(PreEventContext context) throws MetaException, > NoSuchObjectException, > InvalidOperationException { > switch (context.getEventType()) { > case CREATE_TABLE: > handle((PreCreateTableEvent) context); > break; > case ALTER_TABLE: > handle((PreAlterTableEvent) context); > break; > default: > //no validation required.. > } > }{code} > > Note that for read table events it is a no-op. The problem is that the > get_table is evaluated when creating the PreReadTableEvent finally to be just > ignored! > Look at the code below.. {{getMS().getTable(..)}} is evaluated irre
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16730530#comment-16730530 ] Hive QA commented on HIVE-20977: Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12953271/HIVE-20977.1.patch {color:green}SUCCESS:{color} +1 due to 2 test(s) being added or modified. {color:red}ERROR:{color} -1 due to 56 failed/errored test(s), 15761 tests executed *Failed tests:* {noformat} org.apache.hadoop.hive.cli.TestMiniSparkOnYarnCliDriver.testCliDriver[schemeAuthority] (batchId=191) org.apache.hadoop.hive.metastore.TestEmbeddedHiveMetaStore.testPartitionFilter (batchId=223) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStore.testPartitionFilter (batchId=224) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStoreZK.testPartitionFilter (batchId=226) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStoreZKBindHost.testPartitionFilter (batchId=230) org.apache.hadoop.hive.metastore.TestSetUGIOnBothClientServer.testPartitionFilter (batchId=221) org.apache.hadoop.hive.metastore.TestSetUGIOnOnlyClient.testPartitionFilter (batchId=219) org.apache.hadoop.hive.metastore.TestSetUGIOnOnlyServer.testPartitionFilter (batchId=229) org.apache.hadoop.hive.metastore.client.TestGetPartitions.getPartitionsByNamesBogusCatalog[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.getPartitionsByNamesBogusCatalog[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDb[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDb[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTable[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTable[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestListPartitions.listPartitionNamesBogusCatalog[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.listPartitionNamesBogusCatalog[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullDbName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullDbName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullTblName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullTblName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDbName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDbName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDb[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDb[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTable[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTable[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTblName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16732303#comment-16732303 ] Hive QA commented on HIVE-20977: | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 1s{color} | {color:green} The patch does not contain any @author tags. {color} | || || || || {color:brown} master Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 7m 50s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 23s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 6s{color} | {color:green} master passed {color} | | {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 1m 3s{color} | {color:blue} standalone-metastore/metastore-server in master has 188 extant Findbugs warnings. {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 17s{color} | {color:green} master passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 26s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 24s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 24s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 6s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 12s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 19s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:red}-1{color} | {color:red} asflicense {color} | {color:red} 0m 12s{color} | {color:red} The patch generated 1 ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 12m 42s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Optional Tests | asflicense javac javadoc findbugs checkstyle compile | | uname | Linux hiveptest-server-upstream 3.16.0-4-amd64 #1 SMP Debian 3.16.36-1+deb8u1 (2016-09-03) x86_64 GNU/Linux | | Build tool | maven | | Personality | /data/hiveptest/working/yetus_PreCommit-HIVE-Build-15455/dev-support/hive-personality.sh | | git revision | master / 926c1e8 | | Default Java | 1.8.0_111 | | findbugs | v3.0.0 | | asflicense | http://104.198.109.242/logs//PreCommit-HIVE-Build-15455/yetus/patch-asflicense-problems.txt | | modules | C: standalone-metastore/metastore-server U: standalone-metastore/metastore-server | | Console output | http://104.198.109.242/logs//PreCommit-HIVE-Build-15455/yetus.txt | | Powered by | Apache Yetushttp://yetus.apache.org | This message was automatically generated. > Lazy evaluate the table object in PreReadTableEvent to improve get_partition > performance > > > Key: HIVE-20977 > URL: https://issues.apache.org/jira/browse/HIVE-20977 > Project: Hive > Issue Type: Improvement >Reporter: Karthik Manamcheri >Assignee: Karthik Manamcheri >Priority: Minor > Attachments: HIVE-20977.1.patch, HIVE-20977.2.patch > > > The PreReadTableEvent is generated for non-table operations (such as > get_partitions), but only if there is an event listener attached. However, > this is also not necessary if the event listener is not interested in the > read table event. > For example, the TransactionalValidationListener's onEvent looks like this > {code:java} > @Override > public void onEvent(PreEventContext context) throws MetaException, > NoSuchObjectException, > InvalidOperationException { > switch (context.getEventType()) { > case CREATE_TABLE: > handle((PreCreateTableEvent) context); > break; > case ALTER_TABLE: > handle((PreAlterTableEvent) context); > break; > default: > //no validation required.. > } > }{code} > > Note that for read table events it is a no-op. The problem is that the > get_table is evaluated when creating the PreReadTableEvent finally to be just > ignored! > Look at the code below.. {{getMS().getTable(..)
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16732361#comment-16732361 ] Hive QA commented on HIVE-20977: Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12953518/HIVE-20977.2.patch {color:green}SUCCESS:{color} +1 due to 2 test(s) being added or modified. {color:red}ERROR:{color} -1 due to 55 failed/errored test(s), 15761 tests executed *Failed tests:* {noformat} org.apache.hadoop.hive.metastore.TestEmbeddedHiveMetaStore.testPartitionFilter (batchId=223) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStore.testPartitionFilter (batchId=224) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStoreZK.testPartitionFilter (batchId=226) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStoreZKBindHost.testPartitionFilter (batchId=230) org.apache.hadoop.hive.metastore.TestSetUGIOnBothClientServer.testPartitionFilter (batchId=221) org.apache.hadoop.hive.metastore.TestSetUGIOnOnlyClient.testPartitionFilter (batchId=219) org.apache.hadoop.hive.metastore.TestSetUGIOnOnlyServer.testPartitionFilter (batchId=229) org.apache.hadoop.hive.metastore.client.TestGetPartitions.getPartitionsByNamesBogusCatalog[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.getPartitionsByNamesBogusCatalog[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDb[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDb[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTable[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTable[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestListPartitions.listPartitionNamesBogusCatalog[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.listPartitionNamesBogusCatalog[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullDbName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullDbName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullTblName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullTblName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDbName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDbName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDb[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDb[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTable[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTable[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTblName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTblName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestLi
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16732432#comment-16732432 ] Hive QA commented on HIVE-20977: | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | || || || || {color:brown} master Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m 14s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 23s{color} | {color:green} master passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 6s{color} | {color:green} master passed {color} | | {color:blue}0{color} | {color:blue} findbugs {color} | {color:blue} 1m 2s{color} | {color:blue} standalone-metastore/metastore-server in master has 188 extant Findbugs warnings. {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 17s{color} | {color:green} master passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 26s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 25s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 25s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 7s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 1m 7s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 18s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:red}-1{color} | {color:red} asflicense {color} | {color:red} 0m 12s{color} | {color:red} The patch generated 1 ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 12m 56s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Optional Tests | asflicense javac javadoc findbugs checkstyle compile | | uname | Linux hiveptest-server-upstream 3.16.0-4-amd64 #1 SMP Debian 3.16.36-1+deb8u1 (2016-09-03) x86_64 GNU/Linux | | Build tool | maven | | Personality | /data/hiveptest/working/yetus_PreCommit-HIVE-Build-15458/dev-support/hive-personality.sh | | git revision | master / dc215b1 | | Default Java | 1.8.0_111 | | findbugs | v3.0.0 | | asflicense | http://104.198.109.242/logs//PreCommit-HIVE-Build-15458/yetus/patch-asflicense-problems.txt | | modules | C: standalone-metastore/metastore-server U: standalone-metastore/metastore-server | | Console output | http://104.198.109.242/logs//PreCommit-HIVE-Build-15458/yetus.txt | | Powered by | Apache Yetushttp://yetus.apache.org | This message was automatically generated. > Lazy evaluate the table object in PreReadTableEvent to improve get_partition > performance > > > Key: HIVE-20977 > URL: https://issues.apache.org/jira/browse/HIVE-20977 > Project: Hive > Issue Type: Improvement >Reporter: Karthik Manamcheri >Assignee: Karthik Manamcheri >Priority: Minor > Attachments: HIVE-20977.1.patch, HIVE-20977.2.patch, > HIVE-20977.3.patch > > > The PreReadTableEvent is generated for non-table operations (such as > get_partitions), but only if there is an event listener attached. However, > this is also not necessary if the event listener is not interested in the > read table event. > For example, the TransactionalValidationListener's onEvent looks like this > {code:java} > @Override > public void onEvent(PreEventContext context) throws MetaException, > NoSuchObjectException, > InvalidOperationException { > switch (context.getEventType()) { > case CREATE_TABLE: > handle((PreCreateTableEvent) context); > break; > case ALTER_TABLE: > handle((PreAlterTableEvent) context); > break; > default: > //no validation required.. > } > }{code} > > Note that for read table events it is a no-op. The problem is that the > get_table is evaluated when creating the PreReadTableEvent finally to be just > ignored! > Look at the code below..
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16732468#comment-16732468 ] Hive QA commented on HIVE-20977: Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12953526/HIVE-20977.3.patch {color:green}SUCCESS:{color} +1 due to 2 test(s) being added or modified. {color:red}ERROR:{color} -1 due to 59 failed/errored test(s), 15761 tests executed *Failed tests:* {noformat} org.apache.hadoop.hive.metastore.TestEmbeddedHiveMetaStore.testPartitionFilter (batchId=223) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStore.testPartitionFilter (batchId=224) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStoreZK.testPartitionFilter (batchId=226) org.apache.hadoop.hive.metastore.TestRemoteHiveMetaStoreZKBindHost.testPartitionFilter (batchId=230) org.apache.hadoop.hive.metastore.TestSetUGIOnBothClientServer.testPartitionFilter (batchId=221) org.apache.hadoop.hive.metastore.TestSetUGIOnOnlyClient.testPartitionFilter (batchId=219) org.apache.hadoop.hive.metastore.TestSetUGIOnOnlyServer.testPartitionFilter (batchId=229) org.apache.hadoop.hive.metastore.client.TestGetPartitions.getPartitionsByNamesBogusCatalog[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.getPartitionsByNamesBogusCatalog[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionWithAuthInfoNullTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDb[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoDb[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTable[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTable[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNoTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullDbName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullDbName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullTblName[Embedded] (batchId=222) org.apache.hadoop.hive.metastore.client.TestGetPartitions.testGetPartitionsByNamesNullTblName[Remote] (batchId=222) org.apache.hadoop.hive.metastore.client.TestListPartitions.listPartitionNamesBogusCatalog[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.listPartitionNamesBogusCatalog[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullDbName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullDbName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullTblName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesByValuesNullTblName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDbName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDbName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDb[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoDb[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTable[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTable[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTblName[Embedded] (batchId=220) org.apache.hadoop.hive.metastore.client.TestListPartitions.testListPartitionNamesNoTblName[Remote] (batchId=220) org.apache.hadoop.hive.metastore.client.TestLi
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16734700#comment-16734700 ] Karthik Manamcheri commented on HIVE-20977: --- [~alangates] [~thejas] I want to hear your thoughts on a particular issue regarding this change. In this ticket, I am changing the behavior of bulk listing partitions API. Previously, if you issue a getPartitionsByNames (or one of the similar APIs) with an empty database, we threw an exception. After this change, we will return an empty list of partitions instead. This behavior is similar to what happens if you issue a getTablesByNames call (an empty list of tables are returned). Here is my thoughts on why this is an ok change. In the master branch (without my change), if you remove the TransactionalValidationListener from the list of pre-listeners, the GetPartitions API tests fail! The bulk get partitions APIs behavior actually depends on the absence or presence of a pre-event listener. If that is the case, then clients already expect different return behavior (throwing an exception or returning an empty list). So the API contract itself is that it can throw an exception (OR return an empty list). However, we committed HIVE-12064 which made it so that there will *always* be a pre-event listener (the TransactionalValidationListener is added in code unconditionally). The unit tests were written around the fact that there is always a pre-event listener. I don't think that is a good assumption to make. Listeners are supposed to be plugins and hence we shouldn't design API contracts around there being a listener. My change will revert back to the original behavior (before HIVE-12064), which is that the list partitions APIs can throw an exception (or return an empty list) if an invalid table/db name is specified. We should also fix the tests so that they don't depend of the existence (or non-existence) of listeners or plugins. Thoughts? > Lazy evaluate the table object in PreReadTableEvent to improve get_partition > performance > > > Key: HIVE-20977 > URL: https://issues.apache.org/jira/browse/HIVE-20977 > Project: Hive > Issue Type: Improvement >Reporter: Karthik Manamcheri >Assignee: Karthik Manamcheri >Priority: Minor > Attachments: HIVE-20977.1.patch, HIVE-20977.2.patch, > HIVE-20977.3.patch, HIVE-20977.4.patch > > > The PreReadTableEvent is generated for non-table operations (such as > get_partitions), but only if there is an event listener attached. However, > this is also not necessary if the event listener is not interested in the > read table event. > For example, the TransactionalValidationListener's onEvent looks like this > {code:java} > @Override > public void onEvent(PreEventContext context) throws MetaException, > NoSuchObjectException, > InvalidOperationException { > switch (context.getEventType()) { > case CREATE_TABLE: > handle((PreCreateTableEvent) context); > break; > case ALTER_TABLE: > handle((PreAlterTableEvent) context); > break; > default: > //no validation required.. > } > }{code} > > Note that for read table events it is a no-op. The problem is that the > get_table is evaluated when creating the PreReadTableEvent finally to be just > ignored! > Look at the code below.. {{getMS().getTable(..)}} is evaluated irrespective > of if the listener uses it or not. > {code:java} > private void fireReadTablePreEvent(String catName, String dbName, String > tblName) > throws MetaException, NoSuchObjectException { > if(preListeners.size() > 0) { > // do this only if there is a pre event listener registered (avoid > unnecessary > // metastore api call) > Table t = getMS().getTable(catName, dbName, tblName); > if (t == null) { > throw new NoSuchObjectException(TableName.getQualified(catName, dbName, > tblName) > + " table not found"); > } > firePreEvent(new PreReadTableEvent(t, this)); > } > } > {code} > This can be improved by using a {{Supplier}} and lazily evaluating the table > when needed (once when the first time it is called, memorized after that). > *Motivation* > Whenever a partition call occurs (get_partition, etc.), we fire the > PreReadTableEvent. This affects performance since it fetches the table even > if it is not being used. This change will improve performance on the > get_partition calls. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (HIVE-20977) Lazy evaluate the table object in PreReadTableEvent to improve get_partition performance
[ https://issues.apache.org/jira/browse/HIVE-20977?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16752524#comment-16752524 ] Peter Vary commented on HIVE-20977: --- +1 > Lazy evaluate the table object in PreReadTableEvent to improve get_partition > performance > > > Key: HIVE-20977 > URL: https://issues.apache.org/jira/browse/HIVE-20977 > Project: Hive > Issue Type: Improvement >Reporter: Karthik Manamcheri >Assignee: Karthik Manamcheri >Priority: Minor > Attachments: HIVE-20977.1.patch, HIVE-20977.2.patch, > HIVE-20977.3.patch, HIVE-20977.4.patch > > > The PreReadTableEvent is generated for non-table operations (such as > get_partitions), but only if there is an event listener attached. However, > this is also not necessary if the event listener is not interested in the > read table event. > For example, the TransactionalValidationListener's onEvent looks like this > {code:java} > @Override > public void onEvent(PreEventContext context) throws MetaException, > NoSuchObjectException, > InvalidOperationException { > switch (context.getEventType()) { > case CREATE_TABLE: > handle((PreCreateTableEvent) context); > break; > case ALTER_TABLE: > handle((PreAlterTableEvent) context); > break; > default: > //no validation required.. > } > }{code} > > Note that for read table events it is a no-op. The problem is that the > get_table is evaluated when creating the PreReadTableEvent finally to be just > ignored! > Look at the code below.. {{getMS().getTable(..)}} is evaluated irrespective > of if the listener uses it or not. > {code:java} > private void fireReadTablePreEvent(String catName, String dbName, String > tblName) > throws MetaException, NoSuchObjectException { > if(preListeners.size() > 0) { > // do this only if there is a pre event listener registered (avoid > unnecessary > // metastore api call) > Table t = getMS().getTable(catName, dbName, tblName); > if (t == null) { > throw new NoSuchObjectException(TableName.getQualified(catName, dbName, > tblName) > + " table not found"); > } > firePreEvent(new PreReadTableEvent(t, this)); > } > } > {code} > This can be improved by using a {{Supplier}} and lazily evaluating the table > when needed (once when the first time it is called, memorized after that). > *Motivation* > Whenever a partition call occurs (get_partition, etc.), we fire the > PreReadTableEvent. This affects performance since it fetches the table even > if it is not being used. This change will improve performance on the > get_partition calls. > -- This message was sent by Atlassian JIRA (v7.6.3#76005)