Copilot commented on code in PR #6517:
URL: https://github.com/apache/hive/pull/6517#discussion_r3331969312
##########
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/service/AcidCompactionService.java:
##########
@@ -126,57 +126,56 @@ public void cleanupResultDirs(CompactionInfo ci) {
public Boolean compact(Table table, CompactionInfo ci) throws Exception {
- try (CompactionTxn compactionTxn = new CompactionTxn()) {
-
- if (ci.isRebalanceCompaction() && table.getSd().getNumBuckets() > 0) {
- LOG.error("Cannot execute rebalancing compaction on bucketed tables.");
- ci.errorMessage = "Cannot execute rebalancing compaction on bucketed
tables.";
- msc.markRefused(CompactionInfo.compactionInfoToStruct(ci));
- return false;
- }
+ if (ci.isRebalanceCompaction() && table.getSd().getNumBuckets() > 0) {
+ LOG.error("Cannot execute rebalancing compaction on bucketed tables.");
+ ci.errorMessage = "Cannot execute rebalancing compaction on bucketed
tables.";
+ msc.markRefused(CompactionInfo.compactionInfoToStruct(ci));
+ return false;
+ }
- if (!ci.type.equals(CompactionType.REBALANCE) && ci.numberOfBuckets > 0)
{
- if (LOG.isWarnEnabled()) {
- LOG.warn("Only the REBALANCE compaction accepts the number of
buckets clause (CLUSTERED INTO {N} BUCKETS). " +
- "Since the compaction request is {}, it will be ignored.",
ci.type);
- }
+ if (!ci.type.equals(CompactionType.REBALANCE) && ci.numberOfBuckets > 0) {
+ if (LOG.isWarnEnabled()) {
+ LOG.warn("Only the REBALANCE compaction accepts the number of buckets
clause (CLUSTERED INTO {N} BUCKETS). " +
+ "Since the compaction request is {}, it will be ignored.",
ci.type);
}
+ }
- String fullTableName = TxnUtils.getFullTableName(table.getDbName(),
table.getTableName());
+ String fullTableName = TxnUtils.getFullTableName(table.getDbName(),
table.getTableName());
- // Find the partition we will be working with, if there is one.
- Partition p;
- try {
- p = CompactorUtil.resolvePartition(conf, msc, ci.dbname, ci.tableName,
ci.partName,
- CompactorUtil.METADATA_FETCH_MODE.REMOTE);
- if (p == null && ci.partName != null) {
- ci.errorMessage = "Unable to find partition " +
ci.getFullPartitionName() + ", assuming it was dropped and moving on.";
- LOG.warn(ci.errorMessage + " Compaction info: {}", ci);
- msc.markRefused(CompactionInfo.compactionInfoToStruct(ci));
- return false;
- }
- } catch (Exception e) {
- LOG.error("Unexpected error during resolving partition.", e);
- ci.errorMessage = e.getMessage();
- msc.markFailed(CompactionInfo.compactionInfoToStruct(ci));
+ // Find the partition we will be working with, if there is one.
+ Partition p;
+ try {
+ p = CompactorUtil.resolvePartition(conf, msc, ci.dbname, ci.tableName,
ci.partName,
+ CompactorUtil.METADATA_FETCH_MODE.REMOTE);
+ if (p == null && ci.partName != null) {
+ ci.errorMessage = "Unable to find partition " +
ci.getFullPartitionName() + ", assuming it was dropped and moving on.";
+ LOG.warn(ci.errorMessage + " Compaction info: {}", ci);
+ msc.markRefused(CompactionInfo.compactionInfoToStruct(ci));
return false;
}
+ } catch (Exception e) {
+ LOG.error("Unexpected error during resolving partition.", e);
+ ci.errorMessage = e.getMessage();
+ msc.markFailed(CompactionInfo.compactionInfoToStruct(ci));
+ return false;
Review Comment:
The error log for partition resolution failures doesn’t include compaction
context, and `ci.errorMessage = e.getMessage()` can end up null/too generic.
Including `ci` in the log and using `e.toString()` for the stored message makes
failures easier to diagnose from COMPACTION_QUEUE/COMPLETED_COMPACTIONS without
relying on stack traces.
--
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]
