From c4ff33a9e577e1c9951409f6d640d149e23a19f8 Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Mon, 1 Apr 2019 18:58:55 -0700 Subject: [PATCH] Make region files more reliable to write to Previously we would write to header before writing our chunk data, which opens a window for corruption (or we would overwrite entirely). Now the saving process has been changed to follow this chain of events: 1. We always allocate a new space to write so we do not potentially overwrite and corrupt the current data 2. Write the chunk data first (the order of the fields in the chunk data isn't relevant though) 3. Flush to disk 4. Write to the region header last 5. Flush to disk 6. Then we free the previous space allocated With this chain of events it is impossible for a chunk write to corrupt a region file, unless the operating system has lied and we have NOT flushed to disk. However server administrators are still recommended to continue performing regular backups. Note that when Mojang finally decides to change their region format to deal with oversized chunks this patch must be changed to deal with whatever system they decide to impose. We also make use of two flushes to disk per chunk save (to ensure ordering and ensure data has gone to disk), so this will negatively affect save performance. --- ...gion-files-more-reliable-to-write-to.patch | 187 ++++++++++++++++++ 1 file changed, 187 insertions(+) create mode 100644 Spigot-Server-Patches/0434-Make-region-files-more-reliable-to-write-to.patch diff --git a/Spigot-Server-Patches/0434-Make-region-files-more-reliable-to-write-to.patch b/Spigot-Server-Patches/0434-Make-region-files-more-reliable-to-write-to.patch new file mode 100644 index 0000000000..3ddd958f52 --- /dev/null +++ b/Spigot-Server-Patches/0434-Make-region-files-more-reliable-to-write-to.patch @@ -0,0 +1,187 @@ +From 9cb7285df76b6bef983544eb9ccd85b308d3174e Mon Sep 17 00:00:00 2001 +From: Spottedleaf +Date: Mon, 1 Apr 2019 18:57:32 -0700 +Subject: [PATCH] Make region files more reliable to write to + +Previously we would write to header before writing our chunk data, +which opens a window for corruption (or we would overwrite entirely). +Now the saving process has been changed to follow this chain of events: + +1. We always allocate a new space to write so we do not potentially +overwrite and corrupt the current data +2. Write the chunk data first (the order of the fields in +the chunk data isn't relevant though) +3. Flush to disk +4. Write to the region header last +5. Flush to disk +6. Then we free the previous space allocated + +With this chain of events it is impossible for a chunk write to corrupt +a region file, unless the operating system has lied and we have NOT flushed +to disk. + +However server administrators are still recommended to continue performing +regular backups. + +Note that when Mojang finally decides to change their region format +to deal with oversized chunks this patch must be changed to deal with +whatever system they decide to impose. + +We also make use of two flushes to disk per chunk save (to ensure +ordering and ensure data has gone to disk), so this will negatively +affect save performance. + +diff --git a/src/main/java/net/minecraft/server/RegionFile.java b/src/main/java/net/minecraft/server/RegionFile.java +index 82f7af46f..2e2844133 100644 +--- a/src/main/java/net/minecraft/server/RegionFile.java ++++ b/src/main/java/net/minecraft/server/RegionFile.java +@@ -29,7 +29,7 @@ public class RegionFile { + private RandomAccessFile c;private RandomAccessFile getDataFile() { return c; } // Paper - OBFHELPER + private final int[] d = new int[1024];private int[] offsets = d; // Paper - OBFHELPER + private final int[] e = new int[1024];private int[] timestamps = e; // Paper - OBFHELPER +- private List f; ++ private List f; private List getFreeSectors() { return this.f; } // Paper - OBFHELPER + private int g; + private long h; + +@@ -211,14 +211,15 @@ public class RegionFile { + protected synchronized void a(int i, int j, byte[] abyte, int k) { + try { + int l = this.getOffset(i, j); +- int i1 = l >> 8; +- int j1 = l & 255; ++ int i1 = l >> 8; final int oldSectorOffset = i1; // Paper - store variable for later ++ int j1 = l & 255; final int oldSectorCount; // Paper - store variable for later + // Spigot start + if (j1 == 255) { + this.c.seek(i1 * 4096); + j1 = (this.c.readInt() + 4) / 4096 + 1; + } + // Spigot end ++ oldSectorCount = j1; // Paper - store variable for later (watch out for re-assignments of j1) + int k1 = (k + 5) / 4096 + 1; + + if (k1 >= 256) { +@@ -229,14 +230,12 @@ public class RegionFile { + // Spigot end + } + +- if (i1 != 0 && j1 == k1) { ++ if (false && i1 != 0 && j1 == k1) { // Paper - We never want to overrite old data + this.a(i1, abyte, k); + } else { + int l1; + +- for (l1 = 0; l1 < j1; ++l1) { +- this.f.set(i1 + l1, true); +- } ++ // Paper - We do not free old sectors until we are done writing the new chunk data + + l1 = this.f.indexOf(true); + int i2 = 0; +@@ -263,13 +262,13 @@ public class RegionFile { + + if (i2 >= k1) { + i1 = l1; +- this.a(i, j, l1 << 8 | (k1 > 255 ? 255 : k1)); // Spigot ++ //this.a(i, j, l1 << 8 | (k1 > 255 ? 255 : k1)); // Spigot // Paper - We only write to header after we've written chunk data + + for (j2 = 0; j2 < k1; ++j2) { + this.f.set(i1 + j2, false); + } + +- this.a(i1, abyte, k); ++ this.writeChunk(i, j,i1 << 8 | (k1 > 255 ? 255 : k1), i1, abyte, k); // Paper - Ensure we do not corrupt region files + } else { + this.c.seek(this.c.length()); + i1 = this.f.size(); +@@ -280,9 +279,14 @@ public class RegionFile { + } + + this.g += 4096 * k1; +- this.a(i1, abyte, k); +- this.a(i, j, i1 << 8 | (k1 > 255 ? 255 : k1)); // Spigot ++ this.writeChunk(i, j, i1 << 8 | (k1 > 255 ? 255 : k1), i1, abyte, k); // Paper - Ensure we do not corrupt region files ++ } ++ ++ // Paper start - Now that we've written the new chunk we can free the old data ++ for (int off = 0; off < oldSectorCount; ++off) { ++ this.getFreeSectors().set(oldSectorOffset + off, true); + } ++ // Paper end + } + + this.b(i, j, (int) (SystemUtils.getTimeMillis() / 1000L)); +@@ -292,10 +296,10 @@ public class RegionFile { + + } + ++ private void writeChunkData(final int sectorOffset, final byte[] data, final int dataLength) throws IOException { this.a(sectorOffset, data, dataLength); } // Paper - OBFHELPER + private void a(int i, byte[] abyte, int j) throws IOException { + this.c.seek((long) (i * 4096)); +- this.c.writeInt(j + 1); +- this.c.writeByte(2); ++ this.writeIntAndByte(j + 1, (byte)2); // Paper - Avoid 4 io write calls + this.c.write(abyte, 0, j); + } + +@@ -311,16 +315,17 @@ public class RegionFile { + return this.getOffset(i, j) != 0; + } + ++ private void updateChunkHeader(final int x, final int z, final int offset) throws IOException { this.a(x, z, offset); } // Paper - OBFHELPER + private void a(int i, int j, int k) throws IOException { + this.d[i + j * 32] = k; + this.c.seek((long) ((i + j * 32) * 4)); +- this.c.writeInt(k); ++ this.writeInt(k); // Paper - Avoid 3 io write calls + } + + private void b(int i, int j, int k) throws IOException { + this.e[i + j * 32] = k; + this.c.seek((long) (4096 + (i + j * 32) * 4)); +- this.c.writeInt(k); ++ this.writeInt(k); // Paper - Avoid 3 io write calls + } + + public void close() throws IOException { +@@ -331,6 +336,36 @@ public class RegionFile { + } + + // Paper start ++ private void syncRegionFile() throws IOException { ++ this.getDataFile().getFD().sync(); // rethrow exception as we want to avoid corrupting a regionfile ++ } ++ ++ private final java.nio.ByteBuffer scratchBuffer = java.nio.ByteBuffer.allocate(8); ++ ++ private void writeInt(final int value) throws IOException { ++ synchronized (scratchBuffer) { ++ this.scratchBuffer.putInt(0, value); ++ this.getDataFile().write(this.scratchBuffer.array(), 0, 4); ++ } ++ } ++ ++ // writes v1 then v2 ++ private void writeIntAndByte(final int v1, final byte v2) throws IOException { ++ synchronized (scratchBuffer) { ++ this.scratchBuffer.putInt(0, v1); ++ this.scratchBuffer.put(4, v2); ++ this.getDataFile().write(this.scratchBuffer.array(), 0, 5); ++ } ++ } ++ ++ private void writeChunk(final int x, final int z, final int chunkHeaderData, ++ final int chunkOffset, final byte[] chunkData, final int chunkDataLength) throws IOException { ++ this.writeChunkData(chunkOffset, chunkData, chunkDataLength); ++ this.syncRegionFile(); // Sync is required to ensure the previous data is written successfully ++ this.updateChunkHeader(x, z, chunkHeaderData); ++ this.syncRegionFile(); // Ensure header changes go through ++ } ++ + public synchronized void deleteChunk(int j1) { + backup(); + int k = offsets[j1]; +-- +2.21.0 +