HDDS-1783 : Latency metric for applyTransaction in ContainerStateMach… #1363
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/label ozone |
|
Thanks @avijayanhwx for working on this. The changes look good. In applyTransaction, we execute different cmds which will vary with latency. For example, for a write chunk commit, its a rename operation but for putBlock , it is Rocks DB operation and for close container, its again a disk sync operation of container state. Depending upon the operation the latency will vary as well. Can we have per cmd latency metric? |
adoroszlai
reviewed
Aug 28, 2019
...rc/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/CSMMetrics.java
Show resolved
Hide resolved
|
💔 -1 overall
This message was automatically generated. |
@bshashikant Isn't the per operation latency covered in org.apache.hadoop.ozone.container.common.transport.server.ratis.CSMMetrics#opsLatency? This metric was only intended to cover the whole latency including thread scheduling. |
|
Thanks @avijayanhwx . Yes, the per op applyTransaction latency metrics is already there but we don't have writeStateMachineData latency metric which again runs in a different executor. Can we add that as well. This can be done as a separate jira as well. |
added 2 commits
August 28, 2019 11:25
…ine. (Add a rate metric for WriteStateMachineData)
…ine. (Fix compilation issue)
Added rate metric for writeStateMachineData as well. |
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @avijayanhwx for updating. Can we also add some tests for the added metric in TestCSMMetrics ? |
added 2 commits
August 29, 2019 10:44
Added unit test for the metrics. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
Thanks @avijayanhwx for working on this. I am +1 on the change. |
|
Thanks @avijayanhwx for the contribution. I have committed this. |
amahussein
pushed a commit
to amahussein/hadoop
that referenced
this pull request
Oct 29, 2019
RogPodge
pushed a commit
to RogPodge/hadoop
that referenced
this pull request
Mar 25, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…ine.
applyTransaction is invoked from the Ratis pipeline and the ContainerStateMachine uses a async executor to complete the task.
We require a latency metric to track the performance of log apply operations in the state machine. This will measure the end-to-end latency of apply which includes the queueing delay in the executor queues. Combined with the latency measurement in HddsDispatcher, this will be an indicator if the executors are overloaded.