You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by zm...@apache.org on 2017/06/12 20:14:35 UTC
aurora git commit: Remove restriction on task id length.
Repository: aurora
Updated Branches:
refs/heads/master 06518c86b -> 40d9d4dbe
Remove restriction on task id length.
To work around an old Mesos bug (MESOS-691) we would reject jobs that resulted
in Mesos task ids longer than 255 characters. This is because Mesos used to use
the task id to generate the cgroup path. Now Mesos uses it's own id, we no
longer need to work around this bug.
This removes the restriction in the API layer. This is useful because some users
may have very long role and service names that caused task ids to go over this
limit.
Bugs closed: AURORA-1897
Reviewed at https://reviews.apache.org/r/59957/
Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/40d9d4db
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/40d9d4db
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/40d9d4db
Branch: refs/heads/master
Commit: 40d9d4dbec86cb4a17e281dc10ede25e83613eff
Parents: 06518c8
Author: Zameer Manji <zm...@apache.org>
Authored: Mon Jun 12 13:14:18 2017 -0700
Committer: Zameer Manji <zm...@apache.org>
Committed: Mon Jun 12 13:14:18 2017 -0700
----------------------------------------------------------------------
.../thrift/SchedulerThriftInterface.java | 22 -----
.../thrift/SchedulerThriftInterfaceTest.java | 99 --------------------
2 files changed, 121 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/aurora/blob/40d9d4db/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
index 059fbb8..7ea6fc4 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -70,7 +70,6 @@ import org.apache.aurora.gen.ScheduleStatus;
import org.apache.aurora.gen.StartJobUpdateResult;
import org.apache.aurora.gen.StartMaintenanceResult;
import org.apache.aurora.gen.TaskQuery;
-import org.apache.aurora.scheduler.TaskIdGenerator;
import org.apache.aurora.scheduler.base.JobKeys;
import org.apache.aurora.scheduler.base.Numbers;
import org.apache.aurora.scheduler.base.Query;
@@ -155,11 +154,6 @@ import static org.apache.aurora.scheduler.thrift.Responses.ok;
@DecoratedThrift
class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
- // This number is derived from the maximum file name length limit on most UNIX systems, less
- // the number of characters we've observed being added by mesos for the executor ID, prefix, and
- // delimiters.
- @VisibleForTesting
- static final int MAX_TASK_ID_LENGTH = 255 - 90;
@VisibleForTesting
static final String STAT_PREFIX = "thrift_workload_";
@VisibleForTesting
@@ -197,7 +191,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
private final CronJobManager cronJobManager;
private final QuotaManager quotaManager;
private final StateManager stateManager;
- private final TaskIdGenerator taskIdGenerator;
private final UUIDGenerator uuidGenerator;
private final JobUpdateController jobUpdateController;
private final ReadOnlyScheduler.Iface readOnlyScheduler;
@@ -228,7 +221,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
MaintenanceController maintenance,
QuotaManager quotaManager,
StateManager stateManager,
- TaskIdGenerator taskIdGenerator,
UUIDGenerator uuidGenerator,
JobUpdateController jobUpdateController,
ReadOnlyScheduler.Iface readOnlyScheduler,
@@ -246,7 +238,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
this.cronJobManager = requireNonNull(cronJobManager);
this.quotaManager = requireNonNull(quotaManager);
this.stateManager = requireNonNull(stateManager);
- this.taskIdGenerator = requireNonNull(taskIdGenerator);
this.uuidGenerator = requireNonNull(uuidGenerator);
this.jobUpdateController = requireNonNull(jobUpdateController);
this.readOnlyScheduler = requireNonNull(readOnlyScheduler);
@@ -293,7 +284,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
int count = sanitized.getJobConfig().getInstanceCount();
validateTaskLimits(
- template,
count,
quotaManager.checkInstanceAddition(template, count, storeProvider));
@@ -349,11 +339,9 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
try {
lockManager.assertNotLocked(ILockKey.build(LockKey.job(jobKey.newBuilder())));
- ITaskConfig template = sanitized.getJobConfig().getTaskConfig();
int count = sanitized.getJobConfig().getInstanceCount();
validateTaskLimits(
- template,
count,
quotaManager.checkCronUpdate(sanitized.getJobConfig(), storeProvider));
@@ -863,7 +851,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
ITaskConfig task = templateTask.get().getAssignedTask().getTask();
validateTaskLimits(
- task,
Iterables.size(currentTasks) + instanceIds.size(),
quotaManager.checkInstanceAddition(task, instanceIds.size(), storeProvider));
@@ -891,7 +878,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
}
private void validateTaskLimits(
- ITaskConfig task,
int totalInstances,
QuotaCheckResult quotaCheck) throws TaskValidationException {
@@ -900,13 +886,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
"Instance count must be between 1 and %d inclusive.", thresholds.getMaxTasksPerJob()));
}
- // TODO(maximk): This is a short-term hack to stop the bleeding from
- // https://issues.apache.org/jira/browse/MESOS-691
- if (taskIdGenerator.generate(task, totalInstances).length() > MAX_TASK_ID_LENGTH) {
- throw new TaskValidationException(
- "Task ID is too long, please shorten your role or job name.");
- }
-
if (quotaCheck.getResult() == INSUFFICIENT_QUOTA) {
throw new TaskValidationException("Insufficient resource quota: "
+ quotaCheck.getDetails().or(""));
@@ -1026,7 +1005,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
Response response = empty();
try {
validateTaskLimits(
- request.getTaskConfig(),
request.getInstanceCount(),
quotaManager.checkJobUpdate(update, storeProvider));
http://git-wip-us.apache.org/repos/asf/aurora/blob/40d9d4db/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
index 016859c..9c44668 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -76,7 +76,6 @@ import org.apache.aurora.gen.TaskConstraint;
import org.apache.aurora.gen.TaskQuery;
import org.apache.aurora.gen.ValueConstraint;
import org.apache.aurora.gen.apiConstants;
-import org.apache.aurora.scheduler.TaskIdGenerator;
import org.apache.aurora.scheduler.base.JobKeys;
import org.apache.aurora.scheduler.base.Query;
import org.apache.aurora.scheduler.base.TaskTestUtil;
@@ -164,7 +163,6 @@ import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.DRAIN_
import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.END_MAINTENANCE;
import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.KILL_TASKS;
import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.MAINTENANCE_STATUS;
-import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.MAX_TASK_ID_LENGTH;
import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NOOP_JOB_UPDATE_MESSAGE;
import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NO_CRON;
import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.RESTART_SHARDS;
@@ -202,7 +200,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
private CronJobManager cronJobManager;
private QuotaManager quotaManager;
private StateManager stateManager;
- private TaskIdGenerator taskIdGenerator;
private UUIDGenerator uuidGenerator;
private JobUpdateController jobUpdateController;
private ReadOnlyScheduler.Iface readOnlyScheduler;
@@ -221,7 +218,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
cronJobManager = createMock(CronJobManager.class);
quotaManager = createMock(QuotaManager.class);
stateManager = createMock(StateManager.class);
- taskIdGenerator = createMock(TaskIdGenerator.class);
uuidGenerator = createMock(UUIDGenerator.class);
jobUpdateController = createMock(JobUpdateController.class);
readOnlyScheduler = createMock(ReadOnlyScheduler.Iface.class);
@@ -241,7 +237,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
maintenance,
quotaManager,
stateManager,
- taskIdGenerator,
uuidGenerator,
jobUpdateController,
readOnlyScheduler,
@@ -289,8 +284,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active());
expectNoCronJob();
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectInstanceQuotaCheck(sanitized, ENOUGH_QUOTA);
stateManager.insertPendingTasks(
@@ -311,8 +304,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active());
expectNoCronJob();
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectInstanceQuotaCheck(sanitized, ENOUGH_QUOTA);
stateManager.insertPendingTasks(
@@ -402,35 +393,12 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
}
@Test
- public void testCreateJobFailsTaskIdLength() throws Exception {
- IJobConfiguration job = IJobConfiguration.build(makeJob());
- SanitizedConfiguration sanitized = fromUnsanitized(job);
- lockManager.assertNotLocked(LOCK_KEY);
- storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active());
- expectNoCronJob();
- expect(quotaManager.checkInstanceAddition(
- anyObject(ITaskConfig.class),
- anyInt(),
- eq(storageUtil.mutableStoreProvider))).andReturn(ENOUGH_QUOTA);
-
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(Strings.repeat("a", MAX_TASK_ID_LENGTH + 1));
-
- control.replay();
-
- assertResponse(INVALID_REQUEST, thrift.createJob(job.newBuilder()));
- assertEquals(0L, statsProvider.getLongValue(CREATE_JOB));
- }
-
- @Test
public void testCreateJobFailsQuotaCheck() throws Exception {
IJobConfiguration job = IJobConfiguration.build(makeProdJob());
SanitizedConfiguration sanitized = fromUnsanitized(job);
lockManager.assertNotLocked(LOCK_KEY);
storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active());
expectNoCronJob();
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectInstanceQuotaCheck(sanitized, NOT_ENOUGH_QUOTA);
control.replay();
@@ -553,8 +521,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active());
expectNoCronJob();
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectInstanceQuotaCheck(sanitized, ENOUGH_QUOTA);
stateManager.insertPendingTasks(
@@ -597,8 +563,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active());
expectNoCronJob();
- expect(taskIdGenerator.generate(ITaskConfig.build(sanitized.getTaskConfig()), 1))
- .andReturn(TASK_ID);
expectInstanceQuotaCheck(ITaskConfig.build(sanitized.getTaskConfig()), ENOUGH_QUOTA);
stateManager.insertPendingTasks(
storageUtil.mutableStoreProvider,
@@ -988,8 +952,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
SanitizedConfiguration sanitized = fromUnsanitized(IJobConfiguration.build(CRON_JOB));
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectCronQuotaCheck(sanitized.getJobConfig(), ENOUGH_QUOTA);
cronJobManager.updateJob(anyObject(SanitizedCronJob.class));
control.replay();
@@ -1013,8 +975,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
SanitizedConfiguration sanitized = fromUnsanitized(IJobConfiguration.build(CRON_JOB));
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectCronQuotaCheck(sanitized.getJobConfig(), ENOUGH_QUOTA);
cronJobManager.updateJob(anyObject(SanitizedCronJob.class));
expectLastCall().andThrow(new CronException("Nope"));
@@ -1045,8 +1005,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
SanitizedConfiguration sanitized = fromUnsanitized(IJobConfiguration.build(CRON_JOB));
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectCronQuotaCheck(sanitized.getJobConfig(), ENOUGH_QUOTA);
expectNoCronJob().times(2);
@@ -1061,8 +1019,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
SanitizedConfiguration sanitized = fromUnsanitized(IJobConfiguration.build(CRON_JOB));
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectCronQuotaCheck(sanitized.getJobConfig(), ENOUGH_QUOTA);
expectNoCronJob();
@@ -1078,8 +1034,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
SanitizedConfiguration sanitized = fromUnsanitized(IJobConfiguration.build(CRON_JOB));
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectCronQuotaCheck(sanitized.getJobConfig(), ENOUGH_QUOTA);
expectCronJob();
@@ -1118,8 +1072,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
lockManager.assertNotLocked(LOCK_KEY);
SanitizedConfiguration sanitized = fromUnsanitized(IJobConfiguration.build(CRON_JOB));
- expect(taskIdGenerator.generate(sanitized.getJobConfig().getTaskConfig(), 1))
- .andReturn(TASK_ID);
expectCronQuotaCheck(sanitized.getJobConfig(), NOT_ENOUGH_QUOTA);
control.replay();
@@ -1492,7 +1444,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
IScheduledTask activeTask = buildScheduledTask();
ITaskConfig task = activeTask.getAssignedTask().getTask();
storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active(), activeTask);
- expect(taskIdGenerator.generate(task, 3)).andReturn(TASK_ID);
expect(quotaManager.checkInstanceAddition(
task,
2,
@@ -1567,34 +1518,12 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
}
@Test
- public void testAddInstancesFailsTaskIdLength() throws Exception {
- IScheduledTask activeTask = buildScheduledTask();
- ITaskConfig task = activeTask.getAssignedTask().getTask();
- expectNoCronJob();
- lockManager.assertNotLocked(LOCK_KEY);
- storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active(), activeTask);
- expect(quotaManager.checkInstanceAddition(
- anyObject(ITaskConfig.class),
- anyInt(),
- eq(storageUtil.mutableStoreProvider))).andReturn(ENOUGH_QUOTA);
- expect(taskIdGenerator.generate(task, 2))
- .andReturn(Strings.repeat("a", MAX_TASK_ID_LENGTH + 1));
-
- control.replay();
-
- assertResponse(INVALID_REQUEST, thrift.addInstances(INSTANCE_KEY, 1));
- assertEquals(0L, statsProvider.getLongValue(ADD_INSTANCES));
- }
-
- @Test
public void testAddInstancesFailsQuotaCheck() throws Exception {
IScheduledTask activeTask = buildScheduledTask();
ITaskConfig task = activeTask.getAssignedTask().getTask();
expectNoCronJob();
lockManager.assertNotLocked(LOCK_KEY);
storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active(), activeTask);
- expect(taskIdGenerator.generate(task, 2))
- .andReturn(TASK_ID);
expectInstanceQuotaCheck(task, NOT_ENOUGH_QUOTA);
control.replay();
@@ -1610,8 +1539,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
expectNoCronJob();
lockManager.assertNotLocked(LOCK_KEY);
storageUtil.expectTaskFetch(Query.jobScoped(JOB_KEY).active(), activeTask);
- expect(taskIdGenerator.generate(task, 2))
- .andReturn(TASK_ID);
expectInstanceQuotaCheck(task, ENOUGH_QUOTA);
stateManager.insertPendingTasks(
storageUtil.mutableStoreProvider,
@@ -1631,7 +1558,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
expectNoCronJob();
ITaskConfig newTask = buildTaskForJobUpdate(0).getAssignedTask().getTask();
- expect(taskIdGenerator.generate(newTask, 6)).andReturn(TASK_ID);
IScheduledTask oldTask1 = buildTaskForJobUpdate(0, "old");
IScheduledTask oldTask2 = buildTaskForJobUpdate(1, "old");
@@ -1685,7 +1611,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
expectNoCronJob();
ITaskConfig newTask = buildTaskForJobUpdate(0).getAssignedTask().getTask();
- expect(taskIdGenerator.generate(newTask, 1)).andReturn(TASK_ID);
IScheduledTask oldTask1 = buildTaskForJobUpdate(0);
IScheduledTask oldTask2 = buildTaskForJobUpdate(1);
@@ -1912,8 +1837,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
Query.unscoped().byJob(JOB_KEY).active(),
newTask, oldTask1, oldTask2);
expectJobUpdateQuotaCheck(ENOUGH_QUOTA);
- expect(taskIdGenerator.generate(EasyMock.<ITaskConfig>anyObject(), anyInt()))
- .andReturn(TASK_ID);
jobUpdateController.start(EasyMock.<IJobUpdate>anyObject(), eq(AUDIT));
ITaskConfig newConfig = newTask.getAssignedTask().getTask();
@@ -1950,24 +1873,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
}
@Test
- public void testStartUpdateFailsTaskIdLength() throws Exception {
- JobUpdateRequest request = buildServiceJobUpdateRequest(populatedTask());
- expectGetRemoteUser();
- expectNoCronJob();
- expect(uuidGenerator.createNew()).andReturn(UU_ID);
- storageUtil.expectTaskFetch(Query.unscoped().byJob(JOB_KEY).active());
- expectJobUpdateQuotaCheck(ENOUGH_QUOTA);
-
- expect(taskIdGenerator.generate(ITaskConfig.build(request.getTaskConfig()), 6))
- .andReturn(Strings.repeat("a", MAX_TASK_ID_LENGTH + 1));
-
- control.replay();
-
- assertResponse(INVALID_REQUEST, thrift.startJobUpdate(request, AUDIT_MESSAGE));
- assertEquals(0L, statsProvider.getLongValue(START_JOB_UPDATE));
- }
-
- @Test
public void testStartUpdateFailsQuotaCheck() throws Exception {
JobUpdateRequest request = buildServiceJobUpdateRequest(populatedTask());
expectGetRemoteUser();
@@ -1976,8 +1881,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
IScheduledTask oldTask = buildTaskForJobUpdate(0);
storageUtil.expectTaskFetch(Query.unscoped().byJob(JOB_KEY).active(), oldTask);
- expect(taskIdGenerator.generate(ITaskConfig.build(request.getTaskConfig()), 6))
- .andReturn(TASK_ID);
expectJobUpdateQuotaCheck(NOT_ENOUGH_QUOTA);
@@ -2001,7 +1904,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
ImmutableMap.of(oldTask.getAssignedTask().getTask(), ImmutableSet.of(new Range(0, 0))));
expect(uuidGenerator.createNew()).andReturn(UU_ID);
- expect(taskIdGenerator.generate(newTask, 1)).andReturn(TASK_ID);
expectJobUpdateQuotaCheck(ENOUGH_QUOTA);
storageUtil.expectTaskFetch(Query.unscoped().byJob(JOB_KEY).active(), oldTask);
@@ -2030,7 +1932,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
ImmutableMap.of(oldTask.getAssignedTask().getTask(), ImmutableSet.of(new Range(0, 0))));
expect(uuidGenerator.createNew()).andReturn(UU_ID);
- expect(taskIdGenerator.generate(newTask, 1)).andReturn(TASK_ID);
expectJobUpdateQuotaCheck(ENOUGH_QUOTA);
storageUtil.expectTaskFetch(Query.unscoped().byJob(JOB_KEY).active(), oldTask);