amoro icon indicating copy to clipboard operation
amoro copied to clipboard

[AMORO-2623] Avoid deadlock in task cancellation

Open link3280 opened this issue 1 year ago • 1 comments

Why are the changes needed?

Close #2623.

Brief change log

Add a timeout mechanism for task cancellation to avoid deadlocks.

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

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

link3280 avatar Mar 17 '24 04:03 link3280

Codecov Report

Attention: Patch coverage is 50.00000% with 24 lines in your changes are missing coverage. Please review.

Project coverage is 34.71%. Comparing base (92730de) to head (868bc01).

Files Patch % Lines
...rctic/server/persistence/StatedPersistentBase.java 20.83% 18 Missing and 1 partial :warning:
...tic/server/exception/ConcurrentStateException.java 0.00% 2 Missing :warning:
.../com/netease/arctic/server/table/TableRuntime.java 75.00% 2 Missing :warning:
.../netease/arctic/server/optimizing/TaskRuntime.java 92.30% 1 Missing :warning:
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2644      +/-   ##
============================================
- Coverage     34.71%   34.71%   -0.01%     
  Complexity     4521     4521              
============================================
  Files           608      609       +1     
  Lines         50980    51008      +28     
  Branches       6686     6686              
============================================
+ Hits          17700    17705       +5     
- Misses        31827    31848      +21     
- Partials       1453     1455       +2     
Flag Coverage Δ
core 33.06% <50.00%> (-0.01%) :arrow_down:
trino 50.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Mar 17 '24 04:03 codecov[bot]

@majin1102 @zhoujinsong Please review the optimized version. The complexity is largely reduced by replacing the external timeout mechanism with the built-in timeout in ReentrantLock.

link3280 avatar Mar 24 '24 04:03 link3280

Close in favor of https://github.com/NetEase/amoro/pull/2684

link3280 avatar Mar 28 '24 02:03 link3280