Return-Path: X-Original-To: apmail-aurora-commits-archive@minotaur.apache.org Delivered-To: apmail-aurora-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7E35F177EF for ; Tue, 16 Jun 2015 01:00:33 +0000 (UTC) Received: (qmail 84562 invoked by uid 500); 16 Jun 2015 01:00:33 -0000 Delivered-To: apmail-aurora-commits-archive@aurora.apache.org Received: (qmail 84529 invoked by uid 500); 16 Jun 2015 01:00:33 -0000 Mailing-List: contact commits-help@aurora.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@aurora.apache.org Delivered-To: mailing list commits@aurora.apache.org Received: (qmail 84520 invoked by uid 99); 16 Jun 2015 01:00:33 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 16 Jun 2015 01:00:33 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 2E099E1091; Tue, 16 Jun 2015 01:00:33 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: maxim@apache.org To: commits@aurora.apache.org Message-Id: X-Mailer: ASF-Git Admin Mailer Subject: aurora git commit: Removing deprecated JobUpdateSummary fields. Date: Tue, 16 Jun 2015 01:00:33 +0000 (UTC) Repository: aurora Updated Branches: refs/heads/master 185b48efb -> b09adc624 Removing deprecated JobUpdateSummary fields. Bugs closed: AURORA-1139 Reviewed at https://reviews.apache.org/r/35483/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/b09adc62 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/b09adc62 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/b09adc62 Branch: refs/heads/master Commit: b09adc624b0fcf077a532d55ae0f83491ede1cbd Parents: 185b48e Author: Maxim Khutornenko Authored: Mon Jun 15 17:57:47 2015 -0700 Committer: Maxim Khutornenko Committed: Mon Jun 15 17:57:47 2015 -0700 ---------------------------------------------------------------------- .../thrift/org/apache/aurora/gen/api.thrift | 7 --- .../thrift/org/apache/aurora/gen/storage.thrift | 4 -- .../org/apache/aurora/benchmark/JobUpdates.java | 2 - .../scheduler/storage/ForwardingStore.java | 5 -- .../scheduler/storage/JobUpdateStore.java | 12 ----- .../scheduler/storage/db/DbJobUpdateStore.java | 14 +----- .../storage/db/JobUpdateDetailsMapper.java | 13 ----- .../scheduler/storage/log/LogStorage.java | 43 +++-------------- .../storage/log/SnapshotStoreImpl.java | 6 --- .../storage/log/WriteAheadStorage.java | 5 +- .../thrift/SchedulerThriftInterface.java | 2 - .../updater/JobUpdateControllerImpl.java | 2 +- .../aurora/scheduler/updater/Updates.java | 20 -------- .../storage/db/JobUpdateDetailsMapper.xml | 31 ++---------- .../aurora/scheduler/storage/db/schema.sql | 3 +- .../storage/db/DbJobUpdateStoreTest.java | 36 +++++--------- .../scheduler/storage/log/LogStorageTest.java | 50 +++----------------- .../storage/log/SnapshotStoreImplTest.java | 11 +---- .../thrift/ReadOnlySchedulerImplTest.java | 4 +- .../thrift/SchedulerThriftInterfaceTest.java | 2 - .../aurora/scheduler/updater/JobUpdaterIT.java | 8 ++-- 21 files changed, 44 insertions(+), 236 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/api/src/main/thrift/org/apache/aurora/gen/api.thrift ---------------------------------------------------------------------- diff --git a/api/src/main/thrift/org/apache/aurora/gen/api.thrift b/api/src/main/thrift/org/apache/aurora/gen/api.thrift index dd54e5b..d740a90 100644 --- a/api/src/main/thrift/org/apache/aurora/gen/api.thrift +++ b/api/src/main/thrift/org/apache/aurora/gen/api.thrift @@ -745,13 +745,6 @@ struct JobUpdateState { /** Summary of the job update including job key, user and current state. */ struct JobUpdateSummary { - // TODO(wfarner): As part of AURORA-1093, add a JobUpdateKey field, deprecate updateId and jobKey. - /** Update ID. */ - 1: string updateId - - /** Job key. */ - 2: JobKey jobKey - /** Unique identifier for the update. */ 5: JobUpdateKey key http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/api/src/main/thrift/org/apache/aurora/gen/storage.thrift ---------------------------------------------------------------------- diff --git a/api/src/main/thrift/org/apache/aurora/gen/storage.thrift b/api/src/main/thrift/org/apache/aurora/gen/storage.thrift index 26c5db8..b7c4665 100644 --- a/api/src/main/thrift/org/apache/aurora/gen/storage.thrift +++ b/api/src/main/thrift/org/apache/aurora/gen/storage.thrift @@ -79,15 +79,11 @@ struct StoredJobUpdateDetails { struct SaveJobUpdateEvent { 1: api.JobUpdateEvent event - // TODO(wfarner): Remove this in 0.9.0. - 2: string updateId 3: api.JobUpdateKey key } struct SaveJobInstanceUpdateEvent { 1: api.JobInstanceUpdateEvent event - // TODO(wfarner): Remove this in 0.9.0. - 2: string updateId 3: api.JobUpdateKey key } http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java ---------------------------------------------------------------------- diff --git a/src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java b/src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java index 3c6d4c9..48bee50 100644 --- a/src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java +++ b/src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java @@ -70,8 +70,6 @@ final class JobUpdates { JobUpdate update = new JobUpdate() .setSummary(new JobUpdateSummary() .setKey(key) - .setUpdateId(key.getId()) - .setJobKey(job) .setUser(USER)) .setInstructions(new JobUpdateInstructions() .setSettings(new JobUpdateSettings() http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java b/src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java index 1a63169..5242eba 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java @@ -139,11 +139,6 @@ public class ForwardingStore implements } @Override - public Optional fetchUpdateKey(String updateId) { - return jobUpdateStore.fetchUpdateKey(updateId); - } - - @Override public List fetchJobUpdateSummaries(IJobUpdateQuery query) { return jobUpdateStore.fetchJobUpdateSummaries(query); } http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java b/src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java index 159cb0c..52c3c66 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java @@ -83,18 +83,6 @@ public interface JobUpdateStore { Set fetchAllJobUpdateDetails(); /** - * Fetches an update key based on the update ID. - * - *

