PR review comments

Signed-off-by: Bukhtawar Khan <bukhtawa@amazon.com>
opensearch-project · Nov 19, 2022 · 8a5da20 · 8a5da20
1 parent 0bc8a30
commit 8a5da20
Show file tree

Hide file tree

Showing 5 changed files with 6 additions and 13 deletions.
diff --git a/server/src/main/java/org/opensearch/index/translog/transfer/BlobStoreTransferService.java b/server/src/main/java/org/opensearch/index/translog/transfer/BlobStoreTransferService.java
@@ -65,7 +65,6 @@ public void uploadBlob(final TransferFileSnapshot fileSnapshot, Iterable<String>
         try (InputStream inputStream = fileSnapshot.inputStream()) {
             blobStore.blobContainer(blobPath).writeBlobAtomic(fileSnapshot.getName(), inputStream, fileSnapshot.getContentLength(), true);
         } catch (Exception ex) {
-            logger.error(() -> new ParameterizedMessage("Failed to upload blob {}", fileSnapshot.getName()), ex);
             throw ex;
         }
     }

diff --git a/server/src/main/java/org/opensearch/index/translog/transfer/FileSnapshot.java b/server/src/main/java/org/opensearch/index/translog/transfer/FileSnapshot.java
@@ -38,14 +38,14 @@ public class FileSnapshot implements Closeable {
     @Nullable
     private byte[] content;
 
-    public FileSnapshot(Path path) throws IOException {
+    private FileSnapshot(Path path) throws IOException {
         Objects.requireNonNull(path);
         this.name = path.getFileName().toString();
         this.path = path;
         this.fileChannel = FileChannel.open(path, StandardOpenOption.READ);
     }
 
-    public FileSnapshot(String name, byte[] content) throws IOException {
+    private FileSnapshot(String name, byte[] content) {
         Objects.requireNonNull(name);
         this.name = name;
         this.content = content;
@@ -148,7 +148,7 @@ public boolean equals(Object o) {
      *
      * @opensearch.internal
      */
-    public static class TranslogFileSnapshot extends TransferFileSnapshot {
+    public static final class TranslogFileSnapshot extends TransferFileSnapshot {
 
         private final long generation;
 
@@ -183,7 +183,7 @@ public boolean equals(Object o) {
      *
      * @opensearch.internal
      */
-    public static class CheckpointFileSnapshot extends TransferFileSnapshot {
+    public static final class CheckpointFileSnapshot extends TransferFileSnapshot {
 
         private final long generation;
 

diff --git a/server/src/main/java/org/opensearch/index/translog/transfer/FileTransferException.java b/server/src/main/java/org/opensearch/index/translog/transfer/FileTransferException.java
@@ -24,11 +24,6 @@ public FileTransferException(TransferFileSnapshot fileSnapshot, Throwable cause)
         this.fileSnapshot = fileSnapshot;
     }
 
-    public FileTransferException(TransferFileSnapshot fileSnapshot, String message, Throwable cause) {
-        super(message, cause);
-        this.fileSnapshot = fileSnapshot;
-    }
-
     public TransferFileSnapshot getFileSnapshot() {
         return fileSnapshot;
     }

diff --git a/server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferManager.java b/server/src/main/java/org/opensearch/index/translog/transfer/TranslogTransferManager.java
@@ -100,7 +100,6 @@ public boolean transferSnapshot(TransferSnapshot transferSnapshot, TranslogTrans
                 }
             } catch (InterruptedException ex) {
                 exceptionList.forEach(ex::addSuppressed);
-                logger.error(() -> new ParameterizedMessage("Time failed for snapshot {}", transferSnapshot), ex);
                 Thread.currentThread().interrupt();
                 throw ex;
             }

diff --git a/...r/src/test/java/org/opensearch/index/translog/transfer/BlobStoreTransferServiceTests.java b/...r/src/test/java/org/opensearch/index/translog/transfer/BlobStoreTransferServiceTests.java
@@ -74,8 +74,8 @@ public void onFailure(Exception e) {
                 throw new AssertionError("Failed to perform uploadBlobAsync", e);
             }
         }, latch));
-        latch.await(1000, TimeUnit.MILLISECONDS);
-        assertEquals(true, succeeded.get());
+        assertTrue(latch.await(1000, TimeUnit.MILLISECONDS));
+        assertTrue(succeeded.get());
     }
 
     @Override