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<Boolean> useFileIds, boolean
ignoreEmptyFiles) throws IOException {`
|
`public static AcidDirectory getAcidState(FileSystem fileSystem, Path
candidateDirectory, Configuration conf,
ValidWriteIdList writeIdList, Ref<Boolean> useFileIds,
boolean ignoreEmptyFiles, Map<Path,
HdfsDirSnapshot> 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
