From f1c0b9585520cedb76da6a4dbd08330ad93650bc Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Tue, 16 Jun 2020 10:50:26 +0200 Subject: Avoid busy wait --- .../admin/nodeagent/NodeAgentContextManager.java | 9 ++++++-- .../nodeagent/NodeAgentContextManagerTest.java | 24 ++++++++-------------- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManager.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManager.java index 016d60b1354..ad67e13e044 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManager.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManager.java @@ -64,6 +64,7 @@ public class NodeAgentContextManager implements NodeAgentContextSupplier, NodeAg synchronized (monitor) { nextContext = null; // Reset any previous context and wait for the next one isWaitingForNextContext = true; + monitor.notify(); Duration untilNextContext = Duration.ZERO; while (setAndGetIsFrozen(wantFrozen) || nextContext == null || @@ -110,9 +111,13 @@ public class NodeAgentContextManager implements NodeAgentContextSupplier, NodeAg } /** FOR TESTING ONLY */ - boolean isWaitingForNextContext() { + void waitUntilWaitingForNextContext() { synchronized (monitor) { - return isWaitingForNextContext; + while (!isWaitingForNextContext) { + try { + monitor.wait(); + } catch (InterruptedException ignored) { } + } } } } \ No newline at end of file diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java index 240a29080cc..a082addd775 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java @@ -33,7 +33,7 @@ public class NodeAgentContextManagerTest { assertSame(initialContext, manager.currentContext()); AsyncExecutor async = new AsyncExecutor<>(manager::nextContext); - waitUntilWaitingForNextContext(); + manager.waitUntilWaitingForNextContext(); assertFalse(async.isCompleted()); NodeAgentContext context2 = generateContext(); @@ -46,7 +46,7 @@ public class NodeAgentContextManagerTest { @Test(timeout = TIMEOUT) public void returns_no_earlier_than_at_given_time() { AsyncExecutor async = new AsyncExecutor<>(manager::nextContext); - waitUntilWaitingForNextContext(); + manager.waitUntilWaitingForNextContext(); NodeAgentContext context1 = generateContext(); Instant returnAt = clock.instant().plusMillis(500); @@ -61,7 +61,7 @@ public class NodeAgentContextManagerTest { @Test(timeout = TIMEOUT) public void blocks_in_nextContext_until_one_is_scheduled() { AsyncExecutor async = new AsyncExecutor<>(manager::nextContext); - waitUntilWaitingForNextContext(); + manager.waitUntilWaitingForNextContext(); assertFalse(async.isCompleted()); NodeAgentContext context1 = generateContext(); @@ -75,7 +75,7 @@ public class NodeAgentContextManagerTest { @Test(timeout = TIMEOUT) public void blocks_in_nextContext_until_interrupt() { AsyncExecutor async = new AsyncExecutor<>(manager::nextContext); - waitUntilWaitingForNextContext(); + manager.waitUntilWaitingForNextContext(); assertFalse(async.isCompleted()); manager.interrupt(); @@ -92,7 +92,7 @@ public class NodeAgentContextManagerTest { // Generate new context and get it from the supplier, this completes the unfreeze NodeAgentContext context1 = generateContext(); AsyncExecutor async = new AsyncExecutor<>(manager::nextContext); - waitUntilWaitingForNextContext(); + manager.waitUntilWaitingForNextContext(); manager.scheduleTickWith(context1, clock.instant()); assertSame(context1, async.awaitResult().response.get()); @@ -116,7 +116,7 @@ public class NodeAgentContextManagerTest { Thread.sleep(200); // Simulate running NodeAgent::converge return context; }); - waitUntilWaitingForNextContext(); + manager.waitUntilWaitingForNextContext(); NodeAgentContext context1 = generateContext(); manager.scheduleTickWith(context1, clock.instant()); @@ -130,9 +130,9 @@ public class NodeAgentContextManagerTest { assertFalse(asyncScheduler.isCompleted()); // Still waiting for consumer to converge to frozen AsyncExecutor asyncConsumer2 = new AsyncExecutor<>(manager::nextContext); - waitUntilWaitingForNextContext(); + manager.waitUntilWaitingForNextContext(); assertFalse(asyncConsumer2.isCompleted()); // Waiting for next context - assertTrue(asyncScheduler.isCompleted()); // While consumer is waiting, it has converged to frozen + asyncScheduler.awaitResult(); // We should be able to converge to frozen now // Interrupt manager to end asyncConsumer2 manager.interrupt(); @@ -141,14 +141,6 @@ public class NodeAgentContextManagerTest { assertEquals(Optional.of(true), asyncScheduler.response); } - private void waitUntilWaitingForNextContext() { - while (!manager.isWaitingForNextContext()) { - try { - Thread.sleep(10); - } catch (InterruptedException ignored) { } - } - } - private static NodeAgentContext generateContext() { return new NodeAgentContextImpl.Builder("container-123.domain.tld").build(); } -- cgit v1.2.3