-
Notifications
You must be signed in to change notification settings - Fork 168
[#1459] improvement(server): refactor DefaultFlushEventHandler and support event retry into pending queue #1461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…and support event retry into pending queue
|
PTAL @jerqi @leixm @xianjingfeng |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #1461 +/- ##
============================================
+ Coverage 54.19% 55.18% +0.99%
- Complexity 2775 2776 +1
============================================
Files 423 406 -17
Lines 24204 21876 -2328
Branches 2060 2063 +3
============================================
- Hits 13118 12073 -1045
+ Misses 10275 9058 -1217
+ Partials 811 745 -66 ☔ View full report in Codecov by Sentry. |
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java
Show resolved
Hide resolved
server/src/main/java/org/apache/uniffle/server/DefaultFlushEventHandler.java
Show resolved
Hide resolved
server/src/main/java/org/apache/uniffle/server/ShuffleFlushManager.java
Outdated
Show resolved
Hide resolved
xianjingfeng
left a comment
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
|
Thanks for your review @xianjingfeng Merged. |
…nt is dropped (#1643) ### What changes were proposed in this pull request? Print an error log when an event is dropped. ### Why are the changes needed? A follow-up PR for: #1461. This way, it's easier to find error logs in the log, making it convenient for troubleshooting. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Unnecessary.
…nt is dropped (#1643) ### What changes were proposed in this pull request? Print an error log when an event is dropped. ### Why are the changes needed? A follow-up PR for: #1461. This way, it's easier to find error logs in the log, making it convenient for troubleshooting. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Unnecessary.
…and support event retry into pending queue (apache#1461) ### What changes were proposed in this pull request? 1. Refactor DefaultFlushEventHandler to unify the logic of handling event 2. Fix incorrect some metrics 3. Support retry event into pending queue 4. Fix the incorrect inFlushQueueSize 5. Introduce the underlying executor queue metrics ### Why are the changes needed? Fix: apache#1459 apache#1460 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing UTs
What changes were proposed in this pull request?
Why are the changes needed?
Fix: #1459 #1460
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing UTs