gerashegalov commented on code in PR #8755:
URL: https://github.com/apache/hadoop/pull/8755#discussion_r4156659933
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/test/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/TestResourceLocalizationService.java:
##########
@@ -2650,6 +2654,71 @@ public void
testParallelDownloadAttemptsForPublicResource() throws Exception {
}
+ /**
+ * The Public Localizer must stay up when it dequeues a completed download it
+ * has no record of. Before YARN-11993 run() returned on that path, so the
+ * finally block shut the download pool down and every later public
+ * localization on the node was rejected until the NodeManager restarted.
+ */
+ @Test
+ @Timeout(value = 30)
+ public void testPublicLocalizerSurvivesUnknownResource() throws Exception {
+ conf.setStrings(YarnConfiguration.NM_LOCAL_DIRS,
+ lfs.makeQualified(new Path(basedir, "0")).toString());
+
+ DrainDispatcher dispatcher = new DrainDispatcher();
+ dispatcher.init(conf);
+ dispatcher.start();
+
+ // Nothing is localized here, so the dirs handler is never asked for a
+ // path; mocking it keeps the test off the disk.
+ LocalDirsHandlerService mockDirsHandler =
+ mock(LocalDirsHandlerService.class);
+
+ ResourceLocalizationService service =
+ new ResourceLocalizationService(dispatcher,
+ mock(ContainerExecutor.class), mock(DeletionService.class),
+ mockDirsHandler, nmContext, metrics);
+ dispatcher.register(LocalizationEventType.class, service);
+ service.init(conf);
+
+ PublicLocalizer publicLocalizer = service.getPublicLocalizer();
+ try {
+ publicLocalizer.start();
+
+ // Submit straight to the completion queue so the Future is never
+ // recorded in pending. That is exactly the state in which
+ // pending.remove(completed) returns null.
+ final CountDownLatch downloaded = new CountDownLatch(1);
+ final Path unknown = new Path(basedir, "unknown");
+ publicLocalizer.queue.submit(() -> {
+ downloaded.countDown();
+ return unknown;
+ });
+ assertTrue(downloaded.await(10, TimeUnit.SECONDS),
+ "public download never ran");
+ assertEquals(0, publicLocalizer.pending.size());
+
+ // The localizer should log the unknown resource and carry on. If it
+ // exits instead, run()'s finally block shuts the download pool down.
+ try {
+ GenericTestUtils.waitFor(() -> !publicLocalizer.isAlive()
Review Comment:
`downloaded` `countDown()` runs inside the download task, before that task
returns. Could we wait for the “Localized unknown resource” log before checking
liveness? The latch fires before the result is queued, so this test could pass
without the localizer handling it
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
