You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@nemo.apache.org by GitBox <gi...@apache.org> on 2021/03/30 07:30:57 UTC
[GitHub] [incubator-nemo] Lemarais opened a new pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Lemarais opened a new pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307
**Major changes:**
-
**Minor changes to note:**
- Change yarn command in run_beam.sh file to avoid conflict with javascript package manager
- Split metric saving part from flushMetric function and call metric saving part after waiting metric flush
**Tests for the changes:**
-
**Other comments:**
-
Closes #GITHUB_PR_NUMBER
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] sonarcloud[bot] commented on pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#issuecomment-812362418
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307&metric=duplicated_lines_density&view=list) No Duplication information
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] Lemarais commented on a change in pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
Lemarais commented on a change in pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#discussion_r604751016
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/RuntimeMaster.java
##########
@@ -219,11 +224,32 @@ public void recordIRDAGMetrics(final IRDAG irdag, final String planId) {
* Flush metrics.
*/
public void flushMetrics() {
- // send metric flush request to all executors
- metricManagerMaster.sendMetricFlushRequest();
+ if (metricCountDownLatch.getCount() == 0) {
+ metricCountDownLatch = new CountDownLatch(executorRegistry.getNumberOfRunningExecutors());
+ // send metric flush request to all executors
+ metricManagerMaster.sendMetricFlushRequest();
+ }
+ }
+ /**
+ * Save metrics.
+ */
+ public void saveMetrics() {
Review comment:
Okay, that's make sense, but if i move location of this function to nemo driver, The running thread maybe different. And MetricStrore seems not thread safe. so i will check more about 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] wonook merged pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
wonook merged pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] sonarcloud[bot] commented on pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#issuecomment-809986840
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307&metric=duplicated_lines_density&view=list) No Duplication information
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] sonarcloud[bot] removed a comment on pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#issuecomment-809986840
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307&metric=duplicated_lines_density&view=list) No Duplication information
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] Lemarais commented on a change in pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
Lemarais commented on a change in pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#discussion_r606048988
##########
File path: runtime/driver/src/main/java/org/apache/nemo/driver/NemoDriver.java
##########
@@ -142,7 +142,10 @@ private void setUpLogger() {
*/
private void shutdown() {
LOG.info("Driver shutdown initiated");
- runnerThread.execute(runtimeMaster::terminate);
+ runnerThread.execute(() -> {
+ runtimeMaster.saveMetrics();
Review comment:
i try to minimize side-effects. If it looks like there is no problem to move that line, I will merge two functions, flushMetric and saveMetric
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] Lemarais commented on a change in pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
Lemarais commented on a change in pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#discussion_r605408419
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/RuntimeMaster.java
##########
@@ -274,6 +300,8 @@ public void terminate() {
Thread.currentThread().interrupt();
}
+ saveMetrics();
Review comment:
i moved this function call to nemo driver
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] sonarcloud[bot] commented on pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#issuecomment-811641638
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307&metric=duplicated_lines_density&view=list) No Duplication information
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] sonarcloud[bot] removed a comment on pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#issuecomment-811641638
Kudos, SonarCloud Quality Gate passed!
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=org.apache.nemo%3Anemo-project&pullRequest=307&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo.png' alt='No Coverage information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307) No Coverage information
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo.png' alt='No Duplication information' width='16' height='16' />](https://sonarcloud.io/component_measures?id=org.apache.nemo%3Anemo-project&pullRequest=307&metric=duplicated_lines_density&view=list) No Duplication information
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] bgchun commented on pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
bgchun commented on pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#issuecomment-812475158
Great job!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] wonook commented on a change in pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#discussion_r604721163
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/RuntimeMaster.java
##########
@@ -219,11 +224,32 @@ public void recordIRDAGMetrics(final IRDAG irdag, final String planId) {
* Flush metrics.
*/
public void flushMetrics() {
- // send metric flush request to all executors
- metricManagerMaster.sendMetricFlushRequest();
+ if (metricCountDownLatch.getCount() == 0) {
+ metricCountDownLatch = new CountDownLatch(executorRegistry.getNumberOfRunningExecutors());
+ // send metric flush request to all executors
+ metricManagerMaster.sendMetricFlushRequest();
+ }
+ }
+ /**
+ * Save metrics.
+ */
+ public void saveMetrics() {
Review comment:
I think this method should be called in the NemoDriver.java file, where it originally requests to save the metrics after sending the flush request.
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/RuntimeMaster.java
##########
@@ -219,11 +224,32 @@ public void recordIRDAGMetrics(final IRDAG irdag, final String planId) {
* Flush metrics.
*/
public void flushMetrics() {
- // send metric flush request to all executors
- metricManagerMaster.sendMetricFlushRequest();
+ if (metricCountDownLatch.getCount() == 0) {
+ metricCountDownLatch = new CountDownLatch(executorRegistry.getNumberOfRunningExecutors());
+ // send metric flush request to all executors
+ metricManagerMaster.sendMetricFlushRequest();
+ }
+ }
+ /**
+ * Save metrics.
+ */
+ public void saveMetrics() {
+ try {
+ if (!metricCountDownLatch.await(METRIC_ARRIVE_TIMEOUT, TimeUnit.MILLISECONDS)) {
Review comment:
Have you checked if the job completion time metric does not get affected by the metric-waiting procedure?
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/metric/MetricStore.java
##########
@@ -245,6 +246,8 @@ public void dumpAllMetricToFile(final String filePath) {
try (BufferedWriter writer = new BufferedWriter(new FileWriter(filePath))) {
final String jsonDump = dumpAllMetricToJson();
writer.write(jsonDump);
+ } catch (final FileNotFoundException e) {
+ LOG.warn("fail to write metric to local file: {}", e);
Review comment:
fail -> Failed
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/metric/MetricStore.java
##########
@@ -245,6 +246,8 @@ public void dumpAllMetricToFile(final String filePath) {
try (BufferedWriter writer = new BufferedWriter(new FileWriter(filePath))) {
final String jsonDump = dumpAllMetricToJson();
writer.write(jsonDump);
+ } catch (final FileNotFoundException e) {
+ LOG.warn("fail to write metric to local file: {}", e);
Review comment:
or maybe "Failure while writing metric ..."
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/RuntimeMaster.java
##########
@@ -274,6 +300,8 @@ public void terminate() {
Thread.currentThread().interrupt();
}
+ saveMetrics();
Review comment:
This is also related to the NemoDriver.java comment.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] wonook commented on pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
wonook commented on pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#issuecomment-812473969
@Lemarais Congrats on your first PR!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] Lemarais commented on a change in pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
Lemarais commented on a change in pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#discussion_r606050107
##########
File path: runtime/driver/src/main/java/org/apache/nemo/driver/NemoDriver.java
##########
@@ -142,7 +142,10 @@ private void setUpLogger() {
*/
private void shutdown() {
LOG.info("Driver shutdown initiated");
- runnerThread.execute(runtimeMaster::terminate);
+ runnerThread.execute(() -> {
+ runtimeMaster.saveMetrics();
Review comment:
I tried to minimize the side effect, but it seems to have been my misunderstanding. I will merge flushMetric function and saveMetric function
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] Lemarais commented on a change in pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
Lemarais commented on a change in pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#discussion_r604737345
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/RuntimeMaster.java
##########
@@ -219,11 +224,32 @@ public void recordIRDAGMetrics(final IRDAG irdag, final String planId) {
* Flush metrics.
*/
public void flushMetrics() {
- // send metric flush request to all executors
- metricManagerMaster.sendMetricFlushRequest();
+ if (metricCountDownLatch.getCount() == 0) {
+ metricCountDownLatch = new CountDownLatch(executorRegistry.getNumberOfRunningExecutors());
+ // send metric flush request to all executors
+ metricManagerMaster.sendMetricFlushRequest();
+ }
+ }
+ /**
+ * Save metrics.
+ */
+ public void saveMetrics() {
+ try {
+ if (!metricCountDownLatch.await(METRIC_ARRIVE_TIMEOUT, TimeUnit.MILLISECONDS)) {
Review comment:
Actually i check about it now, It seems that metric does not affect to the job completion time. job completion time is checked in the UserApplication.run Function before RuntimeMaster send the flushMetric signal.
But it can be possible waiting metric with doubled duration at the end of program, when first wait over timeout. But i think it is not a big deal
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] wonook commented on a change in pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
wonook commented on a change in pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#discussion_r606015526
##########
File path: runtime/driver/src/main/java/org/apache/nemo/driver/NemoDriver.java
##########
@@ -142,7 +142,10 @@ private void setUpLogger() {
*/
private void shutdown() {
LOG.info("Driver shutdown initiated");
- runnerThread.execute(runtimeMaster::terminate);
+ runnerThread.execute(() -> {
+ runtimeMaster.saveMetrics();
Review comment:
Would there be a particular reason why this is not on L203?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nemo] Lemarais commented on a change in pull request #307: [MINOR] Avoid yarn conflict + Save Metrics after waiting to get full data
Posted by GitBox <gi...@apache.org>.
Lemarais commented on a change in pull request #307:
URL: https://github.com/apache/incubator-nemo/pull/307#discussion_r605408711
##########
File path: runtime/master/src/main/java/org/apache/nemo/runtime/master/metric/MetricStore.java
##########
@@ -245,6 +246,8 @@ public void dumpAllMetricToFile(final String filePath) {
try (BufferedWriter writer = new BufferedWriter(new FileWriter(filePath))) {
final String jsonDump = dumpAllMetricToJson();
writer.write(jsonDump);
+ } catch (final FileNotFoundException e) {
+ LOG.warn("fail to write metric to local file: {}", e);
Review comment:
i changed error message to Failure while writing metric ...
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org