Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak closed pull request #4859: HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… URL: https://github.com/apache/hive/pull/4859 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859: URL: https://github.com/apache/hive/pull/4859#discussion_r1530054468 ## ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java: ## @@ -69,40 +80,48 @@ public AlterTableCompactOperation(DDLOperationContext context, AlterTableCompact compactionRequest.setNumberOfBuckets(desc.getNumberOfBuckets()); } -InitiatorBase initiatorBase = new InitiatorBase(); -initiatorBase.setConf(context.getConf()); Review Comment: Copying the entire configuration is costly both in terms of CPU and memory but given that it was kind of like that before I am fine to keep it as is for this PR. If you find a better way to do it feel free to raise another PR and ping me for review. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1525934250
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -69,40 +80,48 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
compactionRequest.setNumberOfBuckets(desc.getNumberOfBuckets());
}
-InitiatorBase initiatorBase = new InitiatorBase();
-initiatorBase.setConf(context.getConf());
Review Comment:
1. Changing conf at `resolveValidWriteIds` will disturb initiator flow,
Initiator will set VALID_TXNS_KEY in its config object every time it runs
before calling resolveValidWriteIds. And VALID_TXNS_KEY is not only used in
`resolveValidWriteIds` but also some other methods like
`AcidUtils.getAcidState(sd, writeIds, conf);`
Secondly, `resolveValidWriteIds` is getting executed for every partition, so
it is better to keep common code into `initiateCompactionForPartition` itself
to execute code only once.
2. We cannot add a new property in config object and pass it since other
methods like
`AcidUtils.getAcidState(sd, writeIds, conf);`
|
`return getAcidState(fs, location, conf, writeIds, Ref.from(false), false);`
|
`public static AcidDirectory getAcidState(FileSystem fileSystem, Path
candidateDirectory, Configuration conf,
ValidWriteIdList writeIdList, Ref useFileIds, boolean
ignoreEmptyFiles) throws IOException {`
|
`public static AcidDirectory getAcidState(FileSystem fileSystem, Path
candidateDirectory, Configuration conf,
ValidWriteIdList writeIdList, Ref useFileIds,
boolean ignoreEmptyFiles, Map dirSnapshots) throws IOException {`
`ValidTxnList validTxnList = getValidTxnList(conf);`
expects VALID_TXNS_KEY in the config, so adding another param will need
change here as well
3. So choosing the option to clone config and set VALID_TXNS_KEY in it and
pass it on to the rest of the methods. The new object is created only once per
alter table compact command irrespective of the number of partitions and will
not disturb initiator flow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1998982776 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [19 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4859=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1998540502 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [19 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4859=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1997228646 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [19 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4859=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1996792509 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [19 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4859=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1995833222 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [19 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4859=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859: URL: https://github.com/apache/hive/pull/4859#discussion_r1522923087 ## ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java: ## @@ -69,40 +80,48 @@ public AlterTableCompactOperation(DDLOperationContext context, AlterTableCompact compactionRequest.setNumberOfBuckets(desc.getNumberOfBuckets()); } -InitiatorBase initiatorBase = new InitiatorBase(); -initiatorBase.setConf(context.getConf()); Review Comment: Utility methods should not implicitly change the arguments especially things like configurations cause this makes them harder to use and can easily lead to bugs. I believe we can modify the `CompactorUtil.initiateCompactionForPartition` method to not modify the input configuration by creating the `ValidTxnList` when needed (i.e., inside the `resolveValidWriteIds` method) unless there is a specific reason that we need this to be done at the very beginning. Assuming that it is really necessary to do the initialization of `ValidTxnList` here then we could just propagate this object as it is to subsequent methods by adding an additional param. A final option would be to make a copy of the whole config object right before modifying it but this would be costly and wasteful in terms of resources. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1522892557
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
+
+ public static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map
tblProperties, long baseSize, long deltaSize, HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio >
").append(HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD.varname)
+.append(": ");
+msg.append(bigEnough);
+msg.append(".");
+if (!initiateMajor) {
+ msg.append("not");
+}
+msg.append(" initiating major compaction.");
+LOG.debug(msg.toString());
+ }
+ if (initiateMajor)
+return CompactionType.MAJOR;
+}
+
+String deltaNumProp =
+tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD);
+int deltaNumThreshold = deltaNumProp == null ? HiveConf.getIntVar(conf,
+HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD) :
Integer.parseInt(deltaNumProp);
+boolean enough = deltas.size() > deltaNumThreshold;
+if (!enough) {
+ LOG.debug("Not enough deltas to initiate compaction for table=" +
ci.tableName + "partition=" + ci.partName
+ + ". Found: " + deltas.size() + " deltas, threshold is " +
deltaNumThreshold);
+ return null;
+}
+// If there's no base file, do a major compaction
+LOG.debug("Found " + deltas.size() + " delta files, and " + (noBase ? "no"
: "has") + " base," + "requesting "
++ (noBase ? "major" : "minor") + " compaction");
+
+return noBase || !CompactorUtil.isMinorCompactionSupported(conf,
tblProperties, dir) ? CompactionType.MAJOR : CompactionType.MINOR;
+ }
+
+ public static long getBaseSize(AcidDirectory dir) throws IOException {
+long baseSize = 0;
+if (dir.getBase() != null) {
+ baseSize = getDirSize(dir.getFs(), dir.getBase());
+} else {
+ for (HadoopShims.HdfsFileStatusWithId origStat : dir.getOriginalFiles())
{
+baseSize
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1521799103
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
+
+ public static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map
tblProperties, long baseSize, long deltaSize, HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio >
").append(HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD.varname)
+.append(": ");
+msg.append(bigEnough);
+msg.append(".");
+if (!initiateMajor) {
+ msg.append("not");
+}
+msg.append(" initiating major compaction.");
+LOG.debug(msg.toString());
+ }
+ if (initiateMajor)
+return CompactionType.MAJOR;
+}
+
+String deltaNumProp =
+tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD);
+int deltaNumThreshold = deltaNumProp == null ? HiveConf.getIntVar(conf,
+HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD) :
Integer.parseInt(deltaNumProp);
+boolean enough = deltas.size() > deltaNumThreshold;
+if (!enough) {
+ LOG.debug("Not enough deltas to initiate compaction for table=" +
ci.tableName + "partition=" + ci.partName
+ + ". Found: " + deltas.size() + " deltas, threshold is " +
deltaNumThreshold);
+ return null;
+}
+// If there's no base file, do a major compaction
+LOG.debug("Found " + deltas.size() + " delta files, and " + (noBase ? "no"
: "has") + " base," + "requesting "
++ (noBase ? "major" : "minor") + " compaction");
+
+return noBase || !CompactorUtil.isMinorCompactionSupported(conf,
tblProperties, dir) ? CompactionType.MAJOR : CompactionType.MINOR;
+ }
+
+ public static long getBaseSize(AcidDirectory dir) throws IOException {
+long baseSize = 0;
+if (dir.getBase() != null) {
+ baseSize = getDirSize(dir.getFs(), dir.getBase());
+} else {
+ for (HadoopShims.HdfsFileStatusWithId origStat : dir.getOriginalFiles())
{
+baseSize
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1519784221
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
+
+ public static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map
tblProperties, long baseSize, long deltaSize, HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio >
").append(HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD.varname)
+.append(": ");
+msg.append(bigEnough);
+msg.append(".");
+if (!initiateMajor) {
+ msg.append("not");
+}
+msg.append(" initiating major compaction.");
+LOG.debug(msg.toString());
+ }
+ if (initiateMajor)
+return CompactionType.MAJOR;
+}
+
+String deltaNumProp =
+tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD);
+int deltaNumThreshold = deltaNumProp == null ? HiveConf.getIntVar(conf,
+HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD) :
Integer.parseInt(deltaNumProp);
+boolean enough = deltas.size() > deltaNumThreshold;
+if (!enough) {
+ LOG.debug("Not enough deltas to initiate compaction for table=" +
ci.tableName + "partition=" + ci.partName
+ + ". Found: " + deltas.size() + " deltas, threshold is " +
deltaNumThreshold);
+ return null;
+}
+// If there's no base file, do a major compaction
+LOG.debug("Found " + deltas.size() + " delta files, and " + (noBase ? "no"
: "has") + " base," + "requesting "
++ (noBase ? "major" : "minor") + " compaction");
+
+return noBase || !CompactorUtil.isMinorCompactionSupported(conf,
tblProperties, dir) ? CompactionType.MAJOR : CompactionType.MINOR;
+ }
+
+ public static long getBaseSize(AcidDirectory dir) throws IOException {
+long baseSize = 0;
+if (dir.getBase() != null) {
+ baseSize = getDirSize(dir.getFs(), dir.getBase());
+} else {
+ for (HadoopShims.HdfsFileStatusWithId origStat : dir.getOriginalFiles())
{
+baseSize
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1976272994 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [20 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true)  [0 Accepted issues](https://sonarcloud.io/component_measures?id=apache_hive=4859=new_accepted_issues=list) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1510902113
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -32,23 +38,44 @@
import org.apache.hadoop.hive.metastore.api.Partition;
import org.apache.hadoop.hive.metastore.api.StorageDescriptor;
import org.apache.hadoop.hive.metastore.api.Table;
+import org.apache.hadoop.hive.metastore.api.CompactionResponse;
+import org.apache.hadoop.hive.metastore.api.CompactionRequest;
+import org.apache.hadoop.hive.metastore.api.CompactionType;
+import org.apache.hadoop.hive.metastore.api.GetValidWriteIdsRequest;
+import org.apache.hadoop.hive.metastore.api.NoSuchTxnException;
Review Comment:
changed the order of imports
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -35,27 +37,37 @@
import org.apache.hadoop.hive.ql.metadata.HiveException;
import org.apache.hadoop.hive.ql.metadata.Partition;
import org.apache.hadoop.hive.ql.metadata.Table;
-import org.apache.hadoop.hive.ql.txn.compactor.InitiatorBase;
+import org.apache.hadoop.hive.ql.txn.compactor.CompactorUtil;
-import java.util.*;
-import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.List;
+import java.util.ArrayList;
+import java.util.Map;
+import java.util.LinkedHashMap;
+import java.util.Optional;
import static
org.apache.hadoop.hive.ql.io.AcidUtils.compactionTypeStr2ThriftType;
/**
* Operation process of compacting a table.
*/
public class AlterTableCompactOperation extends
DDLOperation {
+
public AlterTableCompactOperation(DDLOperationContext context,
AlterTableCompactDesc desc) {
super(context, desc);
}
@Override public int execute() throws Exception {
+TxnStore txnHandler;
Table table = context.getDb().getTable(desc.getTableName());
if (!AcidUtils.isTransactionalTable(table) &&
!AcidUtils.isNonNativeAcidTable(table)) {
throw new HiveException(ErrorMsg.NONACID_COMPACTION_NOT_SUPPORTED,
table.getDbName(), table.getTableName());
}
+Map partitionMap =
+convertPartitionsFromThriftToDB(getPartitions(table, desc, context));
+
+txnHandler = TxnUtils.getTxnStore(context.getConf());
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859: URL: https://github.com/apache/hive/pull/4859#discussion_r1510901797 ## ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java: ## @@ -32,23 +38,44 @@ import org.apache.hadoop.hive.metastore.api.Partition; import org.apache.hadoop.hive.metastore.api.StorageDescriptor; import org.apache.hadoop.hive.metastore.api.Table; +import org.apache.hadoop.hive.metastore.api.CompactionResponse; +import org.apache.hadoop.hive.metastore.api.CompactionRequest; +import org.apache.hadoop.hive.metastore.api.CompactionType; +import org.apache.hadoop.hive.metastore.api.GetValidWriteIdsRequest; +import org.apache.hadoop.hive.metastore.api.NoSuchTxnException; + + Review Comment: formatter did not do it automatically. seems like my mistake -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1510901271
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,230 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ private static CompactionResponse requestCompaction(CompactionInfo ci,
String runAs, String hostname,
+ String runtimeVersion, TxnStore txnHandler) throws MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+ compactionRequest.setInitiatorId(hostname + "-" +
Thread.currentThread().getId());
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ private static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map tblProperties, long baseSize, long deltaSize,
HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio >
").append(HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD.varname)
+.append(": ");
+msg.append(bigEnough);
+msg.append(".");
+if (!initiateMajor) {
+ msg.append("not");
+}
+msg.append(" initiating major compaction.");
+LOG.debug(msg.toString());
+ }
+ if (initiateMajor)
+return CompactionType.MAJOR;
+}
+
+String deltaNumProp =
+tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD);
+int deltaNumThreshold = deltaNumProp == null ? HiveConf.getIntVar(conf,
+HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD) :
Integer.parseInt(deltaNumProp);
+boolean enough = deltas.size() > deltaNumThreshold;
+if (!enough) {
+ LOG.debug(
+ "Not enough deltas to initiate compaction for table=" + ci.tableName
+ "partition=" + ci.partName
+ + ". Found: " + deltas.size() + " deltas, threshold is " +
deltaNumThreshold);
+ return null;
+}
+// If there's no base file, do a major compaction
+LOG.debug("Found " + deltas.size() + " delta files, and " + (noBase ? "no"
: "has") + " base," + "requesting "
++ (noBase ? "major" : "minor") + " compaction");
+
+return noBase || !isMinorCompactionSupported(conf, tblProperties,
+dir) ? CompactionType.MAJOR : CompactionType.MINOR;
+ }
+
+ private static long getBaseSize(AcidDirectory dir) throws IOException {
+long baseSize = 0;
+if (dir.getBase() != null) {
+ baseSize = getDirSize(dir.getFs(), dir.getBase());
+} else {
+ for (HadoopShims.HdfsFileStatusWithId origStat : dir.getOriginalFiles())
{
+baseSize += origStat.getFileStatus().getLen();
+ }
+}
+return baseSize;
+ }
+
+ private static long getDirSize(FileSystem fs, AcidUtils.ParsedDirectory dir)
throws IOException {
+return dir.getFiles(fs,
Ref.from(false)).stream().map(HadoopShims.HdfsFileStatusWithId::getFileStatus)
+.mapToLong(FileStatus::getLen).sum();
+ }
+
+ private static CompactionType checkForCompaction(final CompactionInfo ci,
final ValidWriteIdList writeIds,
+ final StorageDescriptor sd, final Map
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1510900808
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
+
+ public static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map
tblProperties, long baseSize, long deltaSize, HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio >
").append(HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD.varname)
+.append(": ");
+msg.append(bigEnough);
+msg.append(".");
+if (!initiateMajor) {
+ msg.append("not");
+}
+msg.append(" initiating major compaction.");
+LOG.debug(msg.toString());
+ }
+ if (initiateMajor)
+return CompactionType.MAJOR;
+}
+
+String deltaNumProp =
+tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD);
+int deltaNumThreshold = deltaNumProp == null ? HiveConf.getIntVar(conf,
+HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD) :
Integer.parseInt(deltaNumProp);
+boolean enough = deltas.size() > deltaNumThreshold;
+if (!enough) {
+ LOG.debug("Not enough deltas to initiate compaction for table=" +
ci.tableName + "partition=" + ci.partName
+ + ". Found: " + deltas.size() + " deltas, threshold is " +
deltaNumThreshold);
+ return null;
+}
+// If there's no base file, do a major compaction
+LOG.debug("Found " + deltas.size() + " delta files, and " + (noBase ? "no"
: "has") + " base," + "requesting "
++ (noBase ? "major" : "minor") + " compaction");
+
+return noBase || !CompactorUtil.isMinorCompactionSupported(conf,
tblProperties, dir) ? CompactionType.MAJOR : CompactionType.MINOR;
+ }
+
+ public static long getBaseSize(AcidDirectory dir) throws IOException {
+long baseSize = 0;
+if (dir.getBase() != null) {
+ baseSize = getDirSize(dir.getFs(), dir.getBase());
+} else {
+ for (HadoopShims.HdfsFileStatusWithId origStat : dir.getOriginalFiles())
{
+baseSize
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1505717881
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,230 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ private static CompactionResponse requestCompaction(CompactionInfo ci,
String runAs, String hostname,
+ String runtimeVersion, TxnStore txnHandler) throws MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+ compactionRequest.setInitiatorId(hostname + "-" +
Thread.currentThread().getId());
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ private static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map tblProperties, long baseSize, long deltaSize,
HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio >
").append(HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD.varname)
+.append(": ");
+msg.append(bigEnough);
+msg.append(".");
+if (!initiateMajor) {
+ msg.append("not");
+}
+msg.append(" initiating major compaction.");
+LOG.debug(msg.toString());
+ }
+ if (initiateMajor)
+return CompactionType.MAJOR;
+}
+
+String deltaNumProp =
+tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD);
+int deltaNumThreshold = deltaNumProp == null ? HiveConf.getIntVar(conf,
+HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD) :
Integer.parseInt(deltaNumProp);
+boolean enough = deltas.size() > deltaNumThreshold;
+if (!enough) {
+ LOG.debug(
+ "Not enough deltas to initiate compaction for table=" + ci.tableName
+ "partition=" + ci.partName
+ + ". Found: " + deltas.size() + " deltas, threshold is " +
deltaNumThreshold);
+ return null;
+}
+// If there's no base file, do a major compaction
+LOG.debug("Found " + deltas.size() + " delta files, and " + (noBase ? "no"
: "has") + " base," + "requesting "
++ (noBase ? "major" : "minor") + " compaction");
+
+return noBase || !isMinorCompactionSupported(conf, tblProperties,
+dir) ? CompactionType.MAJOR : CompactionType.MINOR;
+ }
+
+ private static long getBaseSize(AcidDirectory dir) throws IOException {
+long baseSize = 0;
+if (dir.getBase() != null) {
+ baseSize = getDirSize(dir.getFs(), dir.getBase());
+} else {
+ for (HadoopShims.HdfsFileStatusWithId origStat : dir.getOriginalFiles())
{
+baseSize += origStat.getFileStatus().getLen();
+ }
+}
+return baseSize;
+ }
+
+ private static long getDirSize(FileSystem fs, AcidUtils.ParsedDirectory dir)
throws IOException {
+return dir.getFiles(fs,
Ref.from(false)).stream().map(HadoopShims.HdfsFileStatusWithId::getFileStatus)
+.mapToLong(FileStatus::getLen).sum();
+ }
+
+ private static CompactionType checkForCompaction(final CompactionInfo ci,
final ValidWriteIdList writeIds,
+ final StorageDescriptor sd, final Map
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1967756685 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [20 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1504586845
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
+
+ public static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map
tblProperties, long baseSize, long deltaSize, HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio >
").append(HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD.varname)
+.append(": ");
+msg.append(bigEnough);
+msg.append(".");
+if (!initiateMajor) {
+ msg.append("not");
+}
+msg.append(" initiating major compaction.");
+LOG.debug(msg.toString());
+ }
+ if (initiateMajor)
+return CompactionType.MAJOR;
+}
+
+String deltaNumProp =
+tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD);
+int deltaNumThreshold = deltaNumProp == null ? HiveConf.getIntVar(conf,
+HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD) :
Integer.parseInt(deltaNumProp);
+boolean enough = deltas.size() > deltaNumThreshold;
+if (!enough) {
+ LOG.debug("Not enough deltas to initiate compaction for table=" +
ci.tableName + "partition=" + ci.partName
+ + ". Found: " + deltas.size() + " deltas, threshold is " +
deltaNumThreshold);
+ return null;
+}
+// If there's no base file, do a major compaction
+LOG.debug("Found " + deltas.size() + " delta files, and " + (noBase ? "no"
: "has") + " base," + "requesting "
++ (noBase ? "major" : "minor") + " compaction");
+
+return noBase || !CompactorUtil.isMinorCompactionSupported(conf,
tblProperties, dir) ? CompactionType.MAJOR : CompactionType.MINOR;
+ }
+
+ public static long getBaseSize(AcidDirectory dir) throws IOException {
+long baseSize = 0;
+if (dir.getBase() != null) {
+ baseSize = getDirSize(dir.getFs(), dir.getBase());
+} else {
+ for (HadoopShims.HdfsFileStatusWithId origStat : dir.getOriginalFiles())
{
+baseSize
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1504578529
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
Review Comment:
Done
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
+
+ public static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map
tblProperties, long baseSize, long deltaSize, HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1504579247
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
+
+ public static CompactionType determineCompactionType(CompactionInfo ci,
AcidDirectory dir,
+ Map
tblProperties, long baseSize, long deltaSize, HiveConf conf) {
+boolean noBase = false;
+List deltas = dir.getCurrentDirectories();
+if (baseSize == 0 && deltaSize > 0) {
+ noBase = true;
+} else {
+ String deltaPctProp =
+ tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD);
+ float deltaPctThreshold = deltaPctProp == null ?
HiveConf.getFloatVar(conf,
+ HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD) :
Float.parseFloat(deltaPctProp);
+ boolean bigEnough = (float) deltaSize / (float) baseSize >
deltaPctThreshold;
+ boolean multiBase = dir.getObsolete().stream().anyMatch(path ->
path.getName().startsWith(AcidUtils.BASE_PREFIX));
+
+ boolean initiateMajor = bigEnough || (deltaSize == 0 && multiBase);
+ if (LOG.isDebugEnabled()) {
+StringBuilder msg = new StringBuilder("delta size: ");
+msg.append(deltaSize);
+msg.append(" base size: ");
+msg.append(baseSize);
+msg.append(" multiBase ");
+msg.append(multiBase);
+msg.append(" deltaSize ");
+msg.append(deltaSize);
+msg.append(" threshold: ");
+msg.append(deltaPctThreshold);
+msg.append(" delta/base ratio >
").append(HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_PCT_THRESHOLD.varname)
+.append(": ");
+msg.append(bigEnough);
+msg.append(".");
+if (!initiateMajor) {
+ msg.append("not");
+}
+msg.append(" initiating major compaction.");
+LOG.debug(msg.toString());
+ }
+ if (initiateMajor)
+return CompactionType.MAJOR;
+}
+
+String deltaNumProp =
+tblProperties.get(COMPACTOR_THRESHOLD_PREFIX +
HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD);
+int deltaNumThreshold = deltaNumProp == null ? HiveConf.getIntVar(conf,
+HiveConf.ConfVars.HIVE_COMPACTOR_DELTA_NUM_THRESHOLD) :
Integer.parseInt(deltaNumProp);
+boolean enough = deltas.size() > deltaNumThreshold;
+if (!enough) {
+ LOG.debug("Not enough deltas to initiate compaction for table=" +
ci.tableName + "partition=" + ci.partName
+ + ". Found: " + deltas.size() + " deltas, threshold is " +
deltaNumThreshold);
+ return null;
+}
+// If there's no base file, do a major compaction
+LOG.debug("Found " + deltas.size() + " delta files, and " + (noBase ? "no"
: "has") + " base," + "requesting "
++ (noBase ? "major" : "minor") + " compaction");
+
+return noBase || !CompactorUtil.isMinorCompactionSupported(conf,
tblProperties, dir) ? CompactionType.MAJOR : CompactionType.MINOR;
+ }
+
+ public static long getBaseSize(AcidDirectory dir) throws IOException {
+long baseSize = 0;
+if (dir.getBase() != null) {
+ baseSize = getDirSize(dir.getFs(), dir.getBase());
+} else {
+ for (HadoopShims.HdfsFileStatusWithId origStat : dir.getOriginalFiles())
{
+baseSize
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1504575802
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
Review Comment:
Done
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
Review Comment:
Applied eclipse formatter in dev-support in intellij and corrected all the
indentation related issues
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1504574173
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -59,6 +86,8 @@
public class CompactorUtil {
private static final Logger LOG =
LoggerFactory.getLogger(CompactorUtil.class);
public static final String COMPACTOR = "compactor";
+ public static final String COMPACTOR_THRESHOLD_PREFIX =
"compactorthreshold.";
Review Comment:
Done, only those methods, that need public access were made public
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1503975289
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/InitiatorBase.java:
##
@@ -50,59 +49,53 @@
import java.io.IOException;
import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
public class InitiatorBase extends MetaStoreCompactorThread {
static final private String COMPACTOR_THRESHOLD_PREFIX =
"compactorthreshold.";
- private List
initiateCompactionForMultiplePartitions(Table table,
- Map partitions, CompactionRequest request) {
-List compactionResponses = new ArrayList<>();
-partitions.entrySet().parallelStream().forEach(entry -> {
- try {
-StorageDescriptor sd = CompactorUtil.resolveStorageDescriptor(table,
entry.getValue());
-String runAs = TxnUtils.findUserToRunAs(sd.getLocation(), table, conf);
-CompactionInfo ci =
-new CompactionInfo(table.getDbName(), table.getTableName(),
entry.getKey(), request.getType());
-ci.initiatorId = request.getInitiatorId();
-ci.orderByClause = request.getOrderByClause();
-ci.initiatorVersion = request.getInitiatorVersion();
-if (request.getNumberOfBuckets() > 0) {
- ci.numberOfBuckets = request.getNumberOfBuckets();
-}
-ci.poolName = request.getPoolName();
-LOG.info(
-"Checking to see if we should compact partition " + entry.getKey()
+ " of table " + table.getDbName() + "."
-+ table.getTableName());
-CollectionUtils.addIgnoreNull(compactionResponses,
-scheduleCompactionIfRequired(ci, table, entry.getValue(), runAs,
false));
- } catch (IOException | InterruptedException | MetaException e) {
-LOG.error(
-"Error occurred while Checking if we should compact partition " +
entry.getKey() + " of table " + table.getDbName() + "."
-+ table.getTableName() + " Exception: " + e.getMessage());
-throw new RuntimeException(e);
- }
-});
-return compactionResponses;
- }
-
- public List initiateCompactionForTable(CompactionRequest
request, Table table, Map partitions) throws Exception {
+ public void initialize() throws Exception {
+super.init(new AtomicBoolean());
Review Comment:
Moving the methods in `CompactorUtil` is good idea many thanks for doing
that. I added some comments about the last refactoring in the respective places.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1503941565
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -59,6 +86,8 @@
public class CompactorUtil {
private static final Logger LOG =
LoggerFactory.getLogger(CompactorUtil.class);
public static final String COMPACTOR = "compactor";
+ public static final String COMPACTOR_THRESHOLD_PREFIX =
"compactorthreshold.";
Review Comment:
This field along with most of the new methods added in this class can be
made `private static` to improve encapsulation.
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
Review Comment:
This line and few others that were added now exceed the 120 characters which
is suggested by Hive coding conventions. Please reformat those accordingly.
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
+ public static CompactionResponse requestCompaction(CompactionInfo ci, String
runAs, String hostname, String runtimeVersion, TxnStore txnHandler) throws
MetaException {
+CompactionRequest compactionRequest = new CompactionRequest(ci.dbname,
ci.tableName, ci.type);
+if (ci.partName != null)
+ compactionRequest.setPartitionname(ci.partName);
+compactionRequest.setRunas(runAs);
+if (StringUtils.isEmpty(ci.initiatorId)) {
+
compactionRequest.setInitiatorId(getInitiatorId(Thread.currentThread().getId(),hostname));
+} else {
+ compactionRequest.setInitiatorId(ci.initiatorId);
+}
+compactionRequest.setInitiatorVersion(runtimeVersion);
+compactionRequest.setPoolName(ci.poolName);
+LOG.info("Requesting compaction: " + compactionRequest);
+CompactionResponse resp = txnHandler.compact(compactionRequest);
+if (resp.isAccepted()) {
+ ci.id = resp.getId();
+}
+return resp;
+ }
+
+ static AcidDirectory getAcidDirectory(StorageDescriptor sd, ValidWriteIdList
writeIds, HiveConf conf) throws IOException {
+Path location = new Path(sd.getLocation());
+FileSystem fs = location.getFileSystem(conf);
+return AcidUtils.getAcidState(fs, location, conf, writeIds,
Ref.from(false), false);
+ }
Review Comment:
This method is better fit for `AcidUtils` than here. Please move it there
and rename it to `AcidUtils.getAcidState` for making the APIs more uniform.
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/CompactorUtil.java:
##
@@ -295,4 +324,232 @@ public static LockRequest createLockRequest(HiveConf
conf, CompactionInfo ci, lo
!conf.getBoolVar(HiveConf.ConfVars.TXN_WRITE_X_LOCK));
return requestBuilder.build();
}
+
+ public static String getInitiatorId(long threadId, String hostName) {
+return hostName + "-" + threadId;
+ }
+
Review Comment:
The method is longer than the actual code and it is only used in a single
place. Please remove the method and inline the code where its used.
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -69,40 +80,48 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
compactionRequest.setNumberOfBuckets(desc.getNumberOfBuckets());
}
-InitiatorBase initiatorBase = new InitiatorBase();
-initiatorBase.setConf(context.getConf());
-initiatorBase.init(new AtomicBoolean());
-
-Map partitionMap =
-convertPartitionsFromThriftToDB(getPartitions(table, desc, context));
-
-if(desc.getPartitionSpec() != null){
- Optional partitionName =
partitionMap.keySet().stream().findFirst();
- partitionName.ifPresent(compactionRequest::setPartitionname);
-}
-List compactionResponses =
-initiatorBase.initiateCompactionForTable(compactionRequest,
table.getTTable(), partitionMap);
-for (CompactionResponse compactionResponse : compactionResponses) {
- if (!compactionResponse.isAccepted()) {
-String message;
-if (compactionResponse.isSetErrormessage()) {
- message =
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1964557288 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [19 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1961739088 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [22 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1500923581
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/InitiatorBase.java:
##
@@ -50,59 +49,53 @@
import java.io.IOException;
import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
public class InitiatorBase extends MetaStoreCompactorThread {
static final private String COMPACTOR_THRESHOLD_PREFIX =
"compactorthreshold.";
- private List
initiateCompactionForMultiplePartitions(Table table,
- Map partitions, CompactionRequest request) {
-List compactionResponses = new ArrayList<>();
-partitions.entrySet().parallelStream().forEach(entry -> {
- try {
-StorageDescriptor sd = CompactorUtil.resolveStorageDescriptor(table,
entry.getValue());
-String runAs = TxnUtils.findUserToRunAs(sd.getLocation(), table, conf);
-CompactionInfo ci =
-new CompactionInfo(table.getDbName(), table.getTableName(),
entry.getKey(), request.getType());
-ci.initiatorId = request.getInitiatorId();
-ci.orderByClause = request.getOrderByClause();
-ci.initiatorVersion = request.getInitiatorVersion();
-if (request.getNumberOfBuckets() > 0) {
- ci.numberOfBuckets = request.getNumberOfBuckets();
-}
-ci.poolName = request.getPoolName();
-LOG.info(
-"Checking to see if we should compact partition " + entry.getKey()
+ " of table " + table.getDbName() + "."
-+ table.getTableName());
-CollectionUtils.addIgnoreNull(compactionResponses,
-scheduleCompactionIfRequired(ci, table, entry.getValue(), runAs,
false));
- } catch (IOException | InterruptedException | MetaException e) {
-LOG.error(
-"Error occurred while Checking if we should compact partition " +
entry.getKey() + " of table " + table.getDbName() + "."
-+ table.getTableName() + " Exception: " + e.getMessage());
-throw new RuntimeException(e);
- }
-});
-return compactionResponses;
- }
-
- public List initiateCompactionForTable(CompactionRequest
request, Table table, Map partitions) throws Exception {
+ public void initialize() throws Exception {
+super.init(new AtomicBoolean());
Review Comment:
@zabetak I have moved all the reused functions into CompactorUtil.java by
making them static. So now initiator as well as manual compaction will re-use
the functions and no inheritance between these two, so no issues w.r.t
Initiator. Removed InitiatorBase as well. Please take a look. Thanks a lot for
your time on this. Sorry that I made you review more number of times
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1497657015
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/InitiatorBase.java:
##
@@ -50,59 +49,53 @@
import java.io.IOException;
import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
public class InitiatorBase extends MetaStoreCompactorThread {
static final private String COMPACTOR_THRESHOLD_PREFIX =
"compactorthreshold.";
- private List
initiateCompactionForMultiplePartitions(Table table,
- Map partitions, CompactionRequest request) {
-List compactionResponses = new ArrayList<>();
-partitions.entrySet().parallelStream().forEach(entry -> {
- try {
-StorageDescriptor sd = CompactorUtil.resolveStorageDescriptor(table,
entry.getValue());
-String runAs = TxnUtils.findUserToRunAs(sd.getLocation(), table, conf);
-CompactionInfo ci =
-new CompactionInfo(table.getDbName(), table.getTableName(),
entry.getKey(), request.getType());
-ci.initiatorId = request.getInitiatorId();
-ci.orderByClause = request.getOrderByClause();
-ci.initiatorVersion = request.getInitiatorVersion();
-if (request.getNumberOfBuckets() > 0) {
- ci.numberOfBuckets = request.getNumberOfBuckets();
-}
-ci.poolName = request.getPoolName();
-LOG.info(
-"Checking to see if we should compact partition " + entry.getKey()
+ " of table " + table.getDbName() + "."
-+ table.getTableName());
-CollectionUtils.addIgnoreNull(compactionResponses,
-scheduleCompactionIfRequired(ci, table, entry.getValue(), runAs,
false));
- } catch (IOException | InterruptedException | MetaException e) {
-LOG.error(
-"Error occurred while Checking if we should compact partition " +
entry.getKey() + " of table " + table.getDbName() + "."
-+ table.getTableName() + " Exception: " + e.getMessage());
-throw new RuntimeException(e);
- }
-});
-return compactionResponses;
- }
-
- public List initiateCompactionForTable(CompactionRequest
request, Table table, Map partitions) throws Exception {
+ public void initialize() throws Exception {
+super.init(new AtomicBoolean());
Review Comment:
The latest patch changes the behavior of `Initiator` forcing the latter to
retrieve and set a `validTxnList` during initialization something that wasn't
done before. I reviewed related code in `Initiator` class and the change
appears safe but in general it is best to avoid such refactorings that
implicitly change the behavior of the code.
@tarak271 Did you double check that there are no undesired effects from this
change in behavior? If yes then we can proceed and merge this 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.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1945415717 @zabetak addressed review comments and build is successful. Please help take a look -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1944099025 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1938524160 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1936345992 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1935521432 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** Issues  [12 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1483940891
##
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java:
##
@@ -648,6 +646,52 @@ public void
secondCompactionShouldBeRefusedBeforeEnqueueing() throws Exception {
Assert.assertEquals("ready for cleaning", compacts.get(0).getState());
}
+ @Test
+ public void secondCompactionShouldBeRefusedBeforeEnqueueingForPartition()
throws Exception {
+conf.setBoolVar(HiveConf.ConfVars.COMPACTOR_CRUD_QUERY_BASED, true);
+
+final String dbName = "default";
+final String tableName = "compaction_test";
+executeStatementOnDriver("drop table if exists " + tableName, driver);
+executeStatementOnDriver("CREATE TABLE " + tableName + "(id string, value
string) partitioned by(pt string) CLUSTERED BY(id) "
++ "INTO 10 BUCKETS STORED AS ORC
TBLPROPERTIES('transactional'='true')", driver);
+executeStatementOnDriver("alter table " + tableName + " add
partition(pt='test')",driver);
+executeStatementOnDriver("INSERT INTO TABLE " + tableName + "
partition(pt='test') values ('1','one'),('2','two'),('3','three'),"
++
"('4','four'),('5','five'),('6','six'),('7','seven'),('8','eight'),('9','nine'),('10','ten'),"
++
"('11','eleven'),('12','twelve'),('13','thirteen'),('14','fourteen'),('15','fifteen'),('16','sixteen'),"
++
"('17','seventeen'),('18','eighteen'),('19','nineteen'),('20','twenty')",
driver);
+
+executeStatementOnDriver("insert into " + tableName + "
partition(pt='test') values ('21', 'value21'),('84', 'value84'),"
++ "('66', 'value66'),('54', 'value54')", driver);
+executeStatementOnDriver(
+"insert into " + tableName + " partition(pt='test') values ('22',
'value22'),('34', 'value34')," + "('35', 'value35')", driver);
+executeStatementOnDriver("insert into " + tableName + "
partition(pt='test') values ('75', 'value75'),('99', 'value99')", driver);
+
+TxnStore txnHandler = TxnUtils.getTxnStore(conf);
+
+//Do a compaction directly and wait for it to finish
+CompactionRequest rqst = new CompactionRequest(dbName, tableName,
CompactionType.MAJOR);
+rqst.setPartitionname("pt=test");
+CompactionResponse resp = txnHandler.compact(rqst);
+runWorker(conf);
+
+//Try to do a second compaction on the same table before the cleaner runs.
+try {
+ driver.run("ALTER TABLE " + tableName + " partition(pt='test') COMPACT
'major'");
Review Comment:
1st test case is for non-partitioned table. The newly created one is for
partitioned table. I have made a new test case to make it different for
partitioned table as issuing compact on a partitioned table is different from
non-partitioned table logic wise
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1483939038
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/InitiatorBase.java:
##
@@ -50,59 +49,53 @@
import java.io.IOException;
import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
public class InitiatorBase extends MetaStoreCompactorThread {
static final private String COMPACTOR_THRESHOLD_PREFIX =
"compactorthreshold.";
- private List
initiateCompactionForMultiplePartitions(Table table,
- Map partitions, CompactionRequest request) {
-List compactionResponses = new ArrayList<>();
-partitions.entrySet().parallelStream().forEach(entry -> {
- try {
-StorageDescriptor sd = CompactorUtil.resolveStorageDescriptor(table,
entry.getValue());
-String runAs = TxnUtils.findUserToRunAs(sd.getLocation(), table, conf);
-CompactionInfo ci =
-new CompactionInfo(table.getDbName(), table.getTableName(),
entry.getKey(), request.getType());
-ci.initiatorId = request.getInitiatorId();
-ci.orderByClause = request.getOrderByClause();
-ci.initiatorVersion = request.getInitiatorVersion();
-if (request.getNumberOfBuckets() > 0) {
- ci.numberOfBuckets = request.getNumberOfBuckets();
-}
-ci.poolName = request.getPoolName();
-LOG.info(
-"Checking to see if we should compact partition " + entry.getKey()
+ " of table " + table.getDbName() + "."
-+ table.getTableName());
-CollectionUtils.addIgnoreNull(compactionResponses,
-scheduleCompactionIfRequired(ci, table, entry.getValue(), runAs,
false));
- } catch (IOException | InterruptedException | MetaException e) {
-LOG.error(
-"Error occurred while Checking if we should compact partition " +
entry.getKey() + " of table " + table.getDbName() + "."
-+ table.getTableName() + " Exception: " + e.getMessage());
-throw new RuntimeException(e);
- }
-});
-return compactionResponses;
- }
-
- public List initiateCompactionForTable(CompactionRequest
request, Table table, Map partitions) throws Exception {
+ public void initialize() throws Exception {
+super.init(new AtomicBoolean());
Review Comment:
Yeah, i have overridden init itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859: URL: https://github.com/apache/hive/pull/4859#discussion_r1483938684 ## ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java: ## @@ -56,6 +59,13 @@ public AlterTableCompactOperation(DDLOperationContext context, AlterTableCompact throw new HiveException(ErrorMsg.NONACID_COMPACTION_NOT_SUPPORTED, table.getDbName(), table.getTableName()); } +Map partitionMap = +convertPartitionsFromThriftToDB(getPartitions(table, desc, context)); + +InitiatorBase initiatorBase = new InitiatorBase(); +initiatorBase.setConf(context.getConf()); +initiatorBase.initialize(); + Review Comment: No difference in logic, just changed for readability -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1482918569
##
itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/txn/compactor/TestCrudCompactorOnTez.java:
##
@@ -648,6 +646,52 @@ public void
secondCompactionShouldBeRefusedBeforeEnqueueing() throws Exception {
Assert.assertEquals("ready for cleaning", compacts.get(0).getState());
}
+ @Test
+ public void secondCompactionShouldBeRefusedBeforeEnqueueingForPartition()
throws Exception {
+conf.setBoolVar(HiveConf.ConfVars.COMPACTOR_CRUD_QUERY_BASED, true);
+
+final String dbName = "default";
+final String tableName = "compaction_test";
+executeStatementOnDriver("drop table if exists " + tableName, driver);
+executeStatementOnDriver("CREATE TABLE " + tableName + "(id string, value
string) partitioned by(pt string) CLUSTERED BY(id) "
++ "INTO 10 BUCKETS STORED AS ORC
TBLPROPERTIES('transactional'='true')", driver);
+executeStatementOnDriver("alter table " + tableName + " add
partition(pt='test')",driver);
+executeStatementOnDriver("INSERT INTO TABLE " + tableName + "
partition(pt='test') values ('1','one'),('2','two'),('3','three'),"
++
"('4','four'),('5','five'),('6','six'),('7','seven'),('8','eight'),('9','nine'),('10','ten'),"
++
"('11','eleven'),('12','twelve'),('13','thirteen'),('14','fourteen'),('15','fifteen'),('16','sixteen'),"
++
"('17','seventeen'),('18','eighteen'),('19','nineteen'),('20','twenty')",
driver);
+
+executeStatementOnDriver("insert into " + tableName + "
partition(pt='test') values ('21', 'value21'),('84', 'value84'),"
++ "('66', 'value66'),('54', 'value54')", driver);
+executeStatementOnDriver(
+"insert into " + tableName + " partition(pt='test') values ('22',
'value22'),('34', 'value34')," + "('35', 'value35')", driver);
+executeStatementOnDriver("insert into " + tableName + "
partition(pt='test') values ('75', 'value75'),('99', 'value99')", driver);
+
+TxnStore txnHandler = TxnUtils.getTxnStore(conf);
+
+//Do a compaction directly and wait for it to finish
+CompactionRequest rqst = new CompactionRequest(dbName, tableName,
CompactionType.MAJOR);
+rqst.setPartitionname("pt=test");
+CompactionResponse resp = txnHandler.compact(rqst);
+runWorker(conf);
+
+//Try to do a second compaction on the same table before the cleaner runs.
+try {
+ driver.run("ALTER TABLE " + tableName + " partition(pt='test') COMPACT
'major'");
Review Comment:
This test is mostly a copy of the previous one but not sure we need all of
this stuff. The main part that we wanted to test is the exception message when
a partition is set in the request. To do that it would suffice to just add this
try-catch block in the previous test or rework this test and to keep only the
necessary things.
##
ql/src/java/org/apache/hadoop/hive/ql/txn/compactor/InitiatorBase.java:
##
@@ -50,59 +49,53 @@
import java.io.IOException;
import java.security.PrivilegedExceptionAction;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
public class InitiatorBase extends MetaStoreCompactorThread {
static final private String COMPACTOR_THRESHOLD_PREFIX =
"compactorthreshold.";
- private List
initiateCompactionForMultiplePartitions(Table table,
- Map partitions, CompactionRequest request) {
-List compactionResponses = new ArrayList<>();
-partitions.entrySet().parallelStream().forEach(entry -> {
- try {
-StorageDescriptor sd = CompactorUtil.resolveStorageDescriptor(table,
entry.getValue());
-String runAs = TxnUtils.findUserToRunAs(sd.getLocation(), table, conf);
-CompactionInfo ci =
-new CompactionInfo(table.getDbName(), table.getTableName(),
entry.getKey(), request.getType());
-ci.initiatorId = request.getInitiatorId();
-ci.orderByClause = request.getOrderByClause();
-ci.initiatorVersion = request.getInitiatorVersion();
-if (request.getNumberOfBuckets() > 0) {
- ci.numberOfBuckets = request.getNumberOfBuckets();
-}
-ci.poolName = request.getPoolName();
-LOG.info(
-"Checking to see if we should compact partition " + entry.getKey()
+ " of table " + table.getDbName() + "."
-+ table.getTableName());
-CollectionUtils.addIgnoreNull(compactionResponses,
-scheduleCompactionIfRequired(ci, table, entry.getValue(), runAs,
false));
- } catch (IOException | InterruptedException | MetaException e) {
-LOG.error(
-"Error occurred while Checking if we should compact partition " +
entry.getKey() + " of table " + table.getDbName() + "."
-+ table.getTableName() + " Exception:
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1931498322 @zabetak resolved merge conflicts. Please help review and let me know if any changes are required -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1930945085 ## [](https://sonarcloud.io/dashboard?id=apache_hive=4859) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [3 New issues](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=true) No data about Coverage No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive=4859) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1921698402 Hey @tarak271 apologies for the delay. I think all the comments are addressed so I have to do a careful review of the refactoring to ensure that nothing broke and we should be good to merge this. I see that there are some merge conflicts in the InitiatorBase so if you can resolve these as well that would be helpful. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1914672940 @zabetak could you please help guide me, on the next steps with this pull request -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1431320684
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -69,40 +79,48 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
compactionRequest.setNumberOfBuckets(desc.getNumberOfBuckets());
}
-InitiatorBase initiatorBase = new InitiatorBase();
-initiatorBase.setConf(context.getConf());
-initiatorBase.init(new AtomicBoolean());
-
-Map partitionMap =
-convertPartitionsFromThriftToDB(getPartitions(table, desc, context));
-
-if(desc.getPartitionSpec() != null){
- Optional partitionName =
partitionMap.keySet().stream().findFirst();
- partitionName.ifPresent(compactionRequest::setPartitionname);
-}
-List compactionResponses =
-initiatorBase.initiateCompactionForTable(compactionRequest,
table.getTTable(), partitionMap);
-for (CompactionResponse compactionResponse : compactionResponses) {
- if (!compactionResponse.isAccepted()) {
-String message;
-if (compactionResponse.isSetErrormessage()) {
- message = compactionResponse.getErrormessage();
- throw new HiveException(ErrorMsg.COMPACTION_REFUSED,
table.getDbName(), table.getTableName(),
- "CompactionId: " + compactionResponse.getId(), message);
-}
-context.getConsole().printInfo(
-"Compaction already enqueued with id " +
compactionResponse.getId() + "; State is "
-+ compactionResponse.getState());
-continue;
+//Will directly initiate compaction if an un-partitioned table/a partition
is specified in the request
+if (desc.getPartitionSpec() != null || !table.isPartitioned()) {
+ if (desc.getPartitionSpec() != null) {
+Optional partitionName =
partitionMap.keySet().stream().findFirst();
+partitionName.ifPresent(compactionRequest::setPartitionname);
}
- context.getConsole().printInfo("Compaction enqueued with id " +
compactionResponse.getId());
- if (desc.isBlocking() && compactionResponse.isAccepted()) {
-waitForCompactionToFinish(compactionResponse, context);
+ CompactionResponse compactionResponse =
initiatorBase.initiateCompactionForTable(compactionRequest);
Review Comment:
initiateCompactionForTable is used for straight forward compaction request
for a non-partitioned table or a specific partition is given
Where as initiateCompactionForPartition is used to check whether the
partition is eligible to get compacted or not, if eligible then only compaction
will get initiated. This the reason why i had to pass Table, Partition objects
to re-use methods from Initiator and decide whether to initiate compaction or
not.
**Background**
Supported case for alter table compact command are
```
1. alter table compact ''; // For Unpartitioned
table
2. alter table partition compact
''; // to compact specific partition of a partitioned table
```
This command will not work for partitioned tables if we don't provide a
partition name in that command.
Command fails stating that the command won't work for partitioned table
This has been enhanced(https://issues.apache.org/jira/browse/HIVE-27598) to
support even partitioned table if user want's to compact all the eligible
partitions in one command instead of executing alter table 1000 times if they
have 1000 partitions.
So the plan is to check each partition and initiate compaction if the
partition is eligible to be compacted, otherwise unnecessarily compaction
requests would get flooded if the partition count is high
`3. alter table compact ''; // will
scan all partitions and initiate compactions for eligible partitions`
For case 2, Since user asked to compact a specific partition, we will
initiate compaction straight away. we don't want to check the eligibility of
the partition(which is existing old behaviour)
Use case could be that the partition needs to be compacted irrespective of
the delta folder count threshold for some reason.
Use case:
```
create table test_acid(id int) partitioned by(dt string);
insert into test_acid values(1,"a");
insert into test_acid values(2,"a");
insert into test_acid values(3,"a");
insert into test_acid values(4,"a");
alter table test_acid partition(dt='a') compact 'major'; // partition get
compacted even the number of delta folders is 4
```
before compaction, the table will contain 4 delta folders and after
compaction the table will contain only single base directory
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For queries about this
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1420474418
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -69,40 +79,48 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
compactionRequest.setNumberOfBuckets(desc.getNumberOfBuckets());
}
-InitiatorBase initiatorBase = new InitiatorBase();
-initiatorBase.setConf(context.getConf());
-initiatorBase.init(new AtomicBoolean());
-
-Map partitionMap =
-convertPartitionsFromThriftToDB(getPartitions(table, desc, context));
-
-if(desc.getPartitionSpec() != null){
- Optional partitionName =
partitionMap.keySet().stream().findFirst();
- partitionName.ifPresent(compactionRequest::setPartitionname);
-}
-List compactionResponses =
-initiatorBase.initiateCompactionForTable(compactionRequest,
table.getTTable(), partitionMap);
-for (CompactionResponse compactionResponse : compactionResponses) {
- if (!compactionResponse.isAccepted()) {
-String message;
-if (compactionResponse.isSetErrormessage()) {
- message = compactionResponse.getErrormessage();
- throw new HiveException(ErrorMsg.COMPACTION_REFUSED,
table.getDbName(), table.getTableName(),
- "CompactionId: " + compactionResponse.getId(), message);
-}
-context.getConsole().printInfo(
-"Compaction already enqueued with id " +
compactionResponse.getId() + "; State is "
-+ compactionResponse.getState());
-continue;
+//Will directly initiate compaction if an un-partitioned table/a partition
is specified in the request
+if (desc.getPartitionSpec() != null || !table.isPartitioned()) {
+ if (desc.getPartitionSpec() != null) {
+Optional partitionName =
partitionMap.keySet().stream().findFirst();
+partitionName.ifPresent(compactionRequest::setPartitionname);
}
- context.getConsole().printInfo("Compaction enqueued with id " +
compactionResponse.getId());
- if (desc.isBlocking() && compactionResponse.isAccepted()) {
-waitForCompactionToFinish(compactionResponse, context);
+ CompactionResponse compactionResponse =
initiatorBase.initiateCompactionForTable(compactionRequest);
Review Comment:
The `CompactionRequest` provides ways to select a specific partition using
its name. The method here `initiateCompactionForTable` seems capable of
launching a compaction request targeting a specific table + partition.
Why do we need to have an additional method
(`initiateCompactionForPartition`) to do more or less the same thing? Why can't
we use `initiateCompactionForTable` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1414038927
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -80,15 +84,15 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
Optional partitionName =
partitionMap.keySet().stream().findFirst();
partitionName.ifPresent(compactionRequest::setPartitionname);
}
-List compactionResponses =
+Map compactionResponses =
initiatorBase.initiateCompactionForTable(compactionRequest,
table.getTTable(), partitionMap);
-for (CompactionResponse compactionResponse : compactionResponses) {
+for (Map.Entry compactionResponseEntry :
compactionResponses.entrySet()) {
Review Comment:
changed logic as per the recommendation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1414038396
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -80,15 +84,15 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
Optional partitionName =
partitionMap.keySet().stream().findFirst();
partitionName.ifPresent(compactionRequest::setPartitionname);
}
-List compactionResponses =
+Map compactionResponses =
initiatorBase.initiateCompactionForTable(compactionRequest,
table.getTTable(), partitionMap);
-for (CompactionResponse compactionResponse : compactionResponses) {
+for (Map.Entry compactionResponseEntry :
compactionResponses.entrySet()) {
+ CompactionResponse compactionResponse = compactionResponseEntry.getKey();
if (!compactionResponse.isAccepted()) {
-String message;
if (compactionResponse.isSetErrormessage()) {
- message = compactionResponse.getErrormessage();
throw new HiveException(ErrorMsg.COMPACTION_REFUSED,
table.getDbName(), table.getTableName(),
- "CompactionId: " + compactionResponse.getId(), message);
+ compactionResponseEntry.getValue() == null ? "" :
+ "(partition=" + compactionResponseEntry.getValue() + ")",
compactionResponse.getErrormessage());
Review Comment:
Added new test case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1838179324 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4859) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4859=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4859=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1837516611 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4859) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4859=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4859=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
zabetak commented on code in PR #4859:
URL: https://github.com/apache/hive/pull/4859#discussion_r1410480468
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -80,15 +84,15 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
Optional partitionName =
partitionMap.keySet().stream().findFirst();
partitionName.ifPresent(compactionRequest::setPartitionname);
}
-List compactionResponses =
+Map compactionResponses =
initiatorBase.initiateCompactionForTable(compactionRequest,
table.getTTable(), partitionMap);
-for (CompactionResponse compactionResponse : compactionResponses) {
+for (Map.Entry compactionResponseEntry :
compactionResponses.entrySet()) {
+ CompactionResponse compactionResponse = compactionResponseEntry.getKey();
if (!compactionResponse.isAccepted()) {
-String message;
if (compactionResponse.isSetErrormessage()) {
- message = compactionResponse.getErrormessage();
throw new HiveException(ErrorMsg.COMPACTION_REFUSED,
table.getDbName(), table.getTableName(),
- "CompactionId: " + compactionResponse.getId(), message);
+ compactionResponseEntry.getValue() == null ? "" :
+ "(partition=" + compactionResponseEntry.getValue() + ")",
compactionResponse.getErrormessage());
Review Comment:
Do we have a test case that verifies that partition is present in the
Exception? If not can you please add one.
##
ql/src/java/org/apache/hadoop/hive/ql/ddl/table/storage/compact/AlterTableCompactOperation.java:
##
@@ -80,15 +84,15 @@ public AlterTableCompactOperation(DDLOperationContext
context, AlterTableCompact
Optional partitionName =
partitionMap.keySet().stream().findFirst();
partitionName.ifPresent(compactionRequest::setPartitionname);
}
-List compactionResponses =
+Map compactionResponses =
initiatorBase.initiateCompactionForTable(compactionRequest,
table.getTTable(), partitionMap);
-for (CompactionResponse compactionResponse : compactionResponses) {
+for (Map.Entry compactionResponseEntry :
compactionResponses.entrySet()) {
Review Comment:
Instead of creating and passing a `Map` around why don't we extract the
partition iteration here instead of having it inside the
`initiateCompactionForTable`? This way we will not need a Map and will avoid
the need for iterating over the partitions twice (once inside the
`initiateCompactionForTable` and once over the respective responses here).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
tarak271 commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1803296752 @zabetak removed @ignore annotation, please help review & merge -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1802552760 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4859) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4859=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4859=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
Re: [PR] HIVE-27848 - TestCrudCompactorOnTez.secondCompactionShouldBeRefusedBe… [hive]
sonarcloud[bot] commented on PR #4859: URL: https://github.com/apache/hive/pull/4859#issuecomment-1798989063 Kudos, SonarCloud Quality Gate passed! [](https://sonarcloud.io/dashboard?id=apache_hive=4859) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=BUG) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=VULNERABILITY) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive=4859=false=SECURITY_HOTSPOT) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [3 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive=4859=false=CODE_SMELL) [](https://sonarcloud.io/component_measures?id=apache_hive=4859=coverage=list) No Coverage information [](https://sonarcloud.io/component_measures?id=apache_hive=4859=duplicated_lines_density=list) No Duplication information  The version of Java (11.0.8) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17. Read more [here](https://docs.sonarcloud.io/appendices/scanner-environment/) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org 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
