zhoujinsong commented on code in PR #4230:
URL: https://github.com/apache/amoro/pull/4230#discussion_r3295958845
##########
amoro-ams/src/main/java/org/apache/amoro/server/process/iceberg/IcebergProcessFactory.java:
##########
@@ -275,6 +293,14 @@ private Optional<TableProcess>
triggerAutoCreateTag(TableRuntime tableRuntime) {
return Optional.of(new TagsAutoCreatingProcess(tableRuntime, localEngine));
}
+ private Optional<TableProcess> triggerHiveCommitSync(TableRuntime
tableRuntime) {
+ if (localEngine == null) {
+ return Optional.empty();
+ }
+
+ return Optional.of(new HiveCommitSyncProcess(tableRuntime, localEngine));
Review Comment:
`triggerHiveCommitSync` is missing a table format check before creating the
process.
The `IcebergProcessFactory.formats` list includes `ICEBERG`,
`MIXED_ICEBERG`, and `MIXED_HIVE`, so this trigger will be invoked for all
three formats. Without an early guard, a `HiveCommitSyncProcess` is created and
submitted to the thread pool for every table, only to discover inside `run()`
that the table is not a Hive table and return early — wasted scheduling
overhead on every cycle.
For comparison, `triggerAutoCreateTag` already does this correctly:
```java
if (localEngine == null
|| tableRuntime.getFormat() != TableFormat.ICEBERG
|| ...) {
return Optional.empty();
}
```
Suggested fix — filter at the trigger layer:
```java
private Optional<TableProcess> triggerHiveCommitSync(TableRuntime
tableRuntime) {
if (localEngine == null
|| tableRuntime.getFormat() != TableFormat.MIXED_HIVE) {
return Optional.empty();
}
return Optional.of(new HiveCommitSyncProcess(tableRuntime, localEngine));
}
```
The `TableTypeUtil.isHive()` guard inside `HiveCommitSyncProcess.run()` can
remain as a defensive check, but the trigger layer should be the first gate.
--
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]
