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: [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]
