zhoujinsong commented on code in PR #3113:
URL: https://github.com/apache/amoro/pull/3113#discussion_r1740423175
##########
amoro-ams/amoro-ams-server/src/main/java/org/apache/amoro/server/optimizing/OptimizingQueue.java:
##########
@@ -468,6 +468,7 @@ public void acceptResult(TaskRuntime taskRuntime) {
}
} catch (Exception e) {
LOG.error("accept result error:", e);
+ throw e;
Review Comment:
I agree that we should throw the exception rather than just printing logs.
But we may just remove the catch blocks because we will print error logs in
the service proxy.
##########
amoro-ams/amoro-ams-server/src/main/java/org/apache/amoro/server/table/TableRuntime.java:
##########
@@ -409,7 +409,7 @@ private boolean updateConfigInternal(Map<String, String>
properties) {
}
public void addTaskQuota(TaskRuntime.TaskQuota taskQuota) {
- doAs(OptimizingMapper.class, mapper -> mapper.insertTaskQuota(taskQuota));
+ doAsAlone(OptimizingMapper.class, mapper ->
mapper.insertTaskQuota(taskQuota));
Review Comment:
It seems we started another transaction to update the quota.
Another method to solve this issue is using the same transaction and
ignoring the persistent error. We can add a method like `doAsIgnoreError` in
PersistentBase to get that.
What do you think is the better method? @rfyu
--
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]
