You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "luoyuan3471 (via GitHub)" <gi...@apache.org> on 2026/10/22 08:23:36 UTC
[PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
luoyuan3471 opened a new pull request, #8854:
URL: https://github.com/apache/hadoop/pull/8854
### Description of PR
When keepContainersAcrossApplicationAttempts is enabled, a container from an earlier application attempt may remain in the scheduler after the old attempt has been removed from RMApp. ClientRMService#getContainerReport currently rejects this request before consulting the scheduler.
This patch skips the application-attempt existence check only for applications that retain containers across attempts. Application ACL validation and the scheduler-side container existence check remain unchanged.
JIRA: https://issues.apache.org/jira/browse/YARN-12039
### How was this patch tested?
Added regression coverage for:
- returning a report for a retained container from a removed attempt;
- preserving ApplicationAttemptNotFoundException when container transfer is disabled;
- preserving ContainerNotFoundException when the scheduler no longer contains the container.
The three focused tests pass, and the complete TestClientRMService class passes (43 tests).
### For code changes:
- [x] Does the title of this PR start with the corresponding JIRA issue id?
- [ ] Object storage: Not applicable.
- [ ] No new dependencies were added.
- [ ] No LICENSE or NOTICE changes are required.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "hadoop-yetus (via GitHub) via common-issues" <co...@hadoop.apache.org>.
hadoop-yetus commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6741099480
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 1m 5s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 2 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 49m 28s | | trunk passed |
| +1 :green_heart: | compile | 1m 26s | | trunk passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | compile | 1m 28s | | trunk passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | checkstyle | 1m 23s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 33s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 16s | | trunk passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javadoc | 1m 13s | | trunk passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 2m 41s | | trunk passed |
| +1 :green_heart: | shadedclient | 37m 54s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 2s | | the patch passed |
| +1 :green_heart: | compile | 0m 57s | | the patch passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javac | 0m 57s | | the patch passed |
| +1 :green_heart: | compile | 1m 0s | | the patch passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | javac | 1m 0s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 55s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 6s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 45s | | the patch passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javadoc | 0m 46s | | the patch passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 2m 25s | | the patch passed |
| +1 :green_heart: | shadedclient | 36m 35s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 115m 48s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 37s | | The patch does not generate ASF License warnings. |
| | | 261m 47s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8854/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/8854 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux 32ac38a72186 5.15.0-198-generic #208-Ubuntu SMP Fri Sep 4 10:22:38 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / 1186f1333a43ed2ca7b58ed4bf1cfd3b016f5f72 |
| Default Java | Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8854/2/testReport/ |
| Max. process+thread count | 896 (vs. ulimit of 10000) |
| modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8854/2/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub)" <gi...@apache.org>.
luoyuan3471 commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6355798880
Hi @slfan1989, could you please take a look when you have time? This fixes container report lookup for containers retained across application attempts. All CI checks are green, including Yetus and the focused TestClientRMService tests. Thanks!
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on code in PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#discussion_r4511404195
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException, IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, true);
+
+ GetContainerReportResponse response = rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId));
+
+ assertEquals(containerId, response.getContainerReport().getContainerId());
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingAttemptWithoutContainerTransfer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, false, false, true);
+
+ assertThrows(ApplicationAttemptNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingRetainedContainer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, false);
+
+ assertThrows(ContainerNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ private ClientRMService createRMServiceForContainerReport(
Review Comment:
Replaced the boolean literals at the call sites with local named final boolean variables. This makes the helper arguments clearer while keeping each test case separate. Is it ok?
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on code in PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#discussion_r4511376204
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException, IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, true);
+
+ GetContainerReportResponse response = rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId));
+
+ assertEquals(containerId, response.getContainerReport().getContainerId());
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingAttemptWithoutContainerTransfer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, false, false, true);
+
+ assertThrows(ApplicationAttemptNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingRetainedContainer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, false);
+
+ assertThrows(ContainerNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ private ClientRMService createRMServiceForContainerReport(
+ ContainerId containerId, boolean keepContainersAcrossAttempts,
+ boolean includeAttempt, boolean includeContainer) {
+ ApplicationAttemptId attemptId = containerId.getApplicationAttemptId();
+ ApplicationId applicationId = attemptId.getApplicationId();
+ ResourceScheduler scheduler = mock(ResourceScheduler.class);
+ RMContext rmContext = mock(RMContext.class);
+ RMApp application = mock(RMApp.class);
+
+ ConcurrentHashMap<ApplicationId, RMApp> applications =
+ new ConcurrentHashMap<>();
+ applications.put(applicationId, application);
+ when(rmContext.getRMApps()).thenReturn(applications);
+ when(rmContext.getScheduler()).thenReturn(scheduler);
+ when(application.getApplicationId()).thenReturn(applicationId);
+ when(application.getUser()).thenReturn("test-user");
+
+ ApplicationSubmissionContext submissionContext =
+ mock(ApplicationSubmissionContext.class);
+ when(submissionContext.getKeepContainersAcrossApplicationAttempts())
+ .thenReturn(keepContainersAcrossAttempts);
+ when(application.getApplicationSubmissionContext())
+ .thenReturn(submissionContext);
+
+ Map<ApplicationAttemptId, RMAppAttempt> attempts = new HashMap<>();
+ if (includeAttempt) {
+ attempts.put(attemptId, mock(RMAppAttempt.class));
+ }
+ when(application.getAppAttempts()).thenReturn(attempts);
+
+ if (includeContainer) {
+ RMContainer rmContainer = mock(RMContainer.class);
+ ContainerReport containerReport = mock(ContainerReport.class);
+ when(containerReport.getContainerId()).thenReturn(containerId);
+ when(rmContainer.createContainerReport()).thenReturn(containerReport);
+ when(scheduler.getRMContainer(containerId)).thenReturn(rmContainer);
+ }
+
+ ApplicationACLsManager applicationACLsManager =
+ mock(ApplicationACLsManager.class);
+ QueueACLsManager queueACLsManager = mock(QueueACLsManager.class);
+ when(queueACLsManager.checkAccess(any(UserGroupInformation.class),
Review Comment:
Agreed, we should also verify that access is still denied on this path. I've added an access flag to the helper and a keepContainers test that checks the request is rejected with the “does not have privilege” message.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub)" <gi...@apache.org>.
luoyuan3471 commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6432263418
Hi @ayushtkn, could you please take a look at this PR when you have time? Thank you!
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "gerashegalov (via GitHub) via common-issues" <co...@hadoop.apache.org>.
gerashegalov commented on code in PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#discussion_r4503462904
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException, IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
Review Comment:
This test fakes the problem by mocking an empty attempts map, so it doesn't show how a real RM gets there. The name is also misleading: a container from a previous attempt already works today, as long as that attempt is still in memory.
Could we add an end-to-end test in `TestAMRestart`? The steps are:
1. Turn on keepContainers and work-preserving recovery. Set max attempts to 2 and a short failure validity interval.
2. Start a container in attempt 1.
3. Fail attempts 1, 2 and 3, waiting past the interval each time, so attempt 1 is deleted from the store.
4. Restart the RM and have the NM report the container again.
5. Check that `getContainerReport` returns it.
I tried this locally. It passes with your change and fails on trunk with `ApplicationAttemptNotFoundException`, for both the Capacity and Fair schedulers, in about 5 seconds each. Before the restart it passes on both. Here's a shortened version of the test I ran, if it helps. I haven't re-run this shorter version, and it needs an import for `GetContainerReportRequest`:
<details><summary>Test sketch</summary>
```java
@ParameterizedTest(name = "{0}")
@MethodSource("getParameters")
@Timeout(value = 120)
public void testContainerReportForAttemptRemovedFromStateStore(
SchedulerType type) throws Exception {
initTestAMRestart(type);
getConf().setInt(YarnConfiguration.RM_AM_MAX_ATTEMPTS, 2);
getConf().setBoolean(YarnConfiguration.RECOVERY_ENABLED, true);
getConf().setBoolean(
YarnConfiguration.RM_WORK_PRESERVING_RECOVERY_ENABLED, true);
getConf().setLong(
YarnConfiguration.RM_WORK_PRESERVING_RECOVERY_SCHEDULING_WAIT_MS, 0);
getConf().set(YarnConfiguration.RM_STORE,
MemoryRMStateStore.class.getName());
final long window = 1000;
MockRM rm1 = new MockRM(getConf());
rm1.start();
MemoryRMStateStore memStore = (MemoryRMStateStore) rm1.getRMStateStore();
MockNM nm1 =
new MockNM("127.0.0.1:1234", 8000, rm1.getResourceTrackerService());
nm1.registerNode();
RMApp app1 = MockRMAppSubmitter.submit(rm1,
MockRMAppSubmissionData.Builder.createWithMemory(200, rm1)
.withAppName("")
.withUser(UserGroupInformation.getCurrentUser().getShortUserName())
.withKeepContainers(true)
.withAttemptFailuresValidityInterval(window)
.build());
MockAM am1 = MockRM.launchAndRegisterAM(app1, rm1, nm1);
ApplicationAttemptId attempt1 = am1.getApplicationAttemptId();
allocateContainers(nm1, am1, 1);
nm1.nodeHeartbeat(attempt1, 2, ContainerState.RUNNING);
ContainerId containerId2 = ContainerId.newContainerId(attempt1, 2);
rm1.waitForState(nm1, containerId2, RMContainerState.RUNNING);
// Fail attempts 1-3, each outside the validity window. Failing attempt 3
// deletes attempt 1 from the state store (removeExcessAttempts).
MockAM am = am1;
for (int i = 1; i <= 3; i++) {
nm1.nodeHeartbeat(am.getApplicationAttemptId(), 1,
ContainerState.COMPLETE);
rm1.waitForState(am.getApplicationAttemptId(), RMAppAttemptState.FAILED);
rm1.waitForState(app1.getApplicationId(), RMAppState.ACCEPTED);
if (i < 3) {
Thread.sleep(window + 500);
am = MockRM.launchAndRegisterAM(app1, rm1, nm1);
}
}
GenericTestUtils.waitFor(() -> !memStore.getState().getApplicationState()
.get(app1.getApplicationId()).attempts.containsKey(attempt1),
100, 10000);
MockRM rm2 = new MockRM(getConf(), memStore);
rm2.start();
nm1.setResourceTrackerService(rm2.getResourceTrackerService());
nm1.registerNode(Lists.newArrayList(TestRMRestart.createNMContainerStatus(
attempt1, 2, ContainerState.RUNNING)), null);
rm2.waitForState(nm1, containerId2, RMContainerState.RUNNING);
assertFalse(rm2.getRMContext().getRMApps().get(app1.getApplicationId())
.getAppAttempts().containsKey(attempt1));
assertEquals(containerId2, rm2.getClientRMService().getContainerReport(
GetContainerReportRequest.newInstance(containerId2))
.getContainerReport().getContainerId());
rm2.stop();
rm1.stop();
}
```
</details>
If you keep the mocked test too, maybe rename it to something like `testGetContainerReportWhenAttemptMissingWithKeepContainers`.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException, IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, true);
+
+ GetContainerReportResponse response = rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId));
+
+ assertEquals(containerId, response.getContainerReport().getContainerId());
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingAttemptWithoutContainerTransfer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, false, false, true);
+
+ assertThrows(ApplicationAttemptNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingRetainedContainer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, false);
+
+ assertThrows(ContainerNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ private ClientRMService createRMServiceForContainerReport(
+ ContainerId containerId, boolean keepContainersAcrossAttempts,
+ boolean includeAttempt, boolean includeContainer) {
+ ApplicationAttemptId attemptId = containerId.getApplicationAttemptId();
+ ApplicationId applicationId = attemptId.getApplicationId();
+ ResourceScheduler scheduler = mock(ResourceScheduler.class);
+ RMContext rmContext = mock(RMContext.class);
+ RMApp application = mock(RMApp.class);
+
+ ConcurrentHashMap<ApplicationId, RMApp> applications =
+ new ConcurrentHashMap<>();
+ applications.put(applicationId, application);
+ when(rmContext.getRMApps()).thenReturn(applications);
+ when(rmContext.getScheduler()).thenReturn(scheduler);
+ when(application.getApplicationId()).thenReturn(applicationId);
+ when(application.getUser()).thenReturn("test-user");
+
+ ApplicationSubmissionContext submissionContext =
+ mock(ApplicationSubmissionContext.class);
+ when(submissionContext.getKeepContainersAcrossApplicationAttempts())
+ .thenReturn(keepContainersAcrossAttempts);
+ when(application.getApplicationSubmissionContext())
+ .thenReturn(submissionContext);
+
+ Map<ApplicationAttemptId, RMAppAttempt> attempts = new HashMap<>();
+ if (includeAttempt) {
Review Comment:
Every test passes `includeAttempt = false`, so this branch never runs. Could we add the normal case too, with keepContainers on and the attempt still present (`true, true, true`)? That's what most keepContainers apps actually hit.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException, IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, true);
+
+ GetContainerReportResponse response = rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId));
+
+ assertEquals(containerId, response.getContainerReport().getContainerId());
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingAttemptWithoutContainerTransfer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, false, false, true);
+
+ assertThrows(ApplicationAttemptNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingRetainedContainer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, false);
+
+ assertThrows(ContainerNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ private ClientRMService createRMServiceForContainerReport(
+ ContainerId containerId, boolean keepContainersAcrossAttempts,
+ boolean includeAttempt, boolean includeContainer) {
+ ApplicationAttemptId attemptId = containerId.getApplicationAttemptId();
+ ApplicationId applicationId = attemptId.getApplicationId();
+ ResourceScheduler scheduler = mock(ResourceScheduler.class);
+ RMContext rmContext = mock(RMContext.class);
+ RMApp application = mock(RMApp.class);
+
+ ConcurrentHashMap<ApplicationId, RMApp> applications =
+ new ConcurrentHashMap<>();
+ applications.put(applicationId, application);
+ when(rmContext.getRMApps()).thenReturn(applications);
+ when(rmContext.getScheduler()).thenReturn(scheduler);
+ when(application.getApplicationId()).thenReturn(applicationId);
+ when(application.getUser()).thenReturn("test-user");
+
+ ApplicationSubmissionContext submissionContext =
+ mock(ApplicationSubmissionContext.class);
+ when(submissionContext.getKeepContainersAcrossApplicationAttempts())
+ .thenReturn(keepContainersAcrossAttempts);
+ when(application.getApplicationSubmissionContext())
+ .thenReturn(submissionContext);
+
+ Map<ApplicationAttemptId, RMAppAttempt> attempts = new HashMap<>();
+ if (includeAttempt) {
+ attempts.put(attemptId, mock(RMAppAttempt.class));
+ }
+ when(application.getAppAttempts()).thenReturn(attempts);
+
+ if (includeContainer) {
+ RMContainer rmContainer = mock(RMContainer.class);
+ ContainerReport containerReport = mock(ContainerReport.class);
+ when(containerReport.getContainerId()).thenReturn(containerId);
+ when(rmContainer.createContainerReport()).thenReturn(containerReport);
+ when(scheduler.getRMContainer(containerId)).thenReturn(rmContainer);
+ }
+
+ ApplicationACLsManager applicationACLsManager =
+ mock(ApplicationACLsManager.class);
+ QueueACLsManager queueACLsManager = mock(QueueACLsManager.class);
+ when(queueACLsManager.checkAccess(any(UserGroupInformation.class),
Review Comment:
Access is always granted here, so nothing checks that the new branch still rejects a user without access. Could the helper take a flag to deny access, plus a test that a keepContainers app still throws the "does not have privilege" error?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClientRMService.java:
##########
@@ -512,11 +512,14 @@ public GetContainerReportResponse getContainerReport(
ApplicationAccessType.VIEW_APP, application);
GetContainerReportResponse response = null;
if (allowAccess) {
- RMAppAttempt appAttempt = application.getAppAttempts().get(appAttemptId);
- if (appAttempt == null) {
- throw new ApplicationAttemptNotFoundException(
- "ApplicationAttempt with id '" + appAttemptId +
- "' doesn't exist in RM.");
+ if (!application.getApplicationSubmissionContext()
Review Comment:
For keepContainers apps, this changes which error callers see. Previously, a missing attempt always gave `ApplicationAttemptNotFoundException`. Now it gives `ContainerNotFoundException` when the container is gone, including for an attempt id that never existed.
I think that's an improvement. `YarnClientImpl#getContainerReport` only falls back to the history server on `ApplicationNotFoundException` or `ContainerNotFoundException`, so finished containers of old attempts can now be found there. But it's a behavior change, so it's worth saying in the description.
Related: `testGetContainerReportRejectsMissingRetainedContainer` (L569) is described as "preserving" `ContainerNotFoundException`, but on trunk the same input throws `ApplicationAttemptNotFoundException`. The test checks new behavior, so the name and description should say so.
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException, IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, true);
+
+ GetContainerReportResponse response = rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId));
+
+ assertEquals(containerId, response.getContainerReport().getContainerId());
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingAttemptWithoutContainerTransfer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, false, false, true);
+
+ assertThrows(ApplicationAttemptNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingRetainedContainer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, false);
+
+ assertThrows(ContainerNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ private ClientRMService createRMServiceForContainerReport(
Review Comment:
Optional: three positional booleans are hard to read at the call sites, and the three tests repeat the same id setup. A `@ParameterizedTest` with `@CsvSource` (or named constants) would make the cases clearer and make it easy to add the two from the comments above.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "gerashegalov (via GitHub) via common-issues" <co...@hadoop.apache.org>.
gerashegalov merged PR #8854:
URL: https://github.com/apache/hadoop/pull/8854
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on code in PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#discussion_r4511385446
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ClientRMService.java:
##########
@@ -512,11 +512,14 @@ public GetContainerReportResponse getContainerReport(
ApplicationAccessType.VIEW_APP, application);
GetContainerReportResponse response = null;
if (allowAccess) {
- RMAppAttempt appAttempt = application.getAppAttempts().get(appAttemptId);
- if (appAttempt == null) {
- throw new ApplicationAttemptNotFoundException(
- "ApplicationAttempt with id '" + appAttemptId +
- "' doesn't exist in RM.");
+ if (!application.getApplicationSubmissionContext()
Review Comment:
Thanks for pointing out the behavior change. I've updated the PR and Jira descriptions to clarify that a missing attempt and container now produce ContainerNotFoundException instead of ApplicationAttemptNotFoundException. I've also renamed the test so it no longer implies that the previous behavior is preserved.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on code in PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#discussion_r4511370404
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException, IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, true);
+
+ GetContainerReportResponse response = rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId));
+
+ assertEquals(containerId, response.getContainerReport().getContainerId());
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingAttemptWithoutContainerTransfer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, false, false, true);
+
+ assertThrows(ApplicationAttemptNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ @Test
+ public void testGetContainerReportRejectsMissingRetainedContainer()
+ throws Exception {
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(
+ ApplicationId.newInstance(123456, 1), 1);
+ ContainerId containerId = ContainerId.newContainerId(attemptId, 1);
+ ClientRMService rmService = createRMServiceForContainerReport(
+ containerId, true, false, false);
+
+ assertThrows(ContainerNotFoundException.class,
+ () -> rmService.getContainerReport(
+ GetContainerReportRequest.newInstance(containerId)));
+ }
+
+ private ClientRMService createRMServiceForContainerReport(
+ ContainerId containerId, boolean keepContainersAcrossAttempts,
+ boolean includeAttempt, boolean includeContainer) {
+ ApplicationAttemptId attemptId = containerId.getApplicationAttemptId();
+ ApplicationId applicationId = attemptId.getApplicationId();
+ ResourceScheduler scheduler = mock(ResourceScheduler.class);
+ RMContext rmContext = mock(RMContext.class);
+ RMApp application = mock(RMApp.class);
+
+ ConcurrentHashMap<ApplicationId, RMApp> applications =
+ new ConcurrentHashMap<>();
+ applications.put(applicationId, application);
+ when(rmContext.getRMApps()).thenReturn(applications);
+ when(rmContext.getScheduler()).thenReturn(scheduler);
+ when(application.getApplicationId()).thenReturn(applicationId);
+ when(application.getUser()).thenReturn("test-user");
+
+ ApplicationSubmissionContext submissionContext =
+ mock(ApplicationSubmissionContext.class);
+ when(submissionContext.getKeepContainersAcrossApplicationAttempts())
+ .thenReturn(keepContainersAcrossAttempts);
+ when(application.getApplicationSubmissionContext())
+ .thenReturn(submissionContext);
+
+ Map<ApplicationAttemptId, RMAppAttempt> attempts = new HashMap<>();
+ if (includeAttempt) {
Review Comment:
Agreed, the normal case should be covered as well. I've added a test with keepContainers enabled and both the attempt and container present, verifying that the container report is returned successfully.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6652276225
Hi @gerashegalov, could you review this container-report lookup fix for containers retained across application attempts when convenient? It includes regression tests, and CI is green. Thanks!
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6785646174
Thanks @gerashegalov for the approval and helpful guidance!
Hi @ayushtkn and @slfan1989, could either of you provide a second review when convenient? This fixes `getContainerReport` for retained containers whose original application attempt is missing after RM recovery. Recovery tests cover both Capacity and Fair schedulers, and Jenkins/Yetus have passed. Thanks!
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "gerashegalov (via GitHub) via common-issues" <co...@hadoop.apache.org>.
gerashegalov commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6829419374
Thanks @luoyuan3471 for contributing this fix, and @ayushtkn for an additional review
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "hadoop-yetus (via GitHub)" <gi...@apache.org>.
hadoop-yetus commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6341508907
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|:----:|----------:|--------:|:--------:|:-------:|
| +0 :ok: | reexec | 22m 15s | | Docker mode activated. |
|||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files found. |
| +0 :ok: | codespell | 0m 1s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 1s | | detect-secrets was not available. |
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain any @author tags. |
| +1 :green_heart: | test4tests | 0m 0s | | The patch appears to include 1 new or modified test files. |
|||| _ trunk Compile Tests _ |
| +1 :green_heart: | mvninstall | 49m 44s | | trunk passed |
| +1 :green_heart: | compile | 1m 29s | | trunk passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | compile | 1m 29s | | trunk passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | checkstyle | 1m 26s | | trunk passed |
| +1 :green_heart: | mvnsite | 1m 33s | | trunk passed |
| +1 :green_heart: | javadoc | 1m 17s | | trunk passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javadoc | 1m 15s | | trunk passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 2m 42s | | trunk passed |
| +1 :green_heart: | shadedclient | 36m 20s | | branch has no errors when building and testing our client artifacts. |
|||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 1m 2s | | the patch passed |
| +1 :green_heart: | compile | 0m 58s | | the patch passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javac | 0m 58s | | the patch passed |
| +1 :green_heart: | compile | 1m 1s | | the patch passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | javac | 1m 1s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks issues. |
| +1 :green_heart: | checkstyle | 0m 52s | | the patch passed |
| +1 :green_heart: | mvnsite | 1m 4s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 46s | | the patch passed with JDK Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu |
| +1 :green_heart: | javadoc | 0m 46s | | the patch passed with JDK Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| +1 :green_heart: | spotbugs | 2m 23s | | the patch passed |
| +1 :green_heart: | shadedclient | 35m 59s | | patch has no errors when building and testing our client artifacts. |
|||| _ Other Tests _ |
| +1 :green_heart: | unit | 116m 52s | | hadoop-yarn-server-resourcemanager in the patch passed. |
| +1 :green_heart: | asflicense | 0m 40s | | The patch does not generate ASF License warnings. |
| | | 282m 0s | | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.56 ServerAPI=1.56 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8854/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hadoop/pull/8854 |
| Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets |
| uname | Linux aa423a7cd381 5.15.0-185-generic #195-Ubuntu SMP Fri Jun 19 17:11:50 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/bin/hadoop.sh |
| git revision | trunk / f9f47e4f0957fa2e2bf87d9025834be349804676 |
| Default Java | Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| Multi-JDK versions | /usr/lib/jvm/java-21-openjdk-amd64:Ubuntu-21.0.12.1+1-1-24.04.4-Ubuntu /usr/lib/jvm/java-17-openjdk-amd64:Ubuntu-17.0.20.1+1-1-24.04-Ubuntu |
| Test Results | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8854/1/testReport/ |
| Max. process+thread count | 858 (vs. ulimit of 10000) |
| modules | C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager |
| Console output | https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-8854/1/console |
| versions | git=2.43.0 maven=3.9.15 spotbugs=4.9.7 |
| Powered by | Apache Yetus 0.14.1 https://yetus.apache.org |
This message was automatically generated.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6841783087
Thanks @gerashegalov for the thorough review, helpful test sketch, and merging! Thanks @ayushtkn for the additional review. I really appreciate your time and guidance!
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on code in PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#discussion_r4511419167
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestClientRMService.java:
##########
@@ -533,6 +536,96 @@ public void testGetContainerReport() throws YarnException, IOException {
}
}
+ @Test
+ public void testGetContainerReportForContainerFromPreviousAttempt()
Review Comment:
Thanks for the test sketch. I've followed the suggested approach and added an end-to-end recovery test in TestAMRestart for both Capacity and Fair schedulers, including checks before and after RM recovery. I also renamed the mocked test as suggested.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org
Re: [PR] YARN-12039. Fix container report lookup for retained containers [hadoop]
Posted by "luoyuan3471 (via GitHub) via common-issues" <co...@hadoop.apache.org>.
luoyuan3471 commented on PR #8854:
URL: https://github.com/apache/hadoop/pull/8854#issuecomment-6737163706
Thanks @gerashegalov for the thoughtful review and helpful test sketch! I've added recovery coverage for both schedulers, and updated the PR/Jira descriptions. I really appreciate your time and guidance. Could you take another look when convenient?
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org