veghlaci05 commented on code in PR #3712:
URL: https://github.com/apache/hive/pull/3712#discussion_r1015436368
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##########
@@ -127,14 +128,10 @@ private void waitForCompactionToFinish(CompactionResponse
resp) throws HiveExcep
break;
}
- //this could be expensive when there are a lot of compactions....
- //todo: update to search by ID once HIVE-13353 is done
- ShowCompactResponse allCompactions = context.getDb().showCompactions();
+ ShowCompactRequest request = new ShowCompactRequest();
+ request.setId(resp.getId());
+ ShowCompactResponse allCompactions =
context.getDb().showCompactions(request);
for (ShowCompactResponseElement compaction :
allCompactions.getCompacts()) {
Review Comment:
Please remove the for loop. Since you searching by id you should rather
check if you have exactly one result and throw an exception (with the
appropriate error message) if not.
##########
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##########
@@ -127,14 +128,10 @@ private void waitForCompactionToFinish(CompactionResponse
resp) throws HiveExcep
break;
}
- //this could be expensive when there are a lot of compactions....
- //todo: update to search by ID once HIVE-13353 is done
- ShowCompactResponse allCompactions = context.getDb().showCompactions();
+ ShowCompactRequest request = new ShowCompactRequest();
+ request.setId(resp.getId());
+ ShowCompactResponse allCompactions =
context.getDb().showCompactions(request);
Review Comment:
Please rename the variable since it is no longer holding all the compactions.
##########
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/Initiator.java:
##########
@@ -345,23 +344,17 @@ private void recoverFailedCompactions(boolean remoteOnly)
throws MetaException {
HiveConf.ConfVars.HIVE_COMPACTOR_WORKER_TIMEOUT,
TimeUnit.MILLISECONDS));
}
- private boolean foundCurrentOrFailedCompactions(ShowCompactResponse
compactions, CompactionInfo ci) throws MetaException {
- if (compactions.getCompacts() == null) {
+ private boolean foundCurrentOrFailedCompactions(CompactionInfo ci) throws
MetaException {
+
+ ShowCompactRequest request = new ShowCompactRequest();
+ request.setDbname(ci.dbname);
+ request.setTablename(ci.tableName);
+ request.setPartitionname(ci.partName);
+ final ShowCompactResponse currentCompactions =
txnHandler.showCompact(request);
Review Comment:
I'm not sure this will be faster. Now you are filtering by DB, table, and
partition, but on the other hand executing showcompact multiple times which has
both serious DB and network (thrift) overhead. Did you do any measurements to
check if this improves performance or 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
