review changes

Author: clohfink

priam/src/main/java/com/netflix/priam/aws/DataPart.java

@@ -63,4 +63,4 @@ public byte[] getPartData() {
     public byte[] getMd5() {
         return md5;
     }
-}
+}

priam/src/main/java/com/netflix/priam/aws/S3PartUploader.java

@@ -97,4 +97,4 @@ public Void retriableCall() throws AmazonClientException, BackupRestoreException
                 "Picked up part {} size {}", dataPart.getPartNo(), dataPart.getPartData().length);
         return uploadPart();
     }
-}
+}

priam/src/main/java/com/netflix/priam/config/IConfiguration.java

@@ -1147,6 +1147,6 @@ default boolean skipMetaFileValidationOnRestore() {
      * @return true to use reusable ByteBuffer implementation, false for legacy implementation
      */
     default boolean useReusableBufferForMultipartUploads() {
-        return false;
+        return true;
     }
 }

priam/src/test/java/com/netflix/priam/backup/TestS3FileSystem.java

@@ -29,8 +29,10 @@
 import com.google.common.collect.Lists;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
+import com.netflix.priam.aws.DataPart;
 import com.netflix.priam.aws.RemoteBackupPath;
 import com.netflix.priam.aws.S3FileSystem;
+import com.netflix.priam.aws.S3PartUploader;
 import com.netflix.priam.backup.AbstractBackupPath.BackupFileType;
 import com.netflix.priam.compress.CompressionType;
 import com.netflix.priam.config.IConfiguration;
@@ -80,6 +82,7 @@ public TestS3FileSystem() {
 
     @BeforeClass
     public static void setUp() {
+        new MockS3PartUploader();
         new MockAmazonS3Client();
         if (!DIR.exists()) DIR.mkdirs();
     }
@@ -115,6 +118,41 @@ public void testFileUploadDeleteExists() throws Exception {
         Assert.assertFalse(fs.checkObjectExists(Paths.get(backupfile.getRemotePath())));
     }
 
+    @Test
+    public void testFileUploadFailures() throws Exception {
+        MockS3PartUploader.setup();
+        MockS3PartUploader.partFailure = true;
+        long noOfFailures = backupMetrics.getInvalidUploads().count();
+        S3FileSystem fs = injector.getInstance(S3FileSystem.class);
+        RemoteBackupPath backupfile = injector.getInstance(RemoteBackupPath.class);
+        backupfile.parseLocal(localFile(), BackupFileType.META_V2);
+        try {
+            // temporary hack to allow tests to complete in a timely fashion
+            // This will be removed once we stop inheriting from AbstractFileSystem
+            fs.uploadAndDeleteInternal(backupfile, Instant.EPOCH, 0 /* retries */);
+        } catch (BackupRestoreException e) {
+            // ignore
+        }
+        Assert.assertEquals(0, MockS3PartUploader.compattempts);
+        Assert.assertEquals(1, backupMetrics.getInvalidUploads().count() - noOfFailures);
+    }
+
+    @Test
+    public void testFileUploadCompleteFailure() throws Exception {
+        MockS3PartUploader.setup();
+        MockS3PartUploader.completionFailure = true;
+        S3FileSystem fs = injector.getInstance(S3FileSystem.class);
+        fs.setS3Client(new MockAmazonS3Client().getMockInstance());
+        RemoteBackupPath backupfile = injector.getInstance(RemoteBackupPath.class);
+        backupfile.parseLocal(localFile(), BackupFileType.META_V2);
+        try {
+            // temporary hack to allow tests to complete in a timely fashion
+            // This will be removed once we stop inheriting from AbstractFileSystem
+            fs.uploadAndDeleteInternal(backupfile, Instant.EPOCH, 0 /* retries */);
+        } catch (BackupRestoreException e) {
+            // ignore
+        }
+    }
 
     @Test
     public void testCleanupAdd() throws Exception {
@@ -251,6 +289,48 @@ private File localFile() throws IOException {
         return file;
     }
 
+    // Mock S3PartUploader class
+    static class MockS3PartUploader extends MockUp<S3PartUploader> {
+        static int compattempts = 0;
+        static int partAttempts = 0;
+        static boolean partFailure = false;
+        static boolean completionFailure = false;
+        private static List<PartETag> partETags;
+
+        @Mock
+        public void $init(AmazonS3 client, DataPart dp, List<PartETag> partETags) {
+            MockS3PartUploader.partETags = partETags;
+        }
+
+        @Mock
+        private Void uploadPart() throws AmazonClientException, BackupRestoreException {
+            ++partAttempts;
+            if (partFailure) throw new BackupRestoreException("Test exception");
+            partETags.add(new PartETag(0, null));
+            return null;
+        }
+
+        @Mock
+        public CompleteMultipartUploadResult completeUpload() throws BackupRestoreException {
+            ++compattempts;
+            if (completionFailure) throw new BackupRestoreException("Test exception");
+
+            return null;
+        }
+
+        @Mock
+        public Void retriableCall() throws AmazonClientException, BackupRestoreException {
+            logger.info("MOCK UPLOADING...");
+            return uploadPart();
+        }
+
+        public static void setup() {
+            compattempts = 0;
+            partAttempts = 0;
+            partFailure = false;
+            completionFailure = false;
+        }
+    }
 
     static class MockAmazonS3Client extends MockUp<AmazonS3Client> {
         private boolean ruleAvailable = false;