From 394573780b3ccdd66ca260cf4a770c9f0a9f58f4 Mon Sep 17 00:00:00 2001 From: Carlo Curino Date: Fri, 11 Aug 2017 16:58:04 -0700 Subject: [PATCH] YARN-6687. Validate that the duration of the periodic reservation is less than the periodicity. (subru via curino) (cherry picked from commit 28d97b79b69bb2be02d9320105e155eeed6f9e78) --- .../ReservationInputValidator.java | 18 +++- .../TestReservationInputValidator.java | 93 +++++++++++++++++++ 2 files changed, 106 insertions(+), 5 deletions(-) diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/ReservationInputValidator.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/ReservationInputValidator.java index 0e9a825de07..027d066ab28 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/ReservationInputValidator.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/ReservationInputValidator.java @@ -129,11 +129,12 @@ public class ReservationInputValidator { Resources.multiply(rr.getCapability(), rr.getConcurrency())); } // verify the allocation is possible (skip for ANY) - if (contract.getDeadline() - contract.getArrival() < minDuration + long duration = contract.getDeadline() - contract.getArrival(); + if (duration < minDuration && type != ReservationRequestInterpreter.R_ANY) { message = "The time difference (" - + (contract.getDeadline() - contract.getArrival()) + + (duration) + ") between arrival (" + contract.getArrival() + ") " + "and deadline (" + contract.getDeadline() + ") must " + " be greater or equal to the minimum resource duration (" @@ -158,15 +159,22 @@ public class ReservationInputValidator { // check that the recurrence is a positive long value. String recurrenceExpression = contract.getRecurrenceExpression(); try { - Long recurrence = Long.parseLong(recurrenceExpression); + long recurrence = Long.parseLong(recurrenceExpression); if (recurrence < 0) { message = "Negative Period : " + recurrenceExpression + ". Please try" - + " again with a non-negative long value as period"; + + " again with a non-negative long value as period."; + throw RPCUtil.getRemoteException(message); + } + // verify duration is less than recurrence for periodic reservations + if (recurrence > 0 && duration > recurrence) { + message = "Duration of the requested reservation: " + duration + + " is greater than the recurrence: " + recurrence + + ". Please try again with a smaller duration."; throw RPCUtil.getRemoteException(message); } } catch (NumberFormatException e) { message = "Invalid period " + recurrenceExpression + ". Please try" - + " again with a non-negative long value as period"; + + " again with a non-negative long value as period."; throw RPCUtil.getRemoteException(message); } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/TestReservationInputValidator.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/TestReservationInputValidator.java index 2917cd948ae..90a681d2e9e 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/TestReservationInputValidator.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/reservation/TestReservationInputValidator.java @@ -303,6 +303,7 @@ public class TestReservationInputValidator { @Test public void testSubmitReservationInvalidRecurrenceExpression() { + // first check recurrence expression ReservationSubmissionRequest request = createSimpleReservationSubmissionRequest(1, 1, 1, 5, 3, "123abc"); plan = null; @@ -318,6 +319,23 @@ public class TestReservationInputValidator { .startsWith("Invalid period ")); LOG.info(message); } + + // now check duration + request = + createSimpleReservationSubmissionRequest(1, 1, 1, 50, 3, "10"); + plan = null; + try { + plan = + rrValidator.validateReservationSubmissionRequest(rSystem, request, + ReservationSystemTestUtil.getNewReservationId()); + Assert.fail(); + } catch (YarnException e) { + Assert.assertNull(plan); + String message = e.getMessage(); + Assert.assertTrue(message + .startsWith("Duration of the requested reservation:")); + LOG.info(message); + } } @Test @@ -499,6 +517,73 @@ public class TestReservationInputValidator { } } + @Test + public void testUpdateReservationValidRecurrenceExpression() { + ReservationUpdateRequest request = + createSimpleReservationUpdateRequest(1, 1, 1, 5, 3, "600000"); + plan = null; + try { + plan = + rrValidator.validateReservationUpdateRequest(rSystem, request); + } catch (YarnException e) { + Assert.fail(e.getMessage()); + } + Assert.assertNotNull(plan); + } + + @Test + public void testUpdateReservationNegativeRecurrenceExpression() { + ReservationUpdateRequest request = + createSimpleReservationUpdateRequest(1, 1, 1, 5, 3, "-1234"); + plan = null; + try { + plan = + rrValidator.validateReservationUpdateRequest(rSystem, request); + Assert.fail(); + } catch (YarnException e) { + Assert.assertNull(plan); + String message = e.getMessage(); + Assert.assertTrue(message + .startsWith("Negative Period : ")); + LOG.info(message); + } + } + + @Test + public void testUpdateReservationInvalidRecurrenceExpression() { + // first check recurrence expression + ReservationUpdateRequest request = + createSimpleReservationUpdateRequest(1, 1, 1, 5, 3, "123abc"); + plan = null; + try { + plan = + rrValidator.validateReservationUpdateRequest(rSystem, request); + Assert.fail(); + } catch (YarnException e) { + Assert.assertNull(plan); + String message = e.getMessage(); + Assert.assertTrue(message + .startsWith("Invalid period ")); + LOG.info(message); + } + + // now check duration + request = + createSimpleReservationUpdateRequest(1, 1, 1, 50, 3, "10"); + plan = null; + try { + plan = + rrValidator.validateReservationUpdateRequest(rSystem, request); + Assert.fail(); + } catch (YarnException e) { + Assert.assertNull(plan); + String message = e.getMessage(); + Assert.assertTrue(message + .startsWith("Duration of the requested reservation:")); + LOG.info(message); + } + } + @Test public void testDeleteReservationNormal() { ReservationDeleteRequest request = new ReservationDeleteRequestPBImpl(); @@ -710,11 +795,19 @@ public class TestReservationInputValidator { private ReservationUpdateRequest createSimpleReservationUpdateRequest( int numRequests, int numContainers, long arrival, long deadline, long duration) { + return createSimpleReservationUpdateRequest(numRequests, numContainers, + arrival, deadline, duration, "0"); + } + + private ReservationUpdateRequest createSimpleReservationUpdateRequest( + int numRequests, int numContainers, long arrival, long deadline, + long duration, String recurrence) { // create a request with a single atomic ask ReservationUpdateRequest request = new ReservationUpdateRequestPBImpl(); ReservationDefinition rDef = new ReservationDefinitionPBImpl(); rDef.setArrival(arrival); rDef.setDeadline(deadline); + rDef.setRecurrenceExpression(recurrence); if (numRequests > 0) { ReservationRequests reqs = new ReservationRequestsPBImpl(); rDef.setReservationRequests(reqs);