vinothchandar commented on a change in pull request #3900:
URL: https://github.com/apache/hudi/pull/3900#discussion_r744197331
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -244,14 +245,15 @@ 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 hoodieInstant hoodie instant of inflight operation.
* @param metadata commit metadata for which pre commit is being invoked.
*/
- protected void preCommit(String instantTime, HoodieCommitMetadata metadata) {
+ protected void preCommit(HoodieInstant hoodieInstant, HoodieCommitMetadata
metadata) {
// Create a Hoodie table after starting the transaction 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
HoodieTable table = createTable(config, hadoopConf);
- table.getMetadataWriter().ifPresent(w ->
((HoodieTableMetadataWriter)w).update(metadata, instantTime));
+ boolean isTableService = table.isTableService(hoodieInstant.getAction());
Review comment:
rename. `isTableServiceAction()`
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/action/BaseActionExecutor.java
##########
@@ -56,8 +56,9 @@ public BaseActionExecutor(HoodieEngineContext context,
HoodieWriteConfig config,
* Writes commits metadata to table metadata.
* @param metadata commit metadata of interest.
*/
- protected final void writeTableMetadata(HoodieCommitMetadata metadata) {
- table.getMetadataWriter().ifPresent(w -> w.update(metadata, instantTime));
+ protected final void writeTableMetadata(HoodieCommitMetadata metadata,
String actionType) {
+ boolean isTableService = table.isTableService(actionType);
Review comment:
fix name
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/metadata/HoodieBackedTableMetadataWriter.java
##########
@@ -523,23 +523,24 @@ private void initializeFileGroups(HoodieTableMetaClient
dataMetaClient, Metadata
* @param instantTime instant time of interest.
* @param convertMetadataFunction converter function to convert the
respective metadata to List of HoodieRecords to be written to metadata table.
* @param <T> type of commit metadata.
+ * @param canTriggerTableService true if table services can be triggered.
false otherwise.
*/
- private <T> void processAndCommit(String instantTime,
ConvertMetadataFunction convertMetadataFunction) {
+ private <T> void processAndCommit(String instantTime,
ConvertMetadataFunction convertMetadataFunction, boolean
canTriggerTableService) {
if (enabled && metadata != null) {
List<HoodieRecord> records = convertMetadataFunction.convertMetadata();
- commit(records, MetadataPartitionType.FILES.partitionPath(),
instantTime);
+ commit(records, MetadataPartitionType.FILES.partitionPath(),
instantTime, canTriggerTableService);
}
}
/**
* Update from {@code HoodieCommitMetadata}.
- *
* @param commitMetadata {@code HoodieCommitMetadata}
* @param instantTime Timestamp at which the commit was performed
+ * @param isTableService
Review comment:
javadoc
##########
File path:
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/client/SparkRDDWriteClient.java
##########
@@ -474,13 +475,14 @@ private void completeTableService(TableServiceType
tableServiceType, HoodieCommi
}
@Override
- protected void preCommit(String instantTime, HoodieCommitMetadata metadata) {
+ protected void preCommit(HoodieInstant hoodieInstant, HoodieCommitMetadata
metadata) {
// 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
HoodieTable table = createTable(config, hadoopConf);
TransactionUtils.resolveWriteConflictIfAny(table,
this.txnManager.getCurrentTransactionOwner(),
Option.of(metadata), config,
txnManager.getLastCompletedTransactionOwner());
- table.getMetadataWriter().ifPresent(w ->
((HoodieTableMetadataWriter)w).update(metadata, instantTime));
+ boolean isTableService = table.isTableService(hoodieInstant.getAction());
Review comment:
rename do you need this variable everywhere? just call the method inline?
##########
File path:
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/client/SparkRDDWriteClient.java
##########
@@ -406,9 +406,10 @@ private void writeTableMetadata(HoodieTable<T,
JavaRDD<HoodieRecord<T>>, JavaRDD
HoodieInstant hoodieInstant) {
try {
this.txnManager.beginTransaction(Option.of(hoodieInstant),
Option.empty());
+ boolean isTableService = table.isTableService(hoodieInstant.getAction());
Review comment:
rename
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/table/HoodieTable.java
##########
@@ -728,6 +729,11 @@ public HoodieEngineContext getContext() {
return getMetadataWriter(Option.empty());
}
+ public boolean isTableService(String actionType) {
Review comment:
lets override this at the sub classes? so no need for table type check
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -244,14 +245,15 @@ 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 hoodieInstant hoodie instant of inflight operation.
* @param metadata commit metadata for which pre commit is being invoked.
*/
- protected void preCommit(String instantTime, HoodieCommitMetadata metadata) {
+ protected void preCommit(HoodieInstant hoodieInstant, HoodieCommitMetadata
metadata) {
// Create a Hoodie table after starting the transaction 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
HoodieTable table = createTable(config, hadoopConf);
- table.getMetadataWriter().ifPresent(w ->
((HoodieTableMetadataWriter)w).update(metadata, instantTime));
+ boolean isTableService = table.isTableService(hoodieInstant.getAction());
Review comment:
and also variables
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -244,14 +245,15 @@ 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 hoodieInstant hoodie instant of inflight operation.
Review comment:
drop "hoodie" extraneous context
##########
File path:
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/AbstractHoodieWriteClient.java
##########
@@ -244,14 +245,15 @@ 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 hoodieInstant hoodie instant of inflight operation.
* @param metadata commit metadata for which pre commit is being invoked.
*/
- protected void preCommit(String instantTime, HoodieCommitMetadata metadata) {
+ protected void preCommit(HoodieInstant hoodieInstant, HoodieCommitMetadata
metadata) {
Review comment:
rename: hoodieInstant -> inflightInstant?
##########
File path:
hudi-client/hudi-spark-client/src/main/java/org/apache/hudi/client/SparkRDDWriteClient.java
##########
@@ -474,13 +475,14 @@ private void completeTableService(TableServiceType
tableServiceType, HoodieCommi
}
@Override
- protected void preCommit(String instantTime, HoodieCommitMetadata metadata) {
+ protected void preCommit(HoodieInstant hoodieInstant, HoodieCommitMetadata
metadata) {
Review comment:
rename
--
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]
