the-other-tim-brown commented on code in PR #12272:
URL: https://github.com/apache/hudi/pull/12272#discussion_r1847555196
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/utils/TransactionUtils.java:
##########
@@ -71,12 +71,12 @@ public static Option<HoodieCommitMetadata>
resolveWriteConflictIfAny(
WriteOperationType operationType =
thisCommitMetadata.map(HoodieCommitMetadata::getOperationType).orElse(null);
if (config.needResolveWriteConflict(operationType)) {
// deal with pendingInstants
- Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation =
getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(),
pendingInstants);
-
- ConflictResolutionStrategy resolutionStrategy =
config.getWriteConflictResolutionStrategy();
if (reloadActiveTimeline) {
table.getMetaClient().reloadActiveTimeline();
}
+ Stream<HoodieInstant> completedInstantsDuringCurrentWriteOperation =
getCompletedInstantsDuringCurrentWriteOperation(table.getMetaClient(),
pendingInstants);
Review Comment:
There is an argument to this method called `reloadActiveTimeline` and it is
true
[here](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/commit/BaseCommitActionExecutor.java#L202-L203)
since the table is created outside of the lock.
It is false
[here](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieClient.java#L239-L240).
This method is called from the preCommit methods in
[BaseHoodieWriteClient](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieWriteClient.java#L378)
and
[BaseHoodieTableServiceClient](https://github.com/apache/hudi/blob/master/hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java#L179).
Both of these are creating the table instance which will create a new meta
client right before calling the method linked in `BaseHoodieWriteClient`.
The point I am making is that there is no purpose for another reload here.
It is just overhead that we can avoid.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
