From 3526a7735a3fbe62773d5edd23290e5e4d94b197 Mon Sep 17 00:00:00 2001
From: Robert Varga <robert.varga@pantheon.tech>
Date: Sun, 5 May 2024 23:03:03 +0200
Subject: [PATCH] Centralize JournalSegmentFile creation

There are only two callers of the constructor: we are either creating
a new file (in which case we initialize the descriptor) or we are
opening a file (in which case we read the descriptor).

JIRA: CONTROLLER-2099
Change-Id: Id3943a637067d3b7f2ba7384e417e935df65276a
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
---
 .../storage/journal/JournalSegmentFile.java   | 20 ++++++++-
 .../storage/journal/SegmentedJournal.java     | 44 ++++++-------------
 2 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/atomix-storage/src/main/java/io/atomix/storage/journal/JournalSegmentFile.java b/atomix-storage/src/main/java/io/atomix/storage/journal/JournalSegmentFile.java
index adc9910087..a7ab481dbb 100644
--- a/atomix-storage/src/main/java/io/atomix/storage/journal/JournalSegmentFile.java
+++ b/atomix-storage/src/main/java/io/atomix/storage/journal/JournalSegmentFile.java
@@ -19,6 +19,8 @@ import static java.util.Objects.requireNonNull;
 
 import com.google.common.base.MoreObjects;
 import java.io.File;
+import java.io.IOException;
+import java.io.RandomAccessFile;
 import java.nio.file.Path;
 import org.eclipse.jdt.annotation.NonNull;
 
@@ -35,11 +37,27 @@ final class JournalSegmentFile {
     private final @NonNull JournalSegmentDescriptor descriptor;
     private final @NonNull Path path;
 
-    JournalSegmentFile(final Path path, final JournalSegmentDescriptor descriptor) {
+    private JournalSegmentFile(final Path path, final JournalSegmentDescriptor descriptor) {
         this.path = requireNonNull(path);
         this.descriptor = requireNonNull(descriptor);
     }
 
+    static @NonNull JournalSegmentFile createNew(final String name, final File directory,
+            final JournalSegmentDescriptor descriptor) throws IOException {
+        final var file = createSegmentFile(name, directory, descriptor.id());
+        try (var raf = new RandomAccessFile(file, "rw")) {
+            raf.setLength(descriptor.maxSegmentSize());
+            raf.write(descriptor.toArray());
+        }
+        return new JournalSegmentFile(file.toPath(), descriptor);
+    }
+
+    static @NonNull JournalSegmentFile openExisting(final Path path) throws IOException {
+        // read the descriptor
+        final var descriptor = JournalSegmentDescriptor.readFrom(path);
+        return new JournalSegmentFile(path, descriptor);
+    }
+
     /**
      * Returns the segment file.
      *
diff --git a/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournal.java b/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournal.java
index 9fef881edd..7e821277ce 100644
--- a/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournal.java
+++ b/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournal.java
@@ -21,7 +21,6 @@ import static java.util.Objects.requireNonNull;
 
 import java.io.File;
 import java.io.IOException;
-import java.io.RandomAccessFile;
 import java.util.Collection;
 import java.util.Map;
 import java.util.TreeMap;
@@ -389,37 +388,24 @@ public final class SegmentedJournal<E> implements Journal<E> {
    * Creates a new segment.
    */
   JournalSegment createSegment(long id, long index) {
-    final var segmentFile = JournalSegmentFile.createSegmentFile(name, directory, id);
-    final var descriptor = JournalSegmentDescriptor.builder()
-        .withId(id)
-        .withIndex(index)
-        .withMaxSegmentSize(maxSegmentSize)
-        .withMaxEntries(maxEntriesPerSegment)
-        .withUpdated(System.currentTimeMillis())
-        .build();
-
-    try (var raf = new RandomAccessFile(segmentFile, "rw")) {
-      raf.setLength(maxSegmentSize);
-      raf.write(descriptor.toArray());
+    final JournalSegmentFile file;
+    try {
+      file = JournalSegmentFile.createNew(name, directory, JournalSegmentDescriptor.builder()
+          .withId(id)
+          .withIndex(index)
+          .withMaxSegmentSize(maxSegmentSize)
+          .withMaxEntries(maxEntriesPerSegment)
+          .withUpdated(System.currentTimeMillis())
+          .build());
     } catch (IOException e) {
       throw new StorageException(e);
     }
 
-    final var segment = newSegment(new JournalSegmentFile(segmentFile.toPath(), descriptor));
+    final var segment = new JournalSegment(file, storageLevel, maxEntrySize, indexDensity);
     LOG.debug("Created segment: {}", segment);
     return segment;
   }
 
-  /**
-   * Creates a new segment instance.
-   *
-   * @param segmentFile The segment file.
-   * @return The segment instance.
-   */
-  protected JournalSegment newSegment(JournalSegmentFile segmentFile) {
-    return new JournalSegment(segmentFile, storageLevel, maxEntrySize, indexDensity);
-  }
-
   /**
    * Loads all segments from disk.
    *
@@ -436,20 +422,18 @@ public final class SegmentedJournal<E> implements Journal<E> {
 
       // If the file looks like a segment file, attempt to load the segment.
       if (JournalSegmentFile.isSegmentFile(name, file)) {
-        final var path = file.toPath();
-        // read the descriptor
-        final JournalSegmentDescriptor descriptor;
+        final JournalSegmentFile segmentFile;
         try {
-          descriptor = JournalSegmentDescriptor.readFrom(path);
+          segmentFile = JournalSegmentFile.openExisting(file.toPath());
         } catch (IOException e) {
           throw new StorageException(e);
         }
 
         // Load the segment.
-        final var segment = newSegment(new JournalSegmentFile(path, descriptor));
-        LOG.debug("Loaded disk segment: {} ({})", descriptor.id(), path);
+        LOG.debug("Loaded disk segment: {} ({})", segmentFile.descriptor().id(), segmentFile.path());
 
         // Add the segment to the segments list.
+        final var segment = new JournalSegment(segmentFile, storageLevel, maxEntrySize, indexDensity);
         segments.put(segment.firstIndex(), segment);
       }
     }
-- 
2.36.6