Improve JournalSegment lifecycle

Disconnect JournalSegment from AutoCloseable, making its close() method
its own thing.

This makes it clear there are really two lifecycle entry points:
- close()
- delete()
Callers of delete() always invoke close(), so let's make sure delete()
does this internally.

JIRA: CONTROLLER-2099
Change-Id: I64b3c12f7a3301e513a36b8a156d8495a405313f
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/atomix-storage/src/main/java/io/atomix/storage/journal/JournalSegment.java b/atomix-storage/src/main/java/io/atomix/storage/journal/JournalSegment.java
index e8954bcc7fb4c4667b9d6d545795b5eee3fa532f..a7f4c5aadc5c2845723ec362f60412b47b373c83 100644 (file)
--- a/atomix-storage/src/main/java/io/atomix/storage/journal/JournalSegment.java
+++ b/atomix-storage/src/main/java/io/atomix/storage/journal/JournalSegment.java
@@ -30,13 +30,17 @@ import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.eclipse.jdt.annotation.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Log segment.
  *
  * @author <a href="http://github.com/kuujo">Jordan Halterman</a>
  */
-final class JournalSegment implements AutoCloseable {
+final class JournalSegment {
+  private static final Logger LOG = LoggerFactory.getLogger(JournalSegment.class);
+
   private final JournalSegmentFile file;
   private final StorageLevel storageLevel;
   private final int maxEntrySize;
@@ -188,7 +192,7 @@ final class JournalSegment implements AutoCloseable {
    *
    * @param reader the closed segment reader
    */
-  void closeReader(JournalSegmentReader reader) {
+  void closeReader(final JournalSegmentReader reader) {
     if (readers.remove(reader)) {
       release();
     }
@@ -208,19 +212,19 @@ final class JournalSegment implements AutoCloseable {
    *
    * @return indicates whether the segment is open
    */
-  public boolean isOpen() {
+  boolean isOpen() {
     return open;
   }
 
   /**
    * Closes the segment.
    */
-  @Override
-  public void close() {
+  void close() {
     if (!open) {
       return;
     }
 
+    LOG.debug("Closing segment: {}", this);
     open = false;
     readers.forEach(JournalSegmentReader::close);
     if (references.get() == 0) {
@@ -241,6 +245,8 @@ final class JournalSegment implements AutoCloseable {
    * Deletes the segment.
    */
   void delete() {
+    close();
+    LOG.debug("Deleting segment: {}", this);
     try {
       Files.deleteIfExists(file.path());
     } catch (IOException e) {

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 c2ca89b40ac381b18f24720d4b62761aeaf4da52..9fef881edd64d513ce1b9123f180d6be7e222429 100644 (file)
--- a/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournal.java
+++ b/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournal.java
@@ -290,15 +290,12 @@ public final class SegmentedJournal<E> implements Journal<E> {
     assertOpen();
 
     // If the index already equals the first segment index, skip the reset.
-    JournalSegment firstSegment = getFirstSegment();
+    final var firstSegment = getFirstSegment();
     if (index == firstSegment.firstIndex()) {
       return firstSegment;
     }
 
-    for (JournalSegment segment : segments.values()) {
-      segment.close();
-      segment.delete();
-    }
+    segments.values().forEach(JournalSegment::delete);
     segments.clear();
 
     currentSegment = createSegment(1, index);
@@ -384,7 +381,6 @@ public final class SegmentedJournal<E> implements Journal<E> {
    */
   synchronized void removeSegment(JournalSegment segment) {
     segments.remove(segment.firstIndex());
-    segment.close();
     segment.delete();
     resetCurrentSegment();
   }
@@ -470,7 +466,6 @@ public final class SegmentedJournal<E> implements Journal<E> {
         corrupted = true;
       }
       if (corrupted) {
-        segment.close();
         segment.delete();
         iterator.remove();
       }
@@ -550,11 +545,7 @@ public final class SegmentedJournal<E> implements Journal<E> {
       final var compactSegments = segments.headMap(segmentEntry.getValue().firstIndex());
       if (!compactSegments.isEmpty()) {
         LOG.debug("{} - Compacting {} segment(s)", name, compactSegments.size());
-        for (JournalSegment segment : compactSegments.values()) {
-          LOG.trace("Deleting segment: {}", segment);
-          segment.close();
-          segment.delete();
-        }
+        compactSegments.values().forEach(JournalSegment::delete);
         compactSegments.clear();
         resetHead(segmentEntry.getValue().firstIndex());
       }
@@ -563,10 +554,7 @@ public final class SegmentedJournal<E> implements Journal<E> {
 
   @Override
   public void close() {
-    segments.values().forEach(segment -> {
-      LOG.debug("Closing segment: {}", segment);
-      segment.close();
-    });
+    segments.values().forEach(JournalSegment::close);
     currentSegment = null;
     open = false;
   }