You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "duongkame (via GitHub)" <gi...@apache.org> on 2023/10/11 00:50:10 UTC
[PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
duongkame opened a new pull request, #5421:
URL: https://github.com/apache/ozone/pull/5421
## What changes were proposed in this pull request?
These added metrics expose the details of where write operators spend time one.
- On RPC server:
- Request validation
- PreExecute
- Convert OM request to Ratis format
- Ratis invocation
- Convert ratis response to OM respose.
- On ratis:
- applyTransaction local latency.
## What is the link to the Apache JIRA
(Please create an issue in ASF JIRA before opening a pull request,
and you need to set the title of the pull request which starts with
the corresponding JIRA issue number. (e.g. HDDS-XXXX. Fix a typo in YYY.)
Please replace this section with the link to the Apache JIRA)
## How was this patch tested?
(Please explain how this patch was tested. Ex: unit tests, manual tests)
(If this patch involves UI changes, please attach a screen-shot; otherwise, remove 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #5421:
URL: https://github.com/apache/ozone/pull/5421#discussion_r1366121557
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -278,11 +301,11 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
*/
public OMResponse submitRequest(OMRequest omRequest,
RaftClientRequest raftClientRequest) throws ServiceException {
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
- return processReply(omRequest, raftClientReply);
+ RaftClientReply raftClientReply = submitRequestToRatisImpl(raftClientRequest);
+ return createOmResponseImpl(omRequest, raftClientReply);
Review Comment:
@duongkame , ping, in case you don't see 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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame merged PR #5421:
URL: https://github.com/apache/ozone/pull/5421
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #5421:
URL: https://github.com/apache/ozone/pull/5421#discussion_r1364433076
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -278,11 +301,11 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
*/
public OMResponse submitRequest(OMRequest omRequest,
RaftClientRequest raftClientRequest) throws ServiceException {
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
- return processReply(omRequest, raftClientReply);
+ RaftClientReply raftClientReply = submitRequestToRatisImpl(raftClientRequest);
+ return createOmResponseImpl(omRequest, raftClientReply);
Review Comment:
This should use `createOmResponse`.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on code in PR #5421:
URL: https://github.com/apache/ozone/pull/5421#discussion_r1366211056
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -278,11 +301,12 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
*/
public OMResponse submitRequest(OMRequest omRequest,
RaftClientRequest raftClientRequest) throws ServiceException {
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
- return processReply(omRequest, raftClientReply);
+ RaftClientReply raftClientReply =
+ submitRequestToRatisImpl(raftClientRequest);
Review Comment:
```suggestion
submitRequestToRatis(raftClientRequest);
```
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on PR #5421:
URL: https://github.com/apache/ozone/pull/5421#issuecomment-1773514589
Thanks @szetszwo @tanvipenumudy for the reviews.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on PR #5421:
URL: https://github.com/apache/ozone/pull/5421#issuecomment-1756620412
cc. @tanvipenumudy
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on code in PR #5421:
URL: https://github.com/apache/ozone/pull/5421#discussion_r1361374616
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -246,11 +250,16 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
// In prepare mode, only prepare and cancel requests are allowed to go
// through.
if (ozoneManager.getPrepareState().requestAllowed(omRequest.getCmdType())) {
- RaftClientRequest raftClientRequest =
- createWriteRaftClientRequest(omRequest);
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
-
- return processReply(omRequest, raftClientReply);
+ RaftClientRequest raftClientRequest = captureLatencyNs(
+ perfMetrics.getConvertRatisRequestLatencyNs(),
+ () -> createWriteRaftClientRequest(omRequest));
Review Comment:
Agree. Updated.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "tanvipenumudy (via GitHub)" <gi...@apache.org>.
tanvipenumudy commented on code in PR #5421:
URL: https://github.com/apache/ozone/pull/5421#discussion_r1360271903
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -246,11 +250,16 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
// In prepare mode, only prepare and cancel requests are allowed to go
// through.
if (ozoneManager.getPrepareState().requestAllowed(omRequest.getCmdType())) {
- RaftClientRequest raftClientRequest =
- createWriteRaftClientRequest(omRequest);
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
-
- return processReply(omRequest, raftClientReply);
+ RaftClientRequest raftClientRequest = captureLatencyNs(
+ perfMetrics.getConvertRatisRequestLatencyNs(),
+ () -> createWriteRaftClientRequest(omRequest));
Review Comment:
> Let's use the same name? i.e. when the method name is createWriteRaftClientRequest, the metric name should be getCreateWriteRaftClientRequestLatencyNs().
I agree with maintaining consistency by using the same name for methods and 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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on code in PR #5421:
URL: https://github.com/apache/ozone/pull/5421#discussion_r1366161406
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -278,11 +301,11 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
*/
public OMResponse submitRequest(OMRequest omRequest,
RaftClientRequest raftClientRequest) throws ServiceException {
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
- return processReply(omRequest, raftClientReply);
+ RaftClientReply raftClientReply = submitRequestToRatisImpl(raftClientRequest);
+ return createOmResponseImpl(omRequest, raftClientReply);
Review Comment:
Hi @szetszwo , updated.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #5421:
URL: https://github.com/apache/ozone/pull/5421#discussion_r1364433076
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -278,11 +301,11 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
*/
public OMResponse submitRequest(OMRequest omRequest,
RaftClientRequest raftClientRequest) throws ServiceException {
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
- return processReply(omRequest, raftClientReply);
+ RaftClientReply raftClientReply = submitRequestToRatisImpl(raftClientRequest);
+ return createOmResponseImpl(omRequest, raftClientReply);
Review Comment:
This should use `submitRequestToRatis` and `createOmResponse`.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "szetszwo (via GitHub)" <gi...@apache.org>.
szetszwo commented on code in PR #5421:
URL: https://github.com/apache/ozone/pull/5421#discussion_r1355681598
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -246,11 +250,16 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
// In prepare mode, only prepare and cancel requests are allowed to go
// through.
if (ozoneManager.getPrepareState().requestAllowed(omRequest.getCmdType())) {
- RaftClientRequest raftClientRequest =
- createWriteRaftClientRequest(omRequest);
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
-
- return processReply(omRequest, raftClientReply);
+ RaftClientRequest raftClientRequest = captureLatencyNs(
+ perfMetrics.getConvertRatisRequestLatencyNs(),
+ () -> createWriteRaftClientRequest(omRequest));
Review Comment:
Let's use the same name? i.e. when the method name is ` createWriteRaftClientRequest`, the metric name should be `getCreateWriteRaftClientRequestLatencyNs()`.
If the existing method name is not good, we may change it as well.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/ratis/OzoneManagerRatisServer.java:
##########
@@ -246,11 +250,16 @@ public OMResponse submitRequest(OMRequest omRequest) throws ServiceException {
// In prepare mode, only prepare and cancel requests are allowed to go
// through.
if (ozoneManager.getPrepareState().requestAllowed(omRequest.getCmdType())) {
- RaftClientRequest raftClientRequest =
- createWriteRaftClientRequest(omRequest);
- RaftClientReply raftClientReply = submitRequestToRatis(raftClientRequest);
-
- return processReply(omRequest, raftClientReply);
+ RaftClientRequest raftClientRequest = captureLatencyNs(
+ perfMetrics.getConvertRatisRequestLatencyNs(),
+ () -> createWriteRaftClientRequest(omRequest));
Review Comment:
Also, let's create new methods to hide the metric layer as below. The code will be more readable.
```java
@@ -414,13 +418,19 @@ public void removeRaftPeer(OMNodeDetails omNodeDetails) {
LOG.info("{}: Removed OM {} from Ratis Peers list.", this, removeNodeID);
}
+ private RaftClientRequest createWriteRaftClientRequest(OMRequest omRequest) {
+ return captureLatencyNs(
+ perfMetrics.getCreateWriteRaftClientRequestLatencyNs(),
+ () -> createWriteRaftClientRequestImpl(omRequest));
+ }
+
/**
* Create Write RaftClient request from OMRequest.
* @param omRequest
* @return RaftClientRequest - Raft Client request which is submitted to
* ratis server.
*/
- private RaftClientRequest createWriteRaftClientRequest(OMRequest omRequest) {
+ private RaftClientRequest createWriteRaftClientRequestImpl(OMRequest omRequest) {
if (!ozoneManager.isTestSecureOmFlag()) {
Preconditions.checkArgument(Server.getClientId() != DUMMY_CLIENT_ID);
Preconditions.checkArgument(Server.getCallId() != INVALID_CALL_ID);
```
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-9425. Breakdown metrics for OM writes [ozone]
Posted by "duongkame (via GitHub)" <gi...@apache.org>.
duongkame commented on PR #5421:
URL: https://github.com/apache/ozone/pull/5421#issuecomment-1769095543
> -0 Adding the `captureLatencyNs(..)` everywhere really makes the code hard to read. Please consdier [#5421 (comment)](https://github.com/apache/ozone/pull/5421#discussion_r1355686902)
Thanks @szetszwo, sorry I missed it. Updated.
--
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@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org