Skip to content

Commit 4ac79f9

Browse files
authored
Verify translog checksum before UUID check (#49394)
When opening a translog file, we check whether the UUID matches what we expect (the UUID from the latest commit). The UUID check can in certain cases fail when the translog is corrupted. This commit changes the ordering of the checks so that corruption is detected first.
1 parent 364b21a commit 4ac79f9

File tree

2 files changed

+14
-9
lines changed

2 files changed

+14
-9
lines changed

server/src/main/java/org/elasticsearch/index/translog/TranslogHeader.java

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,6 @@ static TranslogHeader read(final String translogUUID, final Path path, final Fil
137137
final BytesRef uuid = new BytesRef(uuidLen);
138138
uuid.length = uuidLen;
139139
in.read(uuid.bytes, uuid.offset, uuid.length);
140-
final BytesRef expectedUUID = new BytesRef(translogUUID);
141-
if (uuid.bytesEquals(expectedUUID) == false) {
142-
throw new TranslogCorruptedException(
143-
path.toString(),
144-
"expected shard UUID " + expectedUUID + " but got: " + uuid +
145-
" this translog file belongs to a different translog");
146-
}
147140
// Read the primary term
148141
assert version == VERSION_PRIMARY_TERM;
149142
final long primaryTerm = in.readLong();
@@ -154,6 +147,16 @@ static TranslogHeader read(final String translogUUID, final Path path, final Fil
154147
final int headerSizeInBytes = headerSizeInBytes(version, uuid.length);
155148
assert channel.position() == headerSizeInBytes :
156149
"Header is not fully read; header size [" + headerSizeInBytes + "], position [" + channel.position() + "]";
150+
151+
// verify UUID only after checksum, to ensure that UUID is not corrupted
152+
final BytesRef expectedUUID = new BytesRef(translogUUID);
153+
if (uuid.bytesEquals(expectedUUID) == false) {
154+
throw new TranslogCorruptedException(
155+
path.toString(),
156+
"expected shard UUID " + expectedUUID + " but got: " + uuid +
157+
" this translog file belongs to a different translog");
158+
}
159+
157160
return new TranslogHeader(translogUUID, primaryTerm, headerSizeInBytes);
158161
} catch (EOFException e) {
159162
throw new TranslogCorruptedException(path.toString(), "translog header truncated", e);

server/src/test/java/org/elasticsearch/index/translog/TranslogHeaderTests.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import static org.hamcrest.Matchers.anyOf;
3232
import static org.hamcrest.Matchers.containsString;
3333
import static org.hamcrest.Matchers.equalTo;
34+
import static org.hamcrest.Matchers.not;
3435

3536
public class TranslogHeaderTests extends ESTestCase {
3637

@@ -59,9 +60,9 @@ public void testCurrentHeaderVersion() throws Exception {
5960
for (int i = 0; i < corruptions && Files.size(translogFile) > 0; i++) {
6061
TestTranslog.corruptFile(logger, random(), translogFile, false);
6162
}
62-
expectThrows(TranslogCorruptedException.class, () -> {
63+
final TranslogCorruptedException corruption = expectThrows(TranslogCorruptedException.class, () -> {
6364
try (FileChannel channel = FileChannel.open(translogFile, StandardOpenOption.READ)) {
64-
TranslogHeader.read(outHeader.getTranslogUUID(), translogFile, channel);
65+
TranslogHeader.read(randomBoolean() ? outHeader.getTranslogUUID() : UUIDs.randomBase64UUID(), translogFile, channel);
6566
} catch (IllegalStateException e) {
6667
// corruption corrupted the version byte making this look like a v2, v1 or v0 translog
6768
assertThat("version " + TranslogHeader.VERSION_CHECKPOINTS + "-or-earlier translog",
@@ -70,6 +71,7 @@ public void testCurrentHeaderVersion() throws Exception {
7071
throw new TranslogCorruptedException(translogFile.toString(), "adjusted translog version", e);
7172
}
7273
});
74+
assertThat(corruption.getMessage(), not(containsString("this translog file belongs to a different translog")));
7375
}
7476

7577
public void testLegacyTranslogVersions() {

0 commit comments

Comments
 (0)