From 5eaf0a77b90c04d4a26f403c1fbf824bf8d6ed0f Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Sun, 21 Apr 2024 14:51:20 +0200 Subject: [PATCH] Remove JournalReader.getCurrentEntry() JournalReader.currentEntry is just a single-entry cache which is not used anywhere but tests. Remove the public method and private field. While we are at it, make SegmentedJournalReader.tryNext() state index management more obvious. JIRA: CONTROLLER-2115 Change-Id: If92293e8393e71a4c1f402a0370973872a9593ac Signed-off-by: Robert Varga --- .../atomix/storage/journal/JournalReader.java | 7 ------ .../journal/SegmentedJournalReader.java | 19 +++++---------- .../storage/journal/AbstractJournalTest.java | 23 ++++--------------- 3 files changed, 11 insertions(+), 38 deletions(-) diff --git a/atomix-storage/src/main/java/io/atomix/storage/journal/JournalReader.java b/atomix-storage/src/main/java/io/atomix/storage/journal/JournalReader.java index 700f40d1b6..40b700d070 100644 --- a/atomix-storage/src/main/java/io/atomix/storage/journal/JournalReader.java +++ b/atomix-storage/src/main/java/io/atomix/storage/journal/JournalReader.java @@ -44,13 +44,6 @@ public interface JournalReader extends AutoCloseable { */ long getFirstIndex(); - /** - * Returns the last read entry. - * - * @return The last read entry. - */ - Indexed getCurrentEntry(); - /** * Returns the next reader index. * diff --git a/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournalReader.java b/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournalReader.java index 42f40e0c72..0a137090d8 100644 --- a/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournalReader.java +++ b/atomix-storage/src/main/java/io/atomix/storage/journal/SegmentedJournalReader.java @@ -26,7 +26,6 @@ sealed class SegmentedJournalReader implements JournalReader permits Commi private JournalSegment currentSegment; private JournalSegmentReader currentReader; - private Indexed currentEntry; private long nextIndex; SegmentedJournalReader(final SegmentedJournal journal, final JournalSegment segment) { @@ -34,7 +33,6 @@ sealed class SegmentedJournalReader implements JournalReader permits Commi currentSegment = requireNonNull(segment); currentReader = segment.createReader(); nextIndex = currentSegment.firstIndex(); - currentEntry = null; } @Override @@ -42,11 +40,6 @@ sealed class SegmentedJournalReader implements JournalReader permits Commi return journal.getFirstSegment().firstIndex(); } - @Override - public final Indexed getCurrentEntry() { - return currentEntry; - } - @Override public final long getNextIndex() { return nextIndex; @@ -59,7 +52,6 @@ sealed class SegmentedJournalReader implements JournalReader permits Commi currentSegment = journal.getFirstSegment(); currentReader = currentSegment.createReader(); nextIndex = currentSegment.firstIndex(); - currentEntry = null; } @Override @@ -113,10 +105,11 @@ sealed class SegmentedJournalReader implements JournalReader permits Commi @Override public Indexed tryNext() { - var buf = currentReader.readBytes(nextIndex); + final var index = nextIndex; + var buf = currentReader.readBytes(index); if (buf == null) { final var nextSegment = journal.getNextSegment(currentSegment.firstIndex()); - if (nextSegment == null || nextSegment.firstIndex() != nextIndex) { + if (nextSegment == null || nextSegment.firstIndex() != index) { return null; } @@ -124,15 +117,15 @@ sealed class SegmentedJournalReader implements JournalReader permits Commi currentSegment = nextSegment; currentReader = currentSegment.createReader(); - buf = currentReader.readBytes(nextIndex); + buf = currentReader.readBytes(index); if (buf == null) { return null; } } final var entry = journal.serializer().deserialize(buf); - currentEntry = new Indexed<>(nextIndex++, entry, buf.readableBytes()); - return currentEntry; + nextIndex = index + 1; + return new Indexed<>(index, entry, buf.readableBytes()); } @Override diff --git a/atomix-storage/src/test/java/io/atomix/storage/journal/AbstractJournalTest.java b/atomix-storage/src/test/java/io/atomix/storage/journal/AbstractJournalTest.java index 14e59e5d7a..58ae35d711 100644 --- a/atomix-storage/src/test/java/io/atomix/storage/journal/AbstractJournalTest.java +++ b/atomix-storage/src/test/java/io/atomix/storage/journal/AbstractJournalTest.java @@ -16,6 +16,7 @@ */ package io.atomix.storage.journal; +import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; @@ -119,14 +120,12 @@ public abstract class AbstractJournalTest { var entry1 = reader.tryNext(); assertNotNull(entry1); assertEquals(1, entry1.index()); - assertEquals(entry1, reader.getCurrentEntry()); // Test reading a second entry assertEquals(2, reader.getNextIndex()); var entry2 = reader.tryNext(); assertNotNull(entry2); assertEquals(2, entry2.index()); - assertEquals(entry2, reader.getCurrentEntry()); assertEquals(3, reader.getNextIndex()); assertNull(reader.tryNext()); @@ -135,13 +134,11 @@ public abstract class AbstractJournalTest { entry1 = reader.tryNext(); assertNotNull(entry1); assertEquals(1, entry1.index()); - assertEquals(entry1, reader.getCurrentEntry()); assertEquals(2, reader.getNextIndex()); entry2 = reader.tryNext(); assertNotNull(entry2); assertEquals(2, entry2.index()); - assertEquals(entry2, reader.getCurrentEntry()); assertNull(reader.tryNext()); // Reset the reader. @@ -152,13 +149,11 @@ public abstract class AbstractJournalTest { entry1 = reader.tryNext(); assertNotNull(entry1); assertEquals(1, entry1.index()); - assertEquals(entry1, reader.getCurrentEntry()); assertEquals(2, reader.getNextIndex()); entry2 = reader.tryNext(); assertNotNull(entry2); assertEquals(2, entry2.index()); - assertEquals(entry2, reader.getCurrentEntry()); assertNull(reader.tryNext()); // Truncate the journal and write a different entry. @@ -173,14 +168,10 @@ public abstract class AbstractJournalTest { // Reset the reader to a specific index and read the last entry again. reader.reset(2); - final var current = reader.getCurrentEntry(); - assertNotNull(current); - assertEquals(1, current.index()); assertEquals(2, reader.getNextIndex()); entry2 = reader.tryNext(); assertNotNull(entry2); assertEquals(2, entry2.index()); - assertEquals(entry2, reader.getCurrentEntry()); assertNull(reader.tryNext()); } } @@ -267,19 +258,17 @@ public abstract class AbstractJournalTest { var entry = reader.tryNext(); assertNotNull(entry); assertEquals(i, entry.index()); - assertEquals(32, entry.entry().bytes().length); + assertArrayEquals(ENTRY.bytes(), entry.entry().bytes()); reader.reset(i); entry = reader.tryNext(); assertNotNull(entry); assertEquals(i, entry.index()); - assertEquals(32, entry.entry().bytes().length); + assertArrayEquals(ENTRY.bytes(), entry.entry().bytes()); if (i > 6) { reader.reset(i - 5); - final var current = reader.getCurrentEntry(); - assertNotNull(current); - assertEquals(i - 6, current.index()); assertEquals(i - 5, reader.getNextIndex()); + assertNotNull(reader.tryNext()); reader.reset(i + 1); } @@ -291,7 +280,7 @@ public abstract class AbstractJournalTest { entry = reader.tryNext(); assertNotNull(entry); assertEquals(i, entry.index()); - assertEquals(32, entry.entry().bytes().length); + assertArrayEquals(ENTRY.bytes(), entry.entry().bytes()); } } } @@ -358,13 +347,11 @@ public abstract class AbstractJournalTest { journal.compact(entriesPerSegment * 5 + 1); - assertNull(uncommittedReader.getCurrentEntry()); assertEquals(entriesPerSegment * 5 + 1, uncommittedReader.getNextIndex()); var entry = uncommittedReader.tryNext(); assertNotNull(entry); assertEquals(entriesPerSegment * 5 + 1, entry.index()); - assertNull(committedReader.getCurrentEntry()); assertEquals(entriesPerSegment * 5 + 1, committedReader.getNextIndex()); entry = committedReader.tryNext(); assertNotNull(entry); -- 2.36.6