the-other-tim-brown commented on code in PR #9371:
URL: https://github.com/apache/hudi/pull/9371#discussion_r1285294547
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/BaseHoodieTableServiceClient.java:
##########
@@ -244,12 +305,39 @@ public Option<String>
scheduleCompaction(Option<Map<String, String>> extraMetada
* @param metadata All the metadata that gets stored along with
a commit
* @param extraMetadata Extra Metadata to be stored
*/
- public abstract void commitCompaction(String compactionInstantTime,
HoodieCommitMetadata metadata, Option<Map<String, String>> extraMetadata);
+ public void commitCompaction(String compactionInstantTime,
HoodieCommitMetadata metadata, Option<Map<String, String>> extraMetadata) {
+ extraMetadata.ifPresent(m -> m.forEach(metadata::addMetadata));
+ completeCompaction(metadata, createTable(config,
context.getHadoopConf().get()), compactionInstantTime);
+ }
/**
* Commit Compaction and track metrics.
*/
- protected abstract void completeCompaction(HoodieCommitMetadata metadata,
HoodieTable table, String compactionCommitTime);
+ protected void completeCompaction(HoodieCommitMetadata metadata, HoodieTable
table, String compactionCommitTime) {
+ this.context.setJobStatus(this.getClass().getSimpleName(), "Collect
compaction write status and commit compaction: " + config.getTableName());
+ List<HoodieWriteStat> writeStats = metadata.getWriteStats();
+ handleWriteErrors(writeStats, TableServiceType.COMPACT);
+ final HoodieInstant compactionInstant =
HoodieTimeline.getCompactionInflightInstant(compactionCommitTime);
+ try {
+ this.txnManager.beginTransaction(Option.of(compactionInstant),
Option.empty());
+ finalizeWrite(table, compactionCommitTime, writeStats);
+ // commit to data table after committing to metadata table.
+ updateTableMetadata(table, metadata, compactionInstant,
context.emptyHoodieData());
Review Comment:
@nsivabalan, ok can you elaborate on the reason for the difference in the
two methods then? It honestly looks like an example of poor refactoring on the
spark side considering how close they are in implementation.
This is the only difference:
```
checkArgument(table.isTableServiceAction(actionType, instantTime),
String.format("Unsupported action: %s.%s is not table service.", actionType,
instantTime));
context.setJobStatus(this.getClass().getSimpleName(), "Committing to
metadata table: " + config.getTableName());
```
--
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]
