j1wonpark commented on code in PR #4104:
URL: https://github.com/apache/amoro/pull/4104#discussion_r2886745569
##########
amoro-ams/src/main/java/org/apache/amoro/server/DefaultOptimizingService.java:
##########
@@ -384,6 +403,17 @@ public void handleConfigChanged(TableRuntime runtime,
TableConfiguration origina
}
getOptionalQueueByGroup(tableRuntime.getGroupName())
.ifPresent(q -> q.refreshTable(tableRuntime));
+
+ Optional<OptimizingQueue> queue =
getOptionalQueueByGroup(tableRuntime.getGroupName());
+ if (queue.isPresent()) {
+ queue.get().releaseTable(tableRuntime);
+ } else {
+ LOG.warn(
+ "Cannot find resource group: {}, try to release optimizing process
of table {} directly",
+ tableRuntime.getGroupName(),
+ tableRuntime.getTableIdentifier());
+ tableRuntime.completeEmptyProcess();
+ }
Review Comment:
This block is placed outside the group-change condition (`if
(!tableRuntime.getGroupName().equals(originalGroup))`), so it executes on every
config change — not just when the group changes.
Two unintended side effects:
- When the new group exists: releaseTable() is called right after
refreshTable() on the same queue, immediately undoing the refresh. This removes
the table from the scheduler and force-closes any in-flight optimizing process.
- When the group hasn't changed at all (e.g., only the optimizing interval
was updated): the table is still unconditionally released from its current
queue.
Only the case where the group changed to a non-existent group works as
intended.
Suggestion: move this fallback inside the group-change block and remove the
duplicate getOptionalQueueByGroup call.
--
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]