- * This is a compatibility shim for when updates were only identified by a string, and should - * be removed in 0.9.0. - * - * @param updateId Update identifier. - * @return The update key matching {@code updateId}. - */ - Optional fetchUpdateKey(String updateId); - - /** * Gets the lock token associated with a job update. * * @param key Update identifier. http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java b/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java index 4b9d7f5..11c9c4a 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java @@ -27,7 +27,6 @@ import com.twitter.common.base.MorePreconditions; import org.apache.aurora.gen.JobUpdate; import org.apache.aurora.gen.JobUpdateInstructions; -import org.apache.aurora.gen.JobUpdateKey; import org.apache.aurora.gen.JobUpdateStatus; import org.apache.aurora.gen.storage.StoredJobUpdateDetails; import org.apache.aurora.scheduler.stats.CachedCounters; @@ -83,12 +82,10 @@ public class DbJobUpdateStore implements JobUpdateStore.Mutable { "Missing both initial and desired states. At least one is required."); } - IJobUpdateSummary summary = update.getSummary(); - jobKeyMapper.merge(summary.getJobKey().newBuilder()); + IJobUpdateKey key = update.getSummary().getKey(); + jobKeyMapper.merge(key.getJob().newBuilder()); detailsMapper.insert(update.newBuilder()); - IJobUpdateKey key = IJobUpdateKey.build( - new JobUpdateKey(summary.getJobKey().newBuilder(), summary.getUpdateId())); if (lockToken.isPresent()) { detailsMapper.insertLockToken(key, lockToken.get()); } @@ -251,13 +248,6 @@ public class DbJobUpdateStore implements JobUpdateStore.Mutable { return ImmutableSet.copyOf(detailsMapper.selectAllDetails()); } - @Timed("job_update_store_fetch_update_key") - @Override - public Optional fetchUpdateKey(String updateId) { - return Optional.fromNullable(detailsMapper.selectUpdateKey(updateId)) - .transform(IJobUpdateKey.FROM_BUILDER); - } - @Timed("job_update_store_get_lock_token") @Override public Optional getLockToken(IJobUpdateKey key) { http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java b/src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java index b1b6f11..02ea355 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java @@ -21,7 +21,6 @@ import javax.annotation.Nullable; import org.apache.aurora.gen.JobInstanceUpdateEvent; import org.apache.aurora.gen.JobUpdate; import org.apache.aurora.gen.JobUpdateInstructions; -import org.apache.aurora.gen.JobUpdateKey; import org.apache.aurora.gen.JobUpdateQuery; import org.apache.aurora.gen.JobUpdateSummary; import org.apache.aurora.gen.Range; @@ -145,18 +144,6 @@ interface JobUpdateDetailsMapper { List selectSummaries(JobUpdateQuery query); /** - * Selects an update key by the update ID field contained within it. - * - *

- * TODO(wfarner): Remove this in the final phase of AURORA-1093. - * - * @param updateId Update to search by. - * @return The update key that {@code updateId} represents. - */ - @Nullable - JobUpdateKey selectUpdateKey(String updateId); - - /** * Gets details for the provided {@code key}. * * @param key Update to get. http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java index c58f531..82be367 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java @@ -18,7 +18,6 @@ import java.util.Date; import java.util.Map; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.ReentrantLock; import java.util.logging.Level; import java.util.logging.Logger; @@ -34,13 +33,11 @@ import com.twitter.common.inject.TimedInterceptor.Timed; import com.twitter.common.quantity.Amount; import com.twitter.common.quantity.Time; import com.twitter.common.stats.SlidingStats; -import com.twitter.common.stats.Stats; import com.twitter.common.util.concurrent.ExecutorServiceShutdown; import org.apache.aurora.codec.ThriftBinaryCodec.CodingException; import org.apache.aurora.gen.HostAttributes; import org.apache.aurora.gen.JobUpdate; -import org.apache.aurora.gen.JobUpdateKey; import org.apache.aurora.gen.storage.LogEntry; import org.apache.aurora.gen.storage.Op; import org.apache.aurora.gen.storage.RewriteTask; @@ -72,13 +69,11 @@ import org.apache.aurora.scheduler.storage.entities.IJobKey; import org.apache.aurora.scheduler.storage.entities.IJobUpdate; import org.apache.aurora.scheduler.storage.entities.IJobUpdateEvent; import org.apache.aurora.scheduler.storage.entities.IJobUpdateKey; -import org.apache.aurora.scheduler.storage.entities.IJobUpdateSummary; import org.apache.aurora.scheduler.storage.entities.ILock; import org.apache.aurora.scheduler.storage.entities.ILockKey; import org.apache.aurora.scheduler.storage.entities.IResourceAggregate; import org.apache.aurora.scheduler.storage.entities.IScheduledTask; import org.apache.aurora.scheduler.storage.entities.ITaskConfig; -import org.apache.aurora.scheduler.updater.Updates; import static java.util.Objects.requireNonNull; @@ -205,7 +200,6 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore private final SlidingStats writerWaitStats = new SlidingStats("log_storage_write_lock_wait", "ns"); - private final AtomicLong droppedUpdateEvents = Stats.exportLong("dropped_update_events"); private final Map> logEntryReplayActions; private final Map> transactionReplayActions; @@ -428,9 +422,6 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore @Override public void execute(Op op) { JobUpdate update = op.getSaveJobUpdate().getJobUpdate(); - update.setSummary(Updates.backfillJobUpdateKey( - IJobUpdateSummary.build(update.getSummary())).newBuilder()); - writeBehindJobUpdateStore.saveJobUpdate( IJobUpdate.build(update), Optional.fromNullable(op.getSaveJobUpdate().getLockToken())); @@ -440,36 +431,18 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore @Override public void execute(Op op) { SaveJobUpdateEvent event = op.getSaveJobUpdateEvent(); - if (!event.isSetKey()) { - event.setKey(getUpdateKeyById(event.getUpdateId()).orNull()); - } - - if (event.isSetKey()) { - writeBehindJobUpdateStore.saveJobUpdateEvent( - IJobUpdateKey.build(event.getKey()), - IJobUpdateEvent.build(op.getSaveJobUpdateEvent().getEvent())); - } else { - LOG.severe("Dropping unidentifiable op: " + op); - droppedUpdateEvents.incrementAndGet(); - } + writeBehindJobUpdateStore.saveJobUpdateEvent( + IJobUpdateKey.build(event.getKey()), + IJobUpdateEvent.build(op.getSaveJobUpdateEvent().getEvent())); } }) .put(Op._Fields.SAVE_JOB_INSTANCE_UPDATE_EVENT, new Closure() { @Override public void execute(Op op) { SaveJobInstanceUpdateEvent event = op.getSaveJobInstanceUpdateEvent(); - if (!event.isSetKey()) { - event.setKey(getUpdateKeyById(event.getUpdateId()).orNull()); - } - - if (event.isSetKey()) { - writeBehindJobUpdateStore.saveJobInstanceUpdateEvent( - IJobUpdateKey.build(event.getKey()), - IJobInstanceUpdateEvent.build(op.getSaveJobInstanceUpdateEvent().getEvent())); - } else { - LOG.severe("Dropping unidentifiable op: " + op); - droppedUpdateEvents.incrementAndGet(); - } + writeBehindJobUpdateStore.saveJobInstanceUpdateEvent( + IJobUpdateKey.build(event.getKey()), + IJobInstanceUpdateEvent.build(op.getSaveJobInstanceUpdateEvent().getEvent())); } }) .put(Op._Fields.PRUNE_JOB_UPDATE_HISTORY, new Closure() { @@ -482,10 +455,6 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore }).build(); } - private Optional getUpdateKeyById(String updateId) { - return writeBehindJobUpdateStore.fetchUpdateKey(updateId).transform(IJobUpdateKey.TO_BUILDER); - } - @Override public synchronized void prepare() { writeBehindStorage.prepare(); http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java b/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java index c5e2323..64b5b95 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java @@ -50,11 +50,9 @@ import org.apache.aurora.scheduler.storage.entities.IJobInstanceUpdateEvent; import org.apache.aurora.scheduler.storage.entities.IJobUpdate; import org.apache.aurora.scheduler.storage.entities.IJobUpdateEvent; import org.apache.aurora.scheduler.storage.entities.IJobUpdateKey; -import org.apache.aurora.scheduler.storage.entities.IJobUpdateSummary; import org.apache.aurora.scheduler.storage.entities.ILock; import org.apache.aurora.scheduler.storage.entities.IResourceAggregate; import org.apache.aurora.scheduler.storage.entities.IScheduledTask; -import org.apache.aurora.scheduler.updater.Updates; import static java.util.Objects.requireNonNull; @@ -219,10 +217,6 @@ public class SnapshotStoreImpl implements SnapshotStore { if (snapshot.isSetJobUpdateDetails()) { for (StoredJobUpdateDetails storedDetails : snapshot.getJobUpdateDetails()) { JobUpdateDetails details = storedDetails.getDetails(); - details.getUpdate().setSummary( - Updates.backfillJobUpdateKey( - IJobUpdateSummary.build(details.getUpdate().getSummary())).newBuilder()); - updateStore.saveJobUpdate( IJobUpdate.build(details.getUpdate()), Optional.fromNullable(storedDetails.getLockToken())); http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java b/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java index e61a6b4..0b55304 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java @@ -279,8 +279,7 @@ class WriteAheadStorage extends ForwardingStore implements requireNonNull(key); requireNonNull(event); - write(Op.saveJobUpdateEvent( - new SaveJobUpdateEvent(event.newBuilder(), key.getId(), key.newBuilder()))); + write(Op.saveJobUpdateEvent(new SaveJobUpdateEvent(event.newBuilder(), key.newBuilder()))); jobUpdateStore.saveJobUpdateEvent(key, event); } @@ -290,7 +289,7 @@ class WriteAheadStorage extends ForwardingStore implements requireNonNull(event); write(Op.saveJobInstanceUpdateEvent( - new SaveJobInstanceUpdateEvent(event.newBuilder(), key.getId(), key.newBuilder()))); + new SaveJobInstanceUpdateEvent(event.newBuilder(), key.newBuilder()))); jobUpdateStore.saveJobInstanceUpdateEvent(key, event); } http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/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 1ca3a9b..0670b2b 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java @@ -1197,8 +1197,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { IJobUpdate update = IJobUpdate.build(new JobUpdate() .setSummary(new JobUpdateSummary() .setKey(new JobUpdateKey(job.newBuilder(), updateId)) - .setJobKey(job.newBuilder()) - .setUpdateId(updateId) .setUser(context.getIdentity())) .setInstructions(instructions)); try { http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java b/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java index 1ebfa64..dc08587 100644 --- a/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java +++ b/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java @@ -153,7 +153,7 @@ class JobUpdateControllerImpl implements JobUpdateController { IJobUpdateSummary summary = update.getSummary(); IJobUpdateInstructions instructions = update.getInstructions(); - IJobKey job = summary.getJobKey(); + IJobKey job = summary.getKey().getJob(); // Validate the update configuration by making sure we can create an updater for it. updateFactory.newUpdate(update.getInstructions(), true); http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/java/org/apache/aurora/scheduler/updater/Updates.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/updater/Updates.java b/src/main/java/org/apache/aurora/scheduler/updater/Updates.java index 6466473..f949fd5 100644 --- a/src/main/java/org/apache/aurora/scheduler/updater/Updates.java +++ b/src/main/java/org/apache/aurora/scheduler/updater/Updates.java @@ -19,12 +19,9 @@ import com.google.common.collect.ImmutableRangeSet; import com.google.common.collect.Range; import com.google.common.collect.Sets; -import org.apache.aurora.gen.JobUpdateKey; import org.apache.aurora.gen.JobUpdateStatus; -import org.apache.aurora.gen.JobUpdateSummary; import org.apache.aurora.gen.apiConstants; import org.apache.aurora.scheduler.storage.entities.IInstanceTaskConfig; -import org.apache.aurora.scheduler.storage.entities.IJobUpdateSummary; import org.apache.aurora.scheduler.storage.entities.IRange; /** @@ -42,23 +39,6 @@ public final class Updates { Sets.immutableEnumSet(apiConstants.ACTIVE_JOB_UPDATE_STATES); /** - * Populates the {@link IJobUpdateSummary#getKey()} if it is not set by cloning other fields. - * - * @param summary Update summary to backfill. - * @return The original summary, guaranteed to have the {@code key} field populated. - */ - public static IJobUpdateSummary backfillJobUpdateKey(IJobUpdateSummary summary) { - if (summary.isSetKey()) { - return summary; - } else { - JobUpdateSummary mutableSummary = summary.newBuilder(); - mutableSummary.setKey( - new JobUpdateKey(mutableSummary.getJobKey(), mutableSummary.getUpdateId())); - return IJobUpdateSummary.build(mutableSummary); - } - } - - /** * Creates a range set representing all instance IDs represented by a set of instance * configurations included in a job update. * http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml ---------------------------------------------------------------------- diff --git a/src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml b/src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml index cf31cf8..6ffb54f 100644 --- a/src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml +++ b/src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml @@ -58,11 +58,11 @@ ( SELECT ID FROM job_keys - WHERE role = #{summary.jobKey.role} - AND environment = #{summary.jobKey.environment} - AND name = #{summary.jobKey.name} + WHERE role = #{summary.key.job.role} + AND environment = #{summary.key.job.environment} + AND name = #{summary.key.job.name} ), - #{summary.updateId}, + #{summary.key.id}, #{summary.user}, #{instructions.settings.updateGroupSize}, #{instructions.settings.maxPerInstanceFailures}, @@ -144,17 +144,7 @@ - - - - - - - + @@ -328,17 +318,6 @@ - - http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ---------------------------------------------------------------------- diff --git a/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql b/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql index 1ff8d23..24cf526 100644 --- a/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql +++ b/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql @@ -108,8 +108,7 @@ CREATE TABLE job_updates( wait_for_batch_completion BOOLEAN NOT NULL, block_if_no_pulses_after_ms INT NULL, - -- TODO(wfarner): Convert this to UNIQUE(job_key_id, update_id) to complete AURORA-1139. - UNIQUE(update_id) + UNIQUE(update_id, job_key_id) ); CREATE TABLE job_update_locks( http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java b/src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java index d1d7e79..550deae 100644 --- a/src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java +++ b/src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java @@ -131,9 +131,8 @@ public class DbJobUpdateStoreTest { saveUpdate(update2, Optional.absent()); assertUpdate(update2); - // Colliding update IDs should be forbidden. - IJobUpdate update3 = - makeJobUpdate(makeKey(JobKeys.from("role", "env", "name3"), updateId2.getId())); + // Colliding update keys should be forbidden. + IJobUpdate update3 = makeJobUpdate(updateId2); try { saveUpdate(update3, Optional.absent()); fail("Update ID collision should not be allowed"); @@ -336,7 +335,7 @@ public class DbJobUpdateStoreTest { @Override public void execute(MutableStoreProvider storeProvider) { IJobUpdate update = makeJobUpdate(updateId); - storeProvider.getLockStore().saveLock(makeLock(update.getSummary().getJobKey(), "lock1")); + storeProvider.getLockStore().saveLock(makeLock(update, "lock1")); storeProvider.getJobUpdateStore().saveJobUpdate(update, Optional.of("lock1")); } }); @@ -610,15 +609,13 @@ public class DbJobUpdateStoreTest { Optional.of("lock2"), storeProvider.getJobUpdateStore().getLockToken(UPDATE2)); - storeProvider.getLockStore().removeLock( - makeLock(update1.getSummary().getJobKey(), "lock1").getKey()); + storeProvider.getLockStore().removeLock(makeLock(update1, "lock1").getKey()); assertEquals(NO_TOKEN, storeProvider.getJobUpdateStore().getLockToken(UPDATE1)); assertEquals( Optional.of("lock2"), storeProvider.getJobUpdateStore().getLockToken(UPDATE2)); - storeProvider.getLockStore().removeLock( - makeLock(update2.getSummary().getJobKey(), "lock2").getKey()); + storeProvider.getLockStore().removeLock(makeLock(update2, "lock2").getKey()); assertEquals(NO_TOKEN, storeProvider.getJobUpdateStore().getLockToken(UPDATE1)); assertEquals(NO_TOKEN, storeProvider.getJobUpdateStore().getLockToken(UPDATE2)); } @@ -666,17 +663,17 @@ public class DbJobUpdateStoreTest { assertEquals( ImmutableList.of(s5), getSummaries( - new JobUpdateQuery().setKey(new JobUpdateKey(job5.newBuilder(), s5.getUpdateId())))); + new JobUpdateQuery().setKey(new JobUpdateKey(job5.newBuilder(), s5.getKey().getId())))); // Test querying by incorrect update keys. assertEquals( ImmutableList.of(), getSummaries( - new JobUpdateQuery().setKey(new JobUpdateKey(job5.newBuilder(), s4.getUpdateId())))); + new JobUpdateQuery().setKey(new JobUpdateKey(job5.newBuilder(), s4.getKey().getId())))); assertEquals( ImmutableList.of(), getSummaries( - new JobUpdateQuery().setKey(new JobUpdateKey(job4.newBuilder(), s5.getUpdateId())))); + new JobUpdateQuery().setKey(new JobUpdateKey(job4.newBuilder(), s5.getKey().getId())))); // Test query by user. assertEquals(ImmutableList.of(s2, s1), getSummaries(new JobUpdateQuery().setUser("user"))); @@ -862,9 +859,9 @@ public class DbJobUpdateStoreTest { }); } - private static ILock makeLock(IJobKey jobKey, String lockToken) { + private static ILock makeLock(IJobUpdate update, String lockToken) { return ILock.build(new Lock() - .setKey(LockKey.job(jobKey.newBuilder())) + .setKey(LockKey.job(update.getSummary().getKey().getJob().newBuilder())) .setToken(lockToken) .setTimestampMs(100) .setUser("fake user")); @@ -875,8 +872,7 @@ public class DbJobUpdateStoreTest { @Override public void execute(MutableStoreProvider storeProvider) { if (lockToken.isPresent()) { - storeProvider.getLockStore().saveLock( - makeLock(update.getSummary().getJobKey(), lockToken.get())); + storeProvider.getLockStore().saveLock(makeLock(update, lockToken.get())); } storeProvider.getJobUpdateStore().saveJobUpdate(update, lockToken); storeProvider.getJobUpdateStore().saveJobUpdateEvent( @@ -893,8 +889,7 @@ public class DbJobUpdateStoreTest { @Override public void execute(MutableStoreProvider storeProvider) { if (lockToken.isPresent()) { - storeProvider.getLockStore().saveLock( - makeLock(update.getSummary().getJobKey(), lockToken.get())); + storeProvider.getLockStore().saveLock(makeLock(update, lockToken.get())); } storeProvider.getJobUpdateStore().saveJobUpdate(update, lockToken); } @@ -943,8 +938,7 @@ public class DbJobUpdateStoreTest { storage.write(new MutateWork.NoResult.Quiet() { @Override public void execute(MutableStoreProvider storeProvider) { - storeProvider.getLockStore().removeLock( - makeLock(update.getSummary().getJobKey(), lockToken).getKey()); + storeProvider.getLockStore().removeLock(makeLock(update, lockToken).getKey()); } }); } @@ -1012,8 +1006,6 @@ public class DbJobUpdateStoreTest { private IJobUpdateSummary makeSummary(IJobUpdateKey key, String user) { return IJobUpdateSummary.build(new JobUpdateSummary() .setKey(key.newBuilder()) - .setUpdateId(key.getId()) - .setJobKey(key.getJob().newBuilder()) .setUser(user)); } @@ -1026,8 +1018,6 @@ public class DbJobUpdateStoreTest { IJobUpdateSummary summary = IJobUpdateSummary.build(new JobUpdateSummary() .setKey(key.newBuilder()) - .setUpdateId(key.getId()) - .setJobKey(key.getJob().newBuilder()) .setUser(user)); IJobUpdate update = makeJobUpdate(summary); http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java b/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java index cbc2d38..e455946 100644 --- a/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java +++ b/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java @@ -348,34 +348,16 @@ public class LogStorageTest extends EasyMockTest { builder.add(createTransaction(Op.removeLock(removeLock))); storageUtil.lockStore.removeLock(ILockKey.build(removeLock.getLockKey())); - JobUpdate update = new JobUpdate() - .setSummary(new JobUpdateSummary() - .setKey(UPDATE_ID.newBuilder()) - .setJobKey(UPDATE_ID.getJob().newBuilder()) - .setUpdateId(UPDATE_ID.getId())); + JobUpdate update = new JobUpdate().setSummary( + new JobUpdateSummary().setKey(UPDATE_ID.newBuilder())); SaveJobUpdate saveUpdate = new SaveJobUpdate(update, "token"); builder.add(createTransaction(Op.saveJobUpdate(saveUpdate))); storageUtil.jobUpdateStore.saveJobUpdate( IJobUpdate.build(saveUpdate.getJobUpdate()), Optional.of(saveUpdate.getLockToken())); - // Ensure that JobUpdateSummary.key is backfilled. - IJobUpdateKey updateId2 = - IJobUpdateKey.build(new JobUpdateKey(JOB_KEY.newBuilder(), "backfilled")); - JobUpdate legacyUpdate = new JobUpdate() - .setSummary(new JobUpdateSummary() - .setJobKey(updateId2.getJob().newBuilder()) - .setUpdateId(updateId2.getId())); - SaveJobUpdate saveUpdateBackfilled = new SaveJobUpdate(legacyUpdate, "token2"); - builder.add(createTransaction(Op.saveJobUpdate(saveUpdateBackfilled))); - JobUpdate legacyUpdateBackfilled = legacyUpdate.deepCopy(); - legacyUpdateBackfilled.getSummary().setKey(updateId2.newBuilder()); - storageUtil.jobUpdateStore.saveJobUpdate( - IJobUpdate.build(legacyUpdateBackfilled), - Optional.of(saveUpdateBackfilled.getLockToken())); - SaveJobUpdateEvent saveUpdateEvent = - new SaveJobUpdateEvent(new JobUpdateEvent(), "update", UPDATE_ID.newBuilder()); + new SaveJobUpdateEvent(new JobUpdateEvent(), UPDATE_ID.newBuilder()); builder.add(createTransaction(Op.saveJobUpdateEvent(saveUpdateEvent))); storageUtil.jobUpdateStore.saveJobUpdateEvent( UPDATE_ID, @@ -383,27 +365,12 @@ public class LogStorageTest extends EasyMockTest { SaveJobInstanceUpdateEvent saveInstanceEvent = new SaveJobInstanceUpdateEvent( new JobInstanceUpdateEvent(), - "update", UPDATE_ID.newBuilder()); builder.add(createTransaction(Op.saveJobInstanceUpdateEvent(saveInstanceEvent))); storageUtil.jobUpdateStore.saveJobInstanceUpdateEvent( UPDATE_ID, IJobInstanceUpdateEvent.build(saveInstanceEvent.getEvent())); - // Backwards compatibility as part of AURORA-1093. Exercises behavior to drop job - // update-related records that cannot be found based on their update ID string alone. - SaveJobInstanceUpdateEvent unknownSaveInstanceEvent = - new SaveJobInstanceUpdateEvent(new JobInstanceUpdateEvent(), "update5", null); - builder.add(createTransaction(Op.saveJobInstanceUpdateEvent(unknownSaveInstanceEvent))); - expect(storageUtil.jobUpdateStore.fetchUpdateKey("update5")) - .andReturn(Optional.absent()); - - SaveJobUpdateEvent unknownSaveUpdateEvent = - new SaveJobUpdateEvent(new JobUpdateEvent(), "update6", null); - builder.add(createTransaction(Op.saveJobUpdateEvent(unknownSaveUpdateEvent))); - expect(storageUtil.jobUpdateStore.fetchUpdateKey("update6")) - .andReturn(Optional.absent()); - builder.add(createTransaction(Op.pruneJobUpdateHistory(new PruneJobUpdateHistory(5, 10L)))); expect(storageUtil.jobUpdateStore.pruneHistory(5, 10L)) .andReturn(ImmutableSet.of(IJobUpdateKey.build(UPDATE_ID.newBuilder()))); @@ -932,21 +899,20 @@ public class LogStorageTest extends EasyMockTest { @Test public void testSaveUpdateWithLockToken() throws Exception { - saveAndAssertJobUpdate("id1", Optional.of("token")); + saveAndAssertJobUpdate(Optional.of("token")); } @Test public void testSaveUpdateWithNullLockToken() throws Exception { - saveAndAssertJobUpdate("id2", Optional.absent()); + saveAndAssertJobUpdate(Optional.absent()); } - private void saveAndAssertJobUpdate(final String updateId, final Optional lockToken) + private void saveAndAssertJobUpdate(final Optional lockToken) throws Exception { final IJobUpdate update = IJobUpdate.build(new JobUpdate() .setSummary(new JobUpdateSummary() - .setUpdateId(updateId) - .setJobKey(JOB_KEY.newBuilder()) + .setKey(UPDATE_ID.newBuilder()) .setUser("user")) .setInstructions(new JobUpdateInstructions() .setDesiredState(new InstanceTaskConfig() @@ -987,7 +953,6 @@ public class LogStorageTest extends EasyMockTest { storageUtil.jobUpdateStore.saveJobUpdateEvent(UPDATE_ID, event); streamMatcher.expectTransaction(Op.saveJobUpdateEvent(new SaveJobUpdateEvent( event.newBuilder(), - UPDATE_ID.getId(), UPDATE_ID.newBuilder()))).andReturn(position); } @@ -1013,7 +978,6 @@ public class LogStorageTest extends EasyMockTest { streamMatcher.expectTransaction(Op.saveJobInstanceUpdateEvent( new SaveJobInstanceUpdateEvent( event.newBuilder(), - UPDATE_ID.getId(), UPDATE_ID.newBuilder()))) .andReturn(position); } http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java b/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java index 982968a..c183acf 100644 --- a/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java +++ b/src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java @@ -111,20 +111,13 @@ public class SnapshotStoreImplTest extends EasyMockTest { IJobUpdateKey updateId2 = makeKey("updateId2"); IJobUpdateDetails updateDetails1 = IJobUpdateDetails.build(new JobUpdateDetails() .setUpdate(new JobUpdate().setSummary( - new JobUpdateSummary() - .setKey(updateId1.newBuilder()) - .setUpdateId(updateId1.getId()) - .setJobKey(updateId1.getJob().newBuilder()))) + new JobUpdateSummary().setKey(updateId1.newBuilder()))) .setUpdateEvents(ImmutableList.of(new JobUpdateEvent().setStatus(JobUpdateStatus.ERROR))) .setInstanceEvents(ImmutableList.of(new JobInstanceUpdateEvent().setTimestampMs(123L)))); IJobUpdateDetails updateDetails2 = IJobUpdateDetails.build(new JobUpdateDetails() .setUpdate(new JobUpdate().setSummary( - new JobUpdateSummary() - // Deliberately not setting the key field here to validate backwards compatibility - // with data written before that field existed. - .setUpdateId(updateId2.getId()) - .setJobKey(updateId2.getJob().newBuilder())))); + new JobUpdateSummary().setKey(updateId2.newBuilder())))); storageUtil.expectOperations(); expect(storageUtil.taskStore.fetchTasks(Query.unscoped())).andReturn(tasks); http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java index 52c7262..a854ed3 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java @@ -39,6 +39,7 @@ import org.apache.aurora.gen.JobStats; import org.apache.aurora.gen.JobSummary; import org.apache.aurora.gen.JobUpdate; import org.apache.aurora.gen.JobUpdateDetails; +import org.apache.aurora.gen.JobUpdateKey; import org.apache.aurora.gen.JobUpdateQuery; import org.apache.aurora.gen.JobUpdateSummary; import org.apache.aurora.gen.PendingReason; @@ -577,8 +578,7 @@ public class ReadOnlySchedulerImplTest extends EasyMockTest { ImmutableList.Builder builder = ImmutableList.builder(); for (int i = 0; i < count; i++) { builder.add(new JobUpdateSummary() - .setUpdateId("id" + i) - .setJobKey(JOB_KEY.newBuilder()) + .setKey(new JobUpdateKey(JOB_KEY.newBuilder(), "id" + 1)) .setUser(USER)); } return builder.build(); http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/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 21b4044..38ef412 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java @@ -2449,8 +2449,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { return IJobUpdate.build(new JobUpdate() .setSummary(new JobUpdateSummary() .setKey(UPDATE_KEY.newBuilder()) - .setJobKey(UPDATE_KEY.getJob().newBuilder()) - .setUpdateId(UPDATE_KEY.getId()) .setUser(ROLE_IDENTITY.getUser())) .setInstructions(new JobUpdateInstructions() .setSettings(buildJobUpdateSettings()) http://git-wip-us.apache.org/repos/asf/aurora/blob/b09adc62/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java index 7aa19d4..33dd9f1 100644 --- a/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java +++ b/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java @@ -1090,7 +1090,7 @@ public class JobUpdaterIT extends EasyMockTest { ILock lock; try { lock = lockManager.acquireLock( - ILockKey.build(LockKey.job(update.getSummary().getJobKey().newBuilder())), USER); + ILockKey.build(LockKey.job(update.getSummary().getKey().getJob().newBuilder())), USER); } catch (LockManager.LockException e) { throw Throwables.propagate(e); } @@ -1339,7 +1339,7 @@ public class JobUpdaterIT extends EasyMockTest { releaseAllLocks(); JobUpdate builder = makeJobUpdate(makeInstanceConfig(0, 0, OLD_CONFIG)).newBuilder(); - builder.getSummary().setUpdateId("another update"); + builder.getSummary().getKey().setId("another update"); IJobUpdate update2 = IJobUpdate.build(builder); try { @@ -1360,9 +1360,7 @@ public class JobUpdaterIT extends EasyMockTest { private static IJobUpdateSummary makeUpdateSummary() { return IJobUpdateSummary.build(new JobUpdateSummary() .setUser("user") - .setKey(UPDATE_ID.newBuilder()) - .setJobKey(UPDATE_ID.getJob().newBuilder()) - .setUpdateId(UPDATE_ID.getId())); + .setKey(UPDATE_ID.newBuilder())); } private static IJobUpdate makeJobUpdate(IInstanceTaskConfig... configs) {