HDDS-1750. Add block allocation metrics for pipelines in SCM #1047
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
|
💔 -1 overall
This message was automatically generated. |
mukul1987
requested changes
Jul 8, 2019
There was a problem hiding this comment.
Thanks for working on this @lokeshj1703. The patch generally looks good to me.
| @@ -152,6 +152,7 @@ public synchronized Pipeline createPipeline( | |||
| stateManager.addPipeline(pipeline); | |||
| nodeManager.addPipeline(pipeline); | |||
| metrics.incNumPipelineCreated(); | |||
| metrics.createNumBlocksAllocatedMetric(pipeline); | |||
There was a problem hiding this comment.
Can line 154 and 155 be done in one state to pipelineMetrics ?
There was a problem hiding this comment.
This should be named as createPerPipelineMetrics or something like that :)
| @@ -362,6 +364,7 @@ private void finalizePipeline(PipelineID pipelineId) throws IOException { | |||
| for (ContainerID containerID : containerIDs) { | |||
| eventPublisher.fireEvent(SCMEvents.CLOSE_CONTAINER, containerID); | |||
| } | |||
| metrics.clearMetrics(pipelineId); | |||
There was a problem hiding this comment.
Lets rename this to removePipelineMetrics
|
@mukul1987 Thanks for reviewing the PR! I have addresses the review commits in the latest commit. |
|
💔 -1 overall
This message was automatically generated. |
mukul1987
approved these changes
Jul 8, 2019
bshashikant
pushed a commit
to bshashikant/hadoop
that referenced
this pull request
Jul 10, 2019
…uted by Lokesh Jain. (apache#1047)
shanthoosh
pushed a commit
to shanthoosh/hadoop
that referenced
this pull request
Oct 15, 2019
… doing reflection (apache#1047) * SAMZA-2210: Initial majority migration for injecting classloader when doing reflection * apply reordering of arguments for ReflectionUtil methods * rename test method * fixing issues after merge * use buildContainerAllocator in ContainerProcessManager after merge * clean up variables and naming according to PR comments
amahussein
pushed a commit
to amahussein/hadoop
that referenced
this pull request
Oct 29, 2019
…uted by Lokesh Jain. (apache#1047)
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.
This Jira aims to add block allocation metrics for pipelines in SCM. This would help in determining the distribution of block allocations among various pipelines in SCM.