Skip to content

Commit

Permalink
Fix a bug when extending a term causes the contiguous range to extend…
Browse files Browse the repository at this point in the history
… when it shouldn't.
  • Loading branch information
broneill committed Oct 22, 2024
1 parent 5bbedbb commit 8ada891
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 6 deletions.
63 changes: 57 additions & 6 deletions src/main/java/org/cojen/tupl/repl/FileTermLog.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -725,14 +730,47 @@ public void finishTerm(long endPosition) {
}

if (!mNonContigWriters.isEmpty()) {
List<SegmentWriter> replacements = null;

Iterator<SegmentWriter> 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);
}
}
}

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
Expand All @@ -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: {
Expand Down
31 changes: 31 additions & 0 deletions src/test/java/org/cojen/tupl/repl/FileTermLogTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 8ada891

Please sign in to comment.