You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by "jia-gao (via GitHub)" <gi...@apache.org> on 2023/06/30 18:11:58 UTC
[GitHub] [samza] jia-gao opened a new pull request, #1677: Add new SamzaApplicationMaster metric to track containers allocated b…
jia-gao opened a new pull request, #1677:
URL: https://github.com/apache/samza/pull/1677
…y RM and buffered in AM
--
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: commits-unsubscribe@samza.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] shanthoosh commented on a diff in pull request #1677: Add new SamzaApplicationMaster metric to track allocated containers buffered in AM
Posted by "shanthoosh (via GitHub)" <gi...@apache.org>.
shanthoosh commented on code in PR #1677:
URL: https://github.com/apache/samza/pull/1677#discussion_r1267151174
##########
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala:
##########
@@ -42,13 +42,22 @@ class SamzaAppMasterMetrics(val config: Config,
private val metricsConfig = new MetricsConfig(config)
val containersFromPreviousAttempts = newGauge("container-from-previous-attempt", 0L)
+ val allocatedContainersInBuffer = newGauge("allocated-containers-in-buffer", 0L)
Review Comment:
Can we please add the documentation for this metric right after the `container-from-previous-attempt` metrics in this file : https://github.com/apache/samza/blob/master/docs/learn/documentation/versioned/container/metrics-table.html#L523C13-L523C44 ?
--
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: commits-unsubscribe@samza.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] jia-gao commented on a diff in pull request #1677: Add new SamzaApplicationMaster metric to track allocated containers buffered in AM
Posted by "jia-gao (via GitHub)" <gi...@apache.org>.
jia-gao commented on code in PR #1677:
URL: https://github.com/apache/samza/pull/1677#discussion_r1267127390
##########
samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java:
##########
@@ -298,6 +298,7 @@ public void releaseResources(SamzaResource resource) {
}
amClient.releaseAssignedContainer(container.getId());
allocatedResources.remove(resource);
+ metrics.decrementAllocatedContainersInBuffer();
Review Comment:
I want this metric to always in sync with "allocatedResources" Map in this class.
stopStreamProcessor doesn't remove container from allocatedResources.
Actually, in stopStreamProcessor. it calls amClient.releaseAssignedContainer(yarnContainer.id()) only when allocatedResources.get(resource) == null which means the container was not in the "allocatedResources" Map already
--
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: commits-unsubscribe@samza.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] shanthoosh commented on a diff in pull request #1677: Add new SamzaApplicationMaster metric to track allocated containers buffered in AM
Posted by "shanthoosh (via GitHub)" <gi...@apache.org>.
shanthoosh commented on code in PR #1677:
URL: https://github.com/apache/samza/pull/1677#discussion_r1267105644
##########
samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java:
##########
@@ -298,6 +298,7 @@ public void releaseResources(SamzaResource resource) {
}
amClient.releaseAssignedContainer(container.getId());
allocatedResources.remove(resource);
+ metrics.decrementAllocatedContainersInBuffer();
Review Comment:
Do we want to decrement this metric in the stopStreamProcessor method as well?
--
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: commits-unsubscribe@samza.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] shanthoosh merged pull request #1677: Add new SamzaApplicationMaster metric to track allocated containers buffered in AM
Posted by "shanthoosh (via GitHub)" <gi...@apache.org>.
shanthoosh merged PR #1677:
URL: https://github.com/apache/samza/pull/1677
--
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: commits-unsubscribe@samza.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] jia-gao commented on a diff in pull request #1677: Add new SamzaApplicationMaster metric to track allocated containers buffered in AM
Posted by "jia-gao (via GitHub)" <gi...@apache.org>.
jia-gao commented on code in PR #1677:
URL: https://github.com/apache/samza/pull/1677#discussion_r1267127390
##########
samza-yarn/src/main/java/org/apache/samza/job/yarn/YarnClusterResourceManager.java:
##########
@@ -298,6 +298,7 @@ public void releaseResources(SamzaResource resource) {
}
amClient.releaseAssignedContainer(container.getId());
allocatedResources.remove(resource);
+ metrics.decrementAllocatedContainersInBuffer();
Review Comment:
I want this metric to always in sync with "allocatedResources" in this class.
stopStreamProcessor doesn't remove container from allocatedResources.
Actually, in stopStreamProcessor. it calls amClient.releaseAssignedContainer(yarnContainer.id()) only when allocatedResources.get(resource) == null which means the container was not in the "allocatedResources" Map already
--
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: commits-unsubscribe@samza.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [samza] jia-gao commented on a diff in pull request #1677: Add new SamzaApplicationMaster metric to track allocated containers buffered in AM
Posted by "jia-gao (via GitHub)" <gi...@apache.org>.
jia-gao commented on code in PR #1677:
URL: https://github.com/apache/samza/pull/1677#discussion_r1267160104
##########
samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterMetrics.scala:
##########
@@ -42,13 +42,22 @@ class SamzaAppMasterMetrics(val config: Config,
private val metricsConfig = new MetricsConfig(config)
val containersFromPreviousAttempts = newGauge("container-from-previous-attempt", 0L)
+ val allocatedContainersInBuffer = newGauge("allocated-containers-in-buffer", 0L)
Review Comment:
added
--
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: commits-unsubscribe@samza.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org