MSQ: Controller checker should check for "closed" only. (#16161)

* MSQ: Controller checker should check for "closed" only.

Currently, the worker's controller checker will exit the worker if
the controller location is "closed" (no longer running) or if its location
is empty (i.e. location unknown).

This patch changes to only exit on "closed". We shouldn't exit on empty
location, because that may happen if the Overlord is slow to acknowledge the
location of a task.

* Fix test.
This commit is contained in:
Gian Merlino 2024-03-19 19:25:48 -07:00 committed by GitHub
parent e7cf8299ce
commit 2b23d0b5b5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 5 additions and 16 deletions

View File

@ -190,7 +190,9 @@ public class IndexerWorkerContext implements WorkerContext
break;
}
if (controllerLocations.isClosed() || controllerLocations.getLocations().isEmpty()) {
// Note: don't exit on empty location, because that may happen if the Overlord is slow to acknowledge the
// location of a task. Only exit on "closed", because that happens only if the task is really no longer running.
if (controllerLocations.isClosed()) {
log.warn(
"Periodic fetch of controller location returned [%s]. Worker task [%s] will exit.",
controllerLocations,

View File

@ -54,20 +54,6 @@ public class IndexerWorkerContextTest
);
}
@Test
public void testControllerCheckerRunnableExitsWhenEmptyStatus()
{
final ServiceLocator controllerLocatorMock = Mockito.mock(ServiceLocator.class);
Mockito.when(controllerLocatorMock.locate())
.thenReturn(Futures.immediateFuture(ServiceLocations.forLocations(Collections.emptySet())));
final Worker workerMock = Mockito.mock(Worker.class);
indexerWorkerContext.controllerCheckerRunnable(controllerLocatorMock, workerMock);
Mockito.verify(controllerLocatorMock, Mockito.times(1)).locate();
Mockito.verify(workerMock, Mockito.times(1)).controllerFailed();
}
@Test
public void testControllerCheckerRunnableExitsOnlyWhenClosedStatus()
{
@ -76,12 +62,13 @@ public class IndexerWorkerContextTest
.thenReturn(Futures.immediateFuture(ServiceLocations.forLocation(new ServiceLocation("h", 1, -1, "/"))))
// Done to check the behavior of the runnable, the situation of exiting after success might not occur actually
.thenReturn(Futures.immediateFuture(ServiceLocations.forLocation(new ServiceLocation("h", 1, -1, "/"))))
.thenReturn(Futures.immediateFuture(ServiceLocations.forLocations(Collections.emptySet())))
.thenReturn(Futures.immediateFuture(ServiceLocations.closed()));
final Worker workerMock = Mockito.mock(Worker.class);
indexerWorkerContext.controllerCheckerRunnable(controllerLocatorMock, workerMock);
Mockito.verify(controllerLocatorMock, Mockito.times(3)).locate();
Mockito.verify(controllerLocatorMock, Mockito.times(4)).locate();
Mockito.verify(workerMock, Mockito.times(1)).controllerFailed();
}
}