diff --git a/CHANGES.txt b/CHANGES.txt index 74eef72666..c732513b5a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -3,6 +3,7 @@ * Rate limit validation compactions using compaction_throughput_mb_per_sec (CASSANDRA-16161) * SASI's `max_compaction_flush_memory_in_mb` settings over 100GB revert to default of 1GB (CASSANDRA-16071) Merged from 3.0: + * Improve empty hint file handling during startup (CASSANDRA-16162) * Allow empty string in collections with COPY FROM in cqlsh (CASSANDRA-16372) * Fix skipping on pre-3.0 created compact storage sstables due to missing primary key liveness (CASSANDRA-16226) * Extend the exclusion of replica filtering protection to other indices instead of just SASI (CASSANDRA-16311) diff --git a/src/java/org/apache/cassandra/hints/HintsDescriptor.java b/src/java/org/apache/cassandra/hints/HintsDescriptor.java index 00224e2d1e..429d4d4d3f 100644 --- a/src/java/org/apache/cassandra/hints/HintsDescriptor.java +++ b/src/java/org/apache/cassandra/hints/HintsDescriptor.java @@ -21,6 +21,7 @@ import java.io.DataInput; import java.io.IOException; import java.io.RandomAccessFile; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.util.HashMap; import java.util.Map; @@ -30,6 +31,7 @@ import java.util.regex.Pattern; import java.util.zip.CRC32; import javax.crypto.Cipher; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.MoreObjects; import com.google.common.base.Objects; import com.google.common.collect.ImmutableMap; @@ -237,11 +239,35 @@ final class HintsDescriptor } catch (IOException e) { - logger.error("Failed to deserialize hints descriptor {}", path.toString(), e); + handleDescriptorIOE(e, path); return Optional.empty(); } } + @VisibleForTesting + static void handleDescriptorIOE(IOException e, Path path) + { + try + { + if (Files.size(path) > 0) + { + String newFileName = path.getFileName().toString().replace(".hints", ".corrupt.hints"); + Path target = path.getParent().resolve(newFileName); + logger.error("Failed to deserialize hints descriptor {} - saving file as {}", path.toString(), target, e); + Files.move(path, target); + } + else + { + logger.warn("Found empty hints file {} on startup, removing", path.toString()); + Files.delete(path); + } + } + catch (IOException ex) + { + logger.error("Error handling corrupt hints file {}", path.toString(), ex); + } + } + static HintsDescriptor readFromFile(Path path) { try (RandomAccessFile raf = new RandomAccessFile(path.toFile(), "r")) diff --git a/test/unit/org/apache/cassandra/hints/HintsDescriptorTest.java b/test/unit/org/apache/cassandra/hints/HintsDescriptorTest.java index 08487d1874..bab2356b6e 100644 --- a/test/unit/org/apache/cassandra/hints/HintsDescriptorTest.java +++ b/test/unit/org/apache/cassandra/hints/HintsDescriptorTest.java @@ -20,11 +20,14 @@ package org.apache.cassandra.hints; import java.io.DataInput; import java.io.File; import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; import java.util.UUID; import com.google.common.collect.ImmutableMap; import com.google.common.io.ByteStreams; -import com.google.common.io.Files; +import org.junit.Assert; import org.junit.Test; import org.apache.cassandra.io.compress.LZ4Compressor; @@ -33,6 +36,8 @@ import org.apache.cassandra.io.util.DataOutputBuffer; import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.assertNotSame; import static junit.framework.Assert.fail; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; public class HintsDescriptorTest { @@ -100,21 +105,41 @@ public class HintsDescriptorTest ImmutableMap parameters = ImmutableMap.of(); HintsDescriptor expected = new HintsDescriptor(hostId, version, timestamp, parameters); - File directory = Files.createTempDir(); + Path directory = Files.createTempDirectory("hints"); try { - try (HintsWriter ignored = HintsWriter.create(directory, expected)) + try (HintsWriter ignored = HintsWriter.create(directory.toFile(), expected)) { } - HintsDescriptor actual = HintsDescriptor.readFromFile(new File(directory, expected.fileName()).toPath()); + HintsDescriptor actual = HintsDescriptor.readFromFile(directory.resolve(expected.fileName())); assertEquals(expected, actual); } finally { - directory.deleteOnExit(); + directory.toFile().deleteOnExit(); } } + @Test + public void testHandleIOE() throws IOException + { + Path p = Files.createTempFile("testing", ".hints"); + // empty file; + assertTrue(p.toFile().exists()); + Assert.assertEquals(0, Files.size(p)); + HintsDescriptor.handleDescriptorIOE(new IOException("test"), p); + assertFalse(Files.exists(p)); + + // non-empty + p = Files.createTempFile("testing", ".hints"); + Files.write(p, Collections.singleton("hello")); + HintsDescriptor.handleDescriptorIOE(new IOException("test"), p); + File newFile = new File(p.getParent().toFile(), p.getFileName().toString().replace(".hints", ".corrupt.hints")); + assertFalse(Files.exists(p)); + assertTrue(newFile.toString(), newFile.exists()); + newFile.deleteOnExit(); + } + private static void testSerializeDeserializeLoop(HintsDescriptor descriptor) throws IOException { // serialize to a byte array