You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/07/17 16:56:12 UTC
[GitHub] [hadoop-ozone] maobaolong opened a new pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
maobaolong opened a new pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214
## What changes were proposed in this pull request?
When I profile the s3g performance, i find in some log of XceiverClientGrpc, I cannot find the related datanode ip, so i use datanode instead of the UUID and networkFullPath.
Also, i add a debug level log to show the request has been executed and output the try counts.
## What is the link to the Apache JIRA
HDDS-3981
## How was this patch tested?
No need, all changes only related to log.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] elek commented on pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#issuecomment-688169967
@bharatviswa504 What do we need to make it possible to merge this commit?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r464749747
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -433,7 +442,7 @@ public XceiverClientReply sendCommandAsync(
UUID dnId = dn.getUuid();
if (LOG.isDebugEnabled()) {
LOG.debug("Send command {} to datanode {}",
- request.getCmdType(), dn.getNetworkFullPath());
+ request.getCmdType(), dn);
Review comment:
Correct me if I'm wrong, I think we have already print the same debug message on Line 338-340 from the caller. Print just the IP address should be sufficient.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
maobaolong commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r488328103
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -339,6 +343,11 @@ private XceiverClientReply sendCommandWithRetry(
// in case these don't exist for the specific datanode.
reply.addDatanode(dn);
responseProto = sendCommandAsync(request, dn).getResponse().get();
+ if (LOG.isDebugEnabled()) {
Review comment:
@xiaoyuyao Thanks you for your reply and suggestion. I tried to add the LOG into the L464, like following
```java
public void onNext(ContainerCommandResponseProto value) {
replyFuture.complete(value);
metrics.decrPendingContainerOpsMetrics(request.getCmdType());
long cost = System.nanoTime() - requestTime;
metrics.addContainerOpsLatency(request.getCmdType(),
cost);
if (LOG.isDebugEnabled()) {
LOG.debug("Executed command {} on datanode {}, cost = {}, "
+ "retryCount = {}" + ", cmdType = {}", request, dn,
cost, index, request.getCmdType());
}
semaphore.release();
}
```
But, there are some problem
- It cannot print the `Index` which stand for the retried datanode index of the `datanodeList`.
- I should add the same log logic to the `onError` also? Otherwise, when we met error, the `onNext` won't be executed.
As `index` can be calculate from the log context, I drop it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r486649479
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -354,7 +363,7 @@ private XceiverClientReply sendCommandWithRetry(
responseProto = null;
} catch (ExecutionException e) {
LOG.debug("Failed to execute command {} on datanode {}",
Review comment:
Good point, you are right on this.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
maobaolong commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r462704319
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -354,7 +363,7 @@ private XceiverClientReply sendCommandWithRetry(
responseProto = null;
} catch (ExecutionException e) {
LOG.debug("Failed to execute command {} on datanode {}",
- request, dn.getUuid(), e);
Review comment:
I hope to using ip and uuid like ip/uuid, do you think it is ok for you?
Because, i want to get the ip so that i can clearly know the exception related to which datanode, but in local mode, all of ips of datanode are the same, for this situation, an addition uuid can help me to distinguish.
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -339,6 +343,11 @@ private XceiverClientReply sendCommandWithRetry(
// in case these don't exist for the specific datanode.
reply.addDatanode(dn);
responseProto = sendCommandAsync(request, dn).getResponse().get();
+ if (LOG.isDebugEnabled()) {
Review comment:
@xiaoyuyao I hope so, but I think it would be difficult, because, i want to log the retry index and cost time.
when the `sendCommandAsync ` return back, the client still doesn't get the result from datanode. the result return until the `get` return.
If i have something wrong, please correct me.
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -354,7 +363,7 @@ private XceiverClientReply sendCommandWithRetry(
responseProto = null;
} catch (ExecutionException e) {
LOG.debug("Failed to execute command {} on datanode {}",
Review comment:
OK
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
maobaolong commented on pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#issuecomment-692406668
@xiaoyuyao Thanks @xiaoyuyao for the suggestion, I pushed a new commit to address your comment, I reply your comment and ask some question. PTAL.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
maobaolong commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r467662395
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -433,7 +442,7 @@ public XceiverClientReply sendCommandAsync(
UUID dnId = dn.getUuid();
if (LOG.isDebugEnabled()) {
LOG.debug("Send command {} to datanode {}",
- request.getCmdType(), dn.getNetworkFullPath());
+ request.getCmdType(), dn);
Review comment:
OK, you are right, the redundancy log content is useless, I've change it to `dn.getIpAddress()`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r486652573
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -339,6 +343,11 @@ private XceiverClientReply sendCommandWithRetry(
// in case these don't exist for the specific datanode.
reply.addDatanode(dn);
responseProto = sendCommandAsync(request, dn).getResponse().get();
+ if (LOG.isDebugEnabled()) {
Review comment:
I mean if we could move the LOG for latency around Line 464 where the container op latency metrics are updated. This way, we can piggyback LOG with the existing metrics update.
metrics.addContainerOpsLatency(request.getCmdType(),
System.nanoTime() - requestTime);
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#issuecomment-694530571
Thanks @maobaolong for the update. LGTM, +1. Will merge it shortly.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
maobaolong commented on pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#issuecomment-666060954
@xiaoyuyao Thanks to review my PR, i reply to your comments, please check it again, thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao merged pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
xiaoyuyao merged pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
maobaolong commented on pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#issuecomment-671136565
@xiaoyuyao Thank you for the review suggestion, PTAL.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
maobaolong commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r488328103
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -339,6 +343,11 @@ private XceiverClientReply sendCommandWithRetry(
// in case these don't exist for the specific datanode.
reply.addDatanode(dn);
responseProto = sendCommandAsync(request, dn).getResponse().get();
+ if (LOG.isDebugEnabled()) {
Review comment:
@xiaoyuyao Thanks you for your reply and suggestion. I tried to add the LOG into the L464, like following
```java
public void onNext(ContainerCommandResponseProto value) {
replyFuture.complete(value);
metrics.decrPendingContainerOpsMetrics(request.getCmdType());
long cost = System.nanoTime() - requestTime;
metrics.addContainerOpsLatency(request.getCmdType(),
cost);
if (LOG.isDebugEnabled()) {
LOG.debug("Executed command {} on datanode {}, cost = {}, "
+ "retryCount = {}" + ", cmdType = {}", request, dn,
cost, index, request.getCmdType());
}
semaphore.release();
}
```
But, there are some question
- It cannot print the `Index` which stand for the retried datanode index of the `datanodeList`.
- I should add the same log logic to the `onError` also? Otherwise, when we met error, the `onNext` won't be executed.
As `index` can be calculate from the log context, I drop it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r486650134
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -354,7 +363,7 @@ private XceiverClientReply sendCommandWithRetry(
responseProto = null;
} catch (ExecutionException e) {
LOG.debug("Failed to execute command {} on datanode {}",
- request, dn.getUuid(), e);
Review comment:
I think ip address should be sufficient for production environment. The local mode is for mini clsuter test only.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] maobaolong commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
maobaolong commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r462705761
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -354,7 +363,7 @@ private XceiverClientReply sendCommandWithRetry(
responseProto = null;
} catch (ExecutionException e) {
LOG.debug("Failed to execute command {} on datanode {}",
Review comment:
I think for this case, the `if (LOG.isDebugEnabled())` guard is redundancy, because, the L365 have no any need compute parameter, all arguments are reference, for the `LOG.debug`, lets go into the implementation of `Log4jLoggerAdapter`, the `if (LOG.isDebugEnabled())` is there
```java
public void debug(String format, Object... arguments) {
if (this.logger.isDebugEnabled()) {
FormattingTuple ft = MessageFormatter.arrayFormat(format, arguments);
this.logger.log(FQCN, Level.DEBUG, ft.getMessage(), ft.getThrowable());
}
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org
[GitHub] [hadoop-ozone] xiaoyuyao commented on a change in pull request #1214: HDDS-3981. Add more debug level log to XceiverClientGrpc for debug purpose
Posted by GitBox <gi...@apache.org>.
xiaoyuyao commented on a change in pull request #1214:
URL: https://github.com/apache/hadoop-ozone/pull/1214#discussion_r461793007
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -354,7 +363,7 @@ private XceiverClientReply sendCommandWithRetry(
responseProto = null;
} catch (ExecutionException e) {
LOG.debug("Failed to execute command {} on datanode {}",
- request, dn.getUuid(), e);
Review comment:
Full DatanodeDetails.toString() include a lot of information and can hurt the perf on critical path. Consider using either ip or uuid instead.
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -354,7 +363,7 @@ private XceiverClientReply sendCommandWithRetry(
responseProto = null;
} catch (ExecutionException e) {
LOG.debug("Failed to execute command {} on datanode {}",
Review comment:
This needs to be guarded by if (LOG.isDebugEnabled())
##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -339,6 +343,11 @@ private XceiverClientReply sendCommandWithRetry(
// in case these don't exist for the specific datanode.
reply.addDatanode(dn);
responseProto = sendCommandAsync(request, dn).getResponse().get();
+ if (LOG.isDebugEnabled()) {
Review comment:
In sendCommandAsync() below we already have a LOG.debug(cmdType, Dn). Can you move this there to avoid duplicate debug log?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org