zuston commented on code in PR #1400:
URL: 
https://github.com/apache/incubator-uniffle/pull/1400#discussion_r1442433653


##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -538,6 +538,19 @@ public class ShuffleServerConf extends RssBaseConf {
                   + "network_bandwidth = 10Gbps, buffer size should be ~ 
1.25MB."
                   + "Default is 0, OS will dynamically adjust the buf size.");
 
+  public static final ConfigOption<Integer> 
TOP_N_SHUFFLE_DATA_SIZE_OF_APP_LEVEL =
+      ConfigOptions.key("rss.server.topN.shuffle.data.size.of.app-level")
+          .intType()
+          .defaultValue(10)
+          .withDescription("size of TopN total shuffle data size of app 
level.");
+
+  public static final ConfigOption<Integer> 
TOP_N_SHUFFLE_DATA_SIZE_OF_APP_LEVEL_REFRESH_INTERVAL =
+      
ConfigOptions.key("rss.server.topN.shuffle.data.size.of.app-level.refresh.interval")

Review Comment:
   How about `rss.server.topN.appShuffleDataSize.refresh.interval` ? 



##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java:
##########
@@ -538,6 +538,19 @@ public class ShuffleServerConf extends RssBaseConf {
                   + "network_bandwidth = 10Gbps, buffer size should be ~ 
1.25MB."
                   + "Default is 0, OS will dynamically adjust the buf size.");
 
+  public static final ConfigOption<Integer> 
TOP_N_SHUFFLE_DATA_SIZE_OF_APP_LEVEL =
+      ConfigOptions.key("rss.server.topN.shuffle.data.size.of.app-level")

Review Comment:
   How about `rss.server.topN.appShuffleDataSize.number` ? 



##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -833,4 +853,47 @@ private void triggerFlush() {
       this.shuffleBufferManager.flushIfNecessary();
     }
   }
+
+  public List<Map.Entry<String, ShuffleTaskInfo>> 
calcTopNTotalDataSizeTaskInfo() {

Review Comment:
   How about putting these calcTopN methods into 
`TopNShuffleDataSizeOfAppCalcTask`



##########
server/src/main/java/org/apache/uniffle/server/ShuffleTaskManager.java:
##########
@@ -165,8 +172,21 @@ public ShuffleTaskManager(
             .maximumSize(Integer.MAX_VALUE)
             .build();
 
+    startClearResourceThread();
+    memoryExporter =

Review Comment:
   `memoryExporter` is not accurate. Please rename this.



##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerMetrics.java:
##########
@@ -345,5 +358,33 @@ private static void setUpMetrics() {
     summaryTotalRemoveResourceTime = 
metricsManager.addSummary(TOTAL_REMOVE_RESOURCE_TIME);
     summaryTotalRemoveResourceByShuffleIdsTime =
         metricsManager.addSummary(TOTAL_REMOVE_RESOURCE_BY_SHUFFLE_IDS_TIME);
+
+    gaugeDataSizeTotalUsage =
+        Gauge.build()
+            .name(TOPN_OF_TOTAL_DATA_SIZE_FOR_APP)
+            .help("top N of total shuffle data size for app level")
+            .labelNames("topn_of_total_data_size_for_app")

Review Comment:
   the label name is wrong. If we hope this could be shown to find out the app 
and sorted number, these labels should be taken.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to