aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject incubator-aurora git commit: Saving scheduler backups asynchronously.
Date Mon, 23 Feb 2015 21:08:03 GMT
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 <maxim@apache.org>
Authored: Mon Feb 23 13:05:20 2015 -0800
Committer: Maxim Khutornenko <maxim@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


Mime
View raw message