From 8ada891ba81e8c1032e86657bc8d0cde75208d11 Mon Sep 17 00:00:00 2001 From: "Brian S. O'Neill" Date: Tue, 22 Oct 2024 11:46:01 -0700 Subject: [PATCH] Fix a bug when extending a term causes the contiguous range to extend when it shouldn't. --- .../java/org/cojen/tupl/repl/FileTermLog.java | 63 +++++++++++++++++-- .../org/cojen/tupl/repl/FileTermLogTest.java | 31 +++++++++ 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/cojen/tupl/repl/FileTermLog.java b/src/main/java/org/cojen/tupl/repl/FileTermLog.java index 3d243d87f..efe6d1fd1 100644 --- a/src/main/java/org/cojen/tupl/repl/FileTermLog.java +++ b/src/main/java/org/cojen/tupl/repl/FileTermLog.java @@ -699,6 +699,11 @@ public void finishTerm(long endPosition) { mLogVersion++; if (endPosition >= mLogEndPosition) { + // Wait for all work tasks to complete, ensuring that any lingering truncate + // operations are finished before extending the term. + synchronized (mWorker) { + mWorker.join(false); + } mLogEndPosition = endPosition; releaseExclusive(); return; @@ -725,14 +730,47 @@ public void finishTerm(long endPosition) { } if (!mNonContigWriters.isEmpty()) { + List replacements = null; + Iterator it = mNonContigWriters.iterator(); while (it.hasNext()) { SegmentWriter writer = it.next(); if (writer.mWriterStartPosition >= endPosition) { + // The writer is completely ahead of the term end position. it.remove(); - } else if (endPosition < writer.mWriterHighestPosition) { + continue; + } + + if (endPosition < writer.mWriterHighestPosition) { writer.mWriterHighestPosition = endPosition; } + + if (endPosition <= writer.mWriterPosition) { + // The writer overlaps the term end position. Remove it from the queue + // and replace it with a non-cached instance to be used for gap filling + // later. If the writer isn't removed, and the term is later extended, + // then the contiguous region might falsely advance too far ahead. + it.remove(); + + var replacement = new SegmentWriter(); + replacement.mWriterVersion = writer.mWriterVersion; + replacement.mWriterPrevTerm = writer.mWriterPrevTerm; + replacement.mWriterStartPosition = writer.mWriterStartPosition; + replacement.mWriterPosition = endPosition; + replacement.mWriterHighestPosition = writer.mWriterHighestPosition; + + if (replacements == null) { + replacements = new ArrayList<>(); + } + + replacements.add(replacement); + } + } + + if (replacements != null) { + for (SegmentWriter writer : replacements) { + mNonContigWriters.add(writer); + } } } @@ -1069,11 +1107,6 @@ Segment segmentForWriting(SegmentWriter writer, long position) throws IOExceptio if (startPosition != startSegment.mStartPosition || startSegment.mMaxLength != 0) { throw new AssertionError(startSegment); } - // Wait for all work tasks to complete, ensuring that the truncate operation - // against the segment is finished before attempting to untruncate it. - synchronized (mWorker) { - mWorker.join(false); - } startSegment.untruncate(maxLength); segment = startSegment; } @@ -1156,6 +1189,8 @@ void writeFinished(SegmentWriter writer, long currentPosition, long highestPosit if (currentPosition > contigPosition) { contigPosition = currentPosition; + long fallbackHighest = 0; + // Remove non-contiguous writers that are now in the contiguous region. while (true) { SegmentWriter next = mNonContigWriters.peek(); @@ -1175,9 +1210,25 @@ void writeFinished(SegmentWriter writer, long currentPosition, long highestPosit if (nextHighest > highestPosition && highestPosition <= contigPosition) { highestPosition = nextHighest; } + + fallbackHighest = Math.max(fallbackHighest, nextHighest); + } + + if (contigPosition > endPosition) { + // Although the contigPosition could be clamped to the endPosition, this + // case shouldn't happen unless there's something wrong with the term + // extension logic. + releaseExclusive(); + throw new AssertionError(contigPosition + " > " + endPosition); } mLogContigPosition = contigPosition; + + if (highestPosition > contigPosition) { + // It won't be applied if too high, so try to use a highest position which + // was observed earlier. + highestPosition = fallbackHighest; + } } applyHighest: { diff --git a/src/test/java/org/cojen/tupl/repl/FileTermLogTest.java b/src/test/java/org/cojen/tupl/repl/FileTermLogTest.java index f52e9abe0..d951868e7 100644 --- a/src/test/java/org/cojen/tupl/repl/FileTermLogTest.java +++ b/src/test/java/org/cojen/tupl/repl/FileTermLogTest.java @@ -1202,6 +1202,37 @@ public void writeTruncateExtend2() throws Exception { reader.release(); } + @Test + public void writeTruncateExtend3() throws Exception { + // Truncate and extend over non-contiguous ranges. + + LogWriter writer1 = mLog.openWriter(0); + var data1 = new byte[100]; + Arrays.fill(data1, (byte) 1); + write(writer1, data1, 2000); + + LogWriter writer2 = mLog.openWriter(200); + var data2 = new byte[100]; + Arrays.fill(data2, (byte) 2); + write(writer2, data2, 2000); + + LogWriter writer3 = mLog.openWriter(400); + var data3 = new byte[100]; + Arrays.fill(data3, (byte) 3); + write(writer3, data3, 2000); + + mLog.finishTerm(250); + mLog.finishTerm(275); + + // Fill in the first gap. + Arrays.fill(data1, (byte) 4); + write(writer1, data1, 2000); + + var info = new LogInfo(); + mLog.captureHighest(info); + assertEquals(250, info.mHighestPosition); + } + private static void write(LogWriter writer, byte[] data) throws IOException { assertTrue(writer.write(data, 0, data.length, writer.position() + data.length) > 0); }