nsivabalan commented on a change in pull request #3590:
URL: https://github.com/apache/hudi/pull/3590#discussion_r721356787
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -241,13 +242,16 @@ void emitCommitMetrics(String instantTime,
HoodieCommitMetadata metadata, String
}
}
+ /**
+ * Any pre-commit actions like conflict resolution or updating metadata
table goes here.
+ * @param instantTime commit instant time.
+ * @param metadata commit metadata for which pre commit is being invoked.
+ */
protected void preCommit(String instantTime, HoodieCommitMetadata metadata) {
- // no-op
- // TODO : Conflict resolution is not supported for Flink & Java engines
- }
-
- protected void syncTableMetadata() {
- // no-op
+ // Create a Hoodie table after startTxn which encapsulated the commits and
files visible.
+ // Important to create this after the lock to ensure latest commits show
up in the timeline without need for reload
Review comment:
I feel adding this comment at the caller does not fit in well. This is
what it looks like.
```
try {
// Precommit is meant for resolving conflicts if any and to update
metadata table if enabled.
// Create a Hoodie table within preCommit after starting the
transaction which encapsulated the commits and files visible
// (instead of using an existing instance of Hoodie table). It is
important to create this after the lock to ensure latest
// commits show up in the timeline without need for reload.
preCommit(instantTime, metadata);
commit(table, commitActionType, instantTime, metadata, stats);
postCommit(table, metadata, instantTime, extraMetadata);
...
```
I feel having this within preCommit just belongs to the place where it is
needed or used.
Let me know what do you think.
--
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]
