[#1686] feat(netty): Support pending tasks number metrics for Netty EventLoopGroup #1687
Conversation
…etty EventLoopGroup
…etty EventLoopGroup
| @@ -89,6 +89,13 @@ public class ShuffleServerConf extends RssBaseConf { | |||
| .defaultValue(10 * 1000L) | |||
| .withDescription("Direct memory usage tracker interval to MetricSystem (ms)"); | |||
|
|
|||
| public static final ConfigOption<Long> SERVER_NETTY_PENDING_TASKS_NUM_TRACKER_INTERVAL = | |||
| ConfigOptions.key("rss.server.netty.pending.tasks.num.updateMetricsIntervalMs") | |||
There was a problem hiding this comment.
Could you use a more brief name?
There was a problem hiding this comment.
Maybe NETTY_PENDING_TASKS_NUM_TRACKER_INTERVAL?
There was a problem hiding this comment.
Actually I means rss.server.netty.pending.tasks.num.updateMetricsIntervalMs
There was a problem hiding this comment.
I followed the naming of rss.server.netty.directMemoryTracker.memoryUsage.updateMetricsIntervalMs.
Maybe rss.server.netty.pending.tasks.num.polling.interval? If we continue to simplify the naming of this variable, I'm afraid it might become difficult to understand.
There was a problem hiding this comment.
How about rss.server.netty.metrics.pendingTaskNumPollingIntervalMs? Only the last name is camel. The former name segments can use more common word.
Either is fine. Of course it will be nice to have this feature. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1687 +/- ##
============================================
- Coverage 54.86% 54.57% -0.29%
- Complexity 2358 2750 +392
============================================
Files 368 411 +43
Lines 16379 21483 +5104
Branches 1504 2031 +527
============================================
+ Hits 8986 11725 +2739
- Misses 6862 9014 +2152
- Partials 531 744 +213 ☔ View full report in Codecov by Sentry. |
|
Could you add the document for this config option? |
We missed docs for |
Yes. |
|
Added. But only for my new added config. The markdown table is reformatted. |
|
If you want to add other configs in this PR. I'll be OK with that. Or use a new issue to track configs without docs. |
|
Merged to master & branch 0.9. |
What changes were proposed in this pull request?
Support pending tasks number metrics for Netty EventLoopGroup.
Why are the changes needed?
For #1686.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Existing tests.