[AMORO-3110] Avoid optimization status blocking in running#3113
[AMORO-3110] Avoid optimization status blocking in running#3113zhoujinsong merged 3 commits intoapache:masterfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3113 +/- ##
============================================
- Coverage 27.64% 27.18% -0.46%
+ Complexity 2496 2462 -34
============================================
Files 360 363 +3
Lines 37133 37471 +338
Branches 5372 5400 +28
============================================
- Hits 10265 10187 -78
- Misses 25908 26327 +419
+ Partials 960 957 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
zhoujinsong
left a comment
zhoujinsong
left a comment
There was a problem hiding this comment.
@rfyu Thanks for trying to solve this issue.
I left some comments, PTAL.
| } | ||
| } catch (Exception e) { | ||
| LOG.error("accept result error:", e); | ||
| throw e; |
There was a problem hiding this 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.
|
|
||
| public void addTaskQuota(TaskRuntime.TaskQuota taskQuota) { | ||
| doAs(OptimizingMapper.class, mapper -> mapper.insertTaskQuota(taskQuota)); | ||
| doAsAlone(OptimizingMapper.class, mapper -> mapper.insertTaskQuota(taskQuota)); |
There was a problem hiding this 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
There was a problem hiding this comment.
thanks for your suggestion. It is acceptable to handle it using the same transaction. I will make the necessary modifications accordingly.
|
LGTM. However, there are some conflicts that should be fixed. @rfyu |
* [AMORO-3110] Avoid optimization status blocking in running * format * Ignore the exception when it occurs and do not perform a rollback (cherry picked from commit 9877c20) Signed-off-by: zhoujinsong <zhoujinsong0505@163.com>
* [AMORO-3110] Avoid optimization status blocking in running * format * Ignore the exception when it occurs and do not perform a rollback (cherry picked from commit 9877c20)
3 participants
Why are the changes needed?
Close #3110.
Brief change log
How was this patch tested?
Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation