You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@uniffle.apache.org by GitBox <gi...@apache.org> on 2022/11/16 02:56:03 UTC
[GitHub] [incubator-uniffle] leixm opened a new pull request, #327: [FEATURE] Support ShuffleServer latency metrics.
leixm opened a new pull request, #327:
URL: https://github.com/apache/incubator-uniffle/pull/327
### What changes were proposed in this pull request?
For https://github.com/apache/incubator-uniffle/issues/309, support ShuffleServer latency metrics.
### Why are the changes needed?
Accurately determine whether the current service load has caused a large delay to the client's read and write.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
UT
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] leixm commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
leixm commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023467688
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -186,6 +186,11 @@ public void sendShuffleData(SendShuffleDataRequest req,
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
long requireBufferId = req.getRequireBufferId();
+ long sendTime = req.getSendTime();
+ if (sendTime > 0) {
+ shuffleServer.getGrpcMetrics().recordSendTime(ShuffleServerGrpcMetrics.SEND_SHUFFLE_DATA_METHOD,
Review Comment:
I don't think the amount of data will cause great fluctuations in latency. For example, 100K costs 1ms, and 1M costs 10ms. This seems like a normal fluctuation, but it may rise to 10s when the server load is high (according to observations in the production environment) , Of course, if we consider the amount of data, we can divide the sending time by a certain amount of data. Do you have any better suggestions?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023486906
##########
proto/src/main/proto/Rss.proto:
##########
@@ -76,6 +76,7 @@ message GetLocalShuffleDataRequest {
int32 partitionNum = 5;
int64 offset = 6;
int32 length = 7;
+ int64 sendTime = 8;
Review Comment:
Could we give a better name?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023487249
##########
proto/src/main/proto/Rss.proto:
##########
@@ -90,6 +91,7 @@ message GetMemoryShuffleDataRequest {
int32 partitionId = 3;
int64 lastBlockId = 4;
int32 readBufferSize = 5;
+ int64 sendTime = 6;
Review Comment:
Time a duration. This should be timestamp.
##########
proto/src/main/proto/Rss.proto:
##########
@@ -90,6 +91,7 @@ message GetMemoryShuffleDataRequest {
int32 partitionId = 3;
int64 lastBlockId = 4;
int32 readBufferSize = 5;
+ int64 sendTime = 6;
Review Comment:
Time is a duration. This should be timestamp.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] leixm commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
leixm commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023468424
##########
common/src/main/java/org/apache/uniffle/common/metrics/GRPCMetrics.java:
##########
@@ -119,4 +138,12 @@ public Gauge getGaugeGrpcOpen() {
public Counter getCounterGrpcTotal() {
return counterGrpcTotal;
}
+
+ public Map<String, Summary> getSendTimeSummaryMap() {
Review Comment:
Its meaning is the time interval from the client sending to the ShuffleServerGrpcService receiving the request.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023554677
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -186,6 +186,16 @@ public void sendShuffleData(SendShuffleDataRequest req,
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
long requireBufferId = req.getRequireBufferId();
+ long sendTime = req.getTimestamp();
+ if (sendTime > 0) {
+ /*
+ * Here we record the transport time, but we don't consider the impact of data size on transport time.
+ * The amount of data will not cause great fluctuations in latency. For example, 100K costs 1ms,
+ * and 1M costs 10ms. This seems like a normal fluctuation, but it may rise to 10s when the server load is high.
+ * */
+ shuffleServer.getGrpcMetrics().recordTransportTime(ShuffleServerGrpcMetrics.SEND_SHUFFLE_DATA_METHOD,
Review Comment:
Whether does the negative number influence our metrics?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023469290
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -186,6 +186,11 @@ public void sendShuffleData(SendShuffleDataRequest req,
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
long requireBufferId = req.getRequireBufferId();
+ long sendTime = req.getSendTime();
+ if (sendTime > 0) {
+ shuffleServer.getGrpcMetrics().recordSendTime(ShuffleServerGrpcMetrics.SEND_SHUFFLE_DATA_METHOD,
Review Comment:
Make sense. Could we add some comments to explain why we don't choose to use the size of data?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023579632
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -186,6 +186,16 @@ public void sendShuffleData(SendShuffleDataRequest req,
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
long requireBufferId = req.getRequireBufferId();
+ long sendTime = req.getTimestamp();
+ if (sendTime > 0) {
+ /*
+ * Here we record the transport time, but we don't consider the impact of data size on transport time.
+ * The amount of data will not cause great fluctuations in latency. For example, 100K costs 1ms,
+ * and 1M costs 10ms. This seems like a normal fluctuation, but it may rise to 10s when the server load is high.
+ * */
+ shuffleServer.getGrpcMetrics().recordTransportTime(ShuffleServerGrpcMetrics.SEND_SHUFFLE_DATA_METHOD,
Review Comment:
> Perhaps we can add a comment stating that the time of the client machine and the server machine should be in sync. For the case of less than 0, we do a judgment filter, but time out of sync will also affect metrics
OK. actually we should have a document to tell users that how to use the metrics and what to notice. But we don't have such documents, so we can only add some comments.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023554099
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -186,6 +186,16 @@ public void sendShuffleData(SendShuffleDataRequest req,
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
long requireBufferId = req.getRequireBufferId();
+ long sendTime = req.getTimestamp();
+ if (sendTime > 0) {
+ /*
+ * Here we record the transport time, but we don't consider the impact of data size on transport time.
+ * The amount of data will not cause great fluctuations in latency. For example, 100K costs 1ms,
+ * and 1M costs 10ms. This seems like a normal fluctuation, but it may rise to 10s when the server load is high.
+ * */
+ shuffleServer.getGrpcMetrics().recordTransportTime(ShuffleServerGrpcMetrics.SEND_SHUFFLE_DATA_METHOD,
Review Comment:
`System.currentTimeMills() - sendTime` may be less than 0, because they are generated from different machines.
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023465241
##########
common/src/main/java/org/apache/uniffle/common/metrics/GRPCMetrics.java:
##########
@@ -119,4 +138,12 @@ public Gauge getGaugeGrpcOpen() {
public Counter getCounterGrpcTotal() {
return counterGrpcTotal;
}
+
+ public Map<String, Summary> getSendTimeSummaryMap() {
Review Comment:
Could we have a better name?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023465241
##########
common/src/main/java/org/apache/uniffle/common/metrics/GRPCMetrics.java:
##########
@@ -119,4 +138,12 @@ public Gauge getGaugeGrpcOpen() {
public Counter getCounterGrpcTotal() {
return counterGrpcTotal;
}
+
+ public Map<String, Summary> getSendTimeSummaryMap() {
Review Comment:
Could we have a better name? Transport time?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] leixm commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
leixm commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023469062
##########
common/src/main/java/org/apache/uniffle/common/metrics/GRPCMetrics.java:
##########
@@ -119,4 +138,12 @@ public Gauge getGaugeGrpcOpen() {
public Counter getCounterGrpcTotal() {
return counterGrpcTotal;
}
+
+ public Map<String, Summary> getSendTimeSummaryMap() {
Review Comment:
> Could we have a better name? Transport time?
Sounds great!
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] leixm commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
leixm commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023572929
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -186,6 +186,16 @@ public void sendShuffleData(SendShuffleDataRequest req,
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
long requireBufferId = req.getRequireBufferId();
+ long sendTime = req.getTimestamp();
+ if (sendTime > 0) {
+ /*
+ * Here we record the transport time, but we don't consider the impact of data size on transport time.
+ * The amount of data will not cause great fluctuations in latency. For example, 100K costs 1ms,
+ * and 1M costs 10ms. This seems like a normal fluctuation, but it may rise to 10s when the server load is high.
+ * */
+ shuffleServer.getGrpcMetrics().recordTransportTime(ShuffleServerGrpcMetrics.SEND_SHUFFLE_DATA_METHOD,
Review Comment:
Perhaps we can add a comment stating that the time of the client machine and the server machine should be in sync. For the case of less than 0, we do a judgment filter, but time out of sync will also affect metrics
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] codecov-commenter commented on pull request #327: [FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#issuecomment-1316258595
# [Codecov](https://codecov.io/gh/apache/incubator-uniffle/pull/327?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#327](https://codecov.io/gh/apache/incubator-uniffle/pull/327?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2ad8c79) into [master](https://codecov.io/gh/apache/incubator-uniffle/commit/eae2621b33a51013348a3060ed42e9eb1388c59b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (eae2621) will **decrease** coverage by `1.28%`.
> The diff coverage is `31.91%`.
```diff
@@ Coverage Diff @@
## master #327 +/- ##
============================================
- Coverage 61.21% 59.93% -1.29%
+ Complexity 1506 1424 -82
============================================
Files 185 176 -9
Lines 9360 8861 -499
Branches 908 877 -31
============================================
- Hits 5730 5311 -419
+ Misses 3325 3259 -66
+ Partials 305 291 -14
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-uniffle/pull/327?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...java/org/apache/uniffle/common/util/Constants.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NvbW1vbi91dGlsL0NvbnN0YW50cy5qYXZh) | `0.00% <ø> (ø)` | |
| [...pache/uniffle/server/ShuffleServerGrpcService.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyR3JwY1NlcnZpY2UuamF2YQ==) | `0.83% <0.00%> (-0.03%)` | :arrow_down: |
| [...org/apache/uniffle/common/metrics/GRPCMetrics.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NvbW1vbi9tZXRyaWNzL0dSUENNZXRyaWNzLmphdmE=) | `40.00% <16.66%> (-6.52%)` | :arrow_down: |
| [.../apache/uniffle/common/metrics/MetricsManager.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL2NvbW1vbi9tZXRyaWNzL01ldHJpY3NNYW5hZ2VyLmphdmE=) | `68.42% <20.00%> (-17.30%)` | :arrow_down: |
| [...pache/uniffle/server/ShuffleServerGrpcMetrics.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2VydmVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS91bmlmZmxlL3NlcnZlci9TaHVmZmxlU2VydmVyR3JwY01ldHJpY3MuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...storage/handler/impl/DataSkippableReadHandler.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmFnZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvdW5pZmZsZS9zdG9yYWdlL2hhbmRsZXIvaW1wbC9EYXRhU2tpcHBhYmxlUmVhZEhhbmRsZXIuamF2YQ==) | `83.78% <0.00%> (-2.71%)` | :arrow_down: |
| [.../org/apache/spark/shuffle/writer/WriterBuffer.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvV3JpdGVyQnVmZmVyLmphdmE=) | | |
| [...e/spark/shuffle/reader/RssShuffleDataIterator.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS9yZWFkZXIvUnNzU2h1ZmZsZURhdGFJdGVyYXRvci5qYXZh) | | |
| [...k/shuffle/writer/WrappedByteArrayOutputStream.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS93cml0ZXIvV3JhcHBlZEJ5dGVBcnJheU91dHB1dFN0cmVhbS5qYXZh) | | |
| [.../java/org/apache/spark/shuffle/RssSparkConfig.java](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50LXNwYXJrL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc3Bhcmsvc2h1ZmZsZS9Sc3NTcGFya0NvbmZpZy5qYXZh) | | |
| ... and [5 more](https://codecov.io/gh/apache/incubator-uniffle/pull/327/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023463430
##########
common/src/main/java/org/apache/uniffle/common/metrics/GRPCMetrics.java:
##########
@@ -119,4 +138,12 @@ public Gauge getGaugeGrpcOpen() {
public Counter getCounterGrpcTotal() {
return counterGrpcTotal;
}
+
+ public Map<String, Summary> getSendTimeSummaryMap() {
Review Comment:
What's the meaning of `sendTime`?
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcService.java:
##########
@@ -186,6 +186,11 @@ public void sendShuffleData(SendShuffleDataRequest req,
String appId = req.getAppId();
int shuffleId = req.getShuffleId();
long requireBufferId = req.getRequireBufferId();
+ long sendTime = req.getSendTime();
+ if (sendTime > 0) {
+ shuffleServer.getGrpcMetrics().recordSendTime(ShuffleServerGrpcMetrics.SEND_SHUFFLE_DATA_METHOD,
Review Comment:
Do we need consider the data size when we calculate the metrics?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi commented on a diff in pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi commented on code in PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327#discussion_r1023486237
##########
server/src/main/java/org/apache/uniffle/server/ShuffleServerGrpcMetrics.java:
##########
@@ -110,6 +120,20 @@ public void registerMetrics() {
metricsManager.addCounter(GRPC_GET_IN_MEMORY_SHUFFLE_DATA_TOTAL));
counterMap.putIfAbsent(GET_SHUFFLE_INDEX_METHOD,
metricsManager.addCounter(GRPC_GET_SHUFFLE_INDEX_TOTAL));
+
+ transportTimeSummaryMap.putIfAbsent(SEND_SHUFFLE_DATA_METHOD,
+ metricsManager.addSummary(GRPC_SEND_SHUFFLE_DATA_SEND_LATENCY));
+ transportTimeSummaryMap.putIfAbsent(GET_SHUFFLE_DATA_METHOD,
+ metricsManager.addSummary(GRPC_GET_SHUFFLE_DATA_SEND_LATENCY));
+ transportTimeSummaryMap.putIfAbsent(GET_IN_MEMORY_SHUFFLE_DATA_METHOD,
+ metricsManager.addSummary(GRPC_GET_IN_MEMORY_SHUFFLE_DATA_SEND_LATENCY));
Review Comment:
SEND -> TRANSPORT?
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org
[GitHub] [incubator-uniffle] jerqi merged pull request #327: [ISSUE-309][FEATURE] Support ShuffleServer latency metrics.
Posted by GitBox <gi...@apache.org>.
jerqi merged PR #327:
URL: https://github.com/apache/incubator-uniffle/pull/327
--
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: issues-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@uniffle.apache.org
For additional commands, e-mail: issues-help@uniffle.apache.org