You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by ma...@apache.org on 2015/02/23 22:08:03 UTC
incubator-aurora git commit: Saving scheduler backups asynchronously.
Repository: incubator-aurora
Updated Branches:
refs/heads/master ace02d131 -> 277ac4315
Saving scheduler backups asynchronously.
Bugs closed: AURORA-1108
Reviewed at https://reviews.apache.org/r/31171/
Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/277ac431
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/277ac431
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/277ac431
Branch: refs/heads/master
Commit: 277ac4315c30d0f041f84aa990ef607fdca22c7a
Parents: ace02d1
Author: Maxim Khutornenko <ma...@apache.org>
Authored: Mon Feb 23 13:05:20 2015 -0800
Committer: Maxim Khutornenko <ma...@apache.org>
Committed: Mon Feb 23 13:05:20 2015 -0800
----------------------------------------------------------------------
.../scheduler/storage/backup/BackupModule.java | 5 ++++
.../scheduler/storage/backup/StorageBackup.java | 28 ++++++++++++++------
.../storage/log/SnapshotStoreImpl.java | 7 ++---
.../scheduler/storage/backup/RecoveryTest.java | 13 ++++++---
.../storage/backup/StorageBackupTest.java | 7 +++--
5 files changed, 44 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/277ac431/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
index 71feb57..a6b187d 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java
@@ -14,6 +14,7 @@
package org.apache.aurora.scheduler.storage.backup;
import java.io.File;
+import java.util.concurrent.Executor;
import java.util.logging.Logger;
import javax.inject.Inject;
@@ -33,6 +34,7 @@ import com.twitter.common.quantity.Amount;
import com.twitter.common.quantity.Time;
import org.apache.aurora.gen.storage.Snapshot;
+import org.apache.aurora.scheduler.base.AsyncUtil;
import org.apache.aurora.scheduler.storage.SnapshotStore;
import org.apache.aurora.scheduler.storage.backup.Recovery.RecoveryImpl;
import org.apache.aurora.scheduler.storage.backup.StorageBackup.StorageBackupImpl;
@@ -87,6 +89,9 @@ public class BackupModule extends PrivateModule {
@Override
protected void configure() {
+ Executor executor = AsyncUtil.singleThreadLoggingScheduledExecutor("StorageBackup-%d", LOG);
+ bind(Executor.class).toInstance(executor);
+
TypeLiteral<SnapshotStore<Snapshot>> type = new TypeLiteral<SnapshotStore<Snapshot>>() { };
bind(type).annotatedWith(StorageBackupImpl.SnapshotDelegate.class).to(snapshotStore);
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/277ac431/src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java
index a20378a..569648a 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java
@@ -26,6 +26,7 @@ import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.List;
import java.util.Locale;
+import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicLong;
import java.util.logging.Level;
import java.util.logging.Logger;
@@ -60,9 +61,6 @@ import static java.util.Objects.requireNonNull;
/**
* A backup routine that layers over a snapshot store and periodically writes snapshots to
* local disk.
- *
- * TODO(William Farner): Perform backups asynchronously. As written, they are performed in a
- * blocking write operation, which is asking for trouble.
*/
public interface StorageBackup {
@@ -106,6 +104,7 @@ public interface StorageBackup {
private final long backupIntervalMs;
private volatile long lastBackupMs;
private final DateFormat backupDateFormat;
+ private final Executor executor;
private final AtomicLong successes = Stats.exportLong("scheduler_backup_success");
@VisibleForTesting
@@ -123,11 +122,13 @@ public interface StorageBackup {
StorageBackupImpl(
@SnapshotDelegate SnapshotStore<Snapshot> delegate,
Clock clock,
- BackupConfig config) {
+ BackupConfig config,
+ Executor executor) {
this.delegate = requireNonNull(delegate);
this.clock = requireNonNull(clock);
this.config = requireNonNull(config);
+ this.executor = requireNonNull(executor);
backupDateFormat = new SimpleDateFormat("yyyy-MM-dd-HH-mm", Locale.ENGLISH);
backupIntervalMs = config.interval.as(Time.MILLISECONDS);
lastBackupMs = clock.nowMillis();
@@ -135,9 +136,14 @@ public interface StorageBackup {
@Override
public Snapshot createSnapshot() {
- Snapshot snapshot = delegate.createSnapshot();
+ final Snapshot snapshot = delegate.createSnapshot();
if (clock.nowMillis() >= (lastBackupMs + backupIntervalMs)) {
- save(snapshot);
+ executor.execute(new Runnable() {
+ @Override
+ public void run() {
+ save(snapshot);
+ }
+ });
}
return snapshot;
}
@@ -176,7 +182,7 @@ public interface StorageBackup {
} finally {
if (tempFile.exists()) {
LOG.info("Deleting incomplete backup file " + tempFile);
- tempFile.delete();
+ tryDelete(tempFile);
}
}
@@ -191,12 +197,18 @@ public interface StorageBackup {
.sortedCopy(ImmutableList.copyOf(backups)).subList(0, backupsToDelete);
LOG.info("Deleting " + backupsToDelete + " outdated backups: " + toDelete);
for (File outdated : toDelete) {
- outdated.delete();
+ tryDelete(outdated);
}
}
}
}
+ private void tryDelete(File fileToDelete) {
+ if (!fileToDelete.delete()) {
+ LOG.severe("Failed to delete file: " + fileToDelete.getName());
+ }
+ }
+
private static final FilenameFilter BACKUP_FILTER = new FilenameFilter() {
@Override
public boolean accept(File file, String s) {
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/277ac431/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 9b38b7d..2798139 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
@@ -44,7 +44,6 @@ import org.apache.aurora.scheduler.storage.Storage.MutableStoreProvider;
import org.apache.aurora.scheduler.storage.Storage.MutateWork;
import org.apache.aurora.scheduler.storage.Storage.StoreProvider;
import org.apache.aurora.scheduler.storage.Storage.Volatile;
-import org.apache.aurora.scheduler.storage.Storage.Work;
import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
import org.apache.aurora.scheduler.storage.entities.IJobInstanceUpdateEvent;
@@ -258,9 +257,11 @@ public class SnapshotStoreImpl implements SnapshotStore<Snapshot> {
@Timed("snapshot_create")
@Override
public Snapshot createSnapshot() {
- return storage.read(new Work.Quiet<Snapshot>() {
+ // It's important to perform snapshot creation in a write lock to ensure all upstream callers
+ // are correctly synchronized (e.g. during backup creation).
+ return storage.write(new MutateWork.Quiet<Snapshot>() {
@Override
- public Snapshot apply(StoreProvider storeProvider) {
+ public Snapshot apply(MutableStoreProvider storeProvider) {
Snapshot snapshot = new Snapshot();
// Capture timestamp to signify the beginning of a snapshot operation, apply after in case
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/277ac431/src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java b/src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java
index 7602d11..91789e7 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java
@@ -14,6 +14,7 @@
package org.apache.aurora.scheduler.storage.backup;
import java.io.File;
+import java.util.concurrent.ScheduledExecutorService;
import com.google.common.collect.ImmutableSet;
import com.google.common.io.Files;
@@ -48,6 +49,7 @@ import org.apache.aurora.scheduler.storage.backup.StorageBackup.StorageBackupImp
import org.apache.aurora.scheduler.storage.backup.StorageBackup.StorageBackupImpl.BackupConfig;
import org.apache.aurora.scheduler.storage.backup.TemporaryStorage.TemporaryStorageFactory;
import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
+import org.apache.aurora.scheduler.testing.FakeScheduledExecutor;
import org.easymock.Capture;
import org.junit.Before;
import org.junit.Test;
@@ -87,10 +89,15 @@ public class RecoveryTest extends EasyMockTest {
primaryStorage = createMock(Storage.class);
storeProvider = createMock(MutableStoreProvider.class);
shutDownNow = createMock(Command.class);
- clock = new FakeClock();
+ ScheduledExecutorService executor = createMock(ScheduledExecutorService.class);
+ clock = FakeScheduledExecutor.scheduleExecutor(executor);
TemporaryStorageFactory factory = new TemporaryStorageFactory();
- storageBackup =
- new StorageBackupImpl(snapshotStore, clock, new BackupConfig(backupDir, 5, INTERVAL));
+ storageBackup = new StorageBackupImpl(
+ snapshotStore,
+ clock,
+ new BackupConfig(backupDir, 5, INTERVAL),
+ executor);
+
recovery = new RecoveryImpl(backupDir, factory, primaryStorage, distributedStore, shutDownNow);
}
http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/277ac431/src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java b/src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java
index 15fc440..479c8bd 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java
@@ -15,6 +15,7 @@ package org.apache.aurora.scheduler.storage.backup;
import java.io.File;
import java.util.List;
+import java.util.concurrent.ScheduledExecutorService;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
@@ -40,6 +41,7 @@ import org.apache.aurora.gen.storage.StoredJob;
import org.apache.aurora.scheduler.storage.SnapshotStore;
import org.apache.aurora.scheduler.storage.backup.StorageBackup.StorageBackupImpl;
import org.apache.aurora.scheduler.storage.backup.StorageBackup.StorageBackupImpl.BackupConfig;
+import org.apache.aurora.scheduler.testing.FakeScheduledExecutor;
import org.junit.Before;
import org.junit.Test;
@@ -59,8 +61,9 @@ public class StorageBackupTest extends EasyMockTest {
@Before
public void setUp() {
delegate = createMock(new Clazz<SnapshotStore<Snapshot>>() { });
- clock = new FakeClock();
final File backupDir = Files.createTempDir();
+ ScheduledExecutorService executor = createMock(ScheduledExecutorService.class);
+ clock = FakeScheduledExecutor.scheduleExecutor(executor);
addTearDown(new TearDown() {
@Override
public void tearDown() throws Exception {
@@ -69,7 +72,7 @@ public class StorageBackupTest extends EasyMockTest {
});
config = new BackupConfig(backupDir, MAX_BACKUPS, INTERVAL);
clock.advance(Amount.of(365 * 30L, Time.DAYS));
- storageBackup = new StorageBackupImpl(delegate, clock, config);
+ storageBackup = new StorageBackupImpl(delegate, clock, config, executor);
}
@Test