-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Callback mode supports default rejection policy adjustment #1132
Callback mode supports default rejection policy adjustment #1132
Conversation
…hread pool is full
Codecov Report
@@ Coverage Diff @@
## master #1132 +/- ##
============================================
+ Coverage 70.79% 70.85% +0.06%
+ Complexity 830 828 -2
============================================
Files 405 405
Lines 17088 17100 +12
Branches 2669 2669
============================================
+ Hits 12098 12117 +19
+ Misses 3607 3597 -10
- Partials 1383 1386 +3
Continue to review full report at Codecov.
|
...ing/remoting-bolt/src/main/java/com/alipay/sofa/rpc/message/bolt/AbstractInvokeCallback.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
} | ||
|
||
try { | ||
latch.await(100L * invokeCount, TimeUnit.MILLISECONDS); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait time will be 100 * invokeCount ms, if invokeCount==100 ,this case will run 10s. It is too long
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the worst case time consumption, the actual execution time should be (invokeCount / max_threadNum)*100
or so, latch await will stop when count is 0, I have tried to use this time consumption, in the local execution of about 1s, but because of the CI machine performance is poor, the execution is easy to execute before the completion of the timeout results in test case failure, so I modified the maximum wait time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Motivation:
Fix #1131
Modification:
Describe the idea and modifications you've done.
Result:
Fixes #.
If there is no issue then describe the changes introduced by this PR.