You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/07/16 21:56:03 UTC
[GitHub] [cassandra] yifan-c opened a new pull request #682: CASSANDRA-15945 verify sstable components on startup
yifan-c opened a new pull request #682:
URL: https://github.com/apache/cassandra/pull/682
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c commented on pull request #682: CASSANDRA-15945 verify sstable components on startup
Posted by GitBox <gi...@apache.org>.
yifan-c commented on pull request #682:
URL: https://github.com/apache/cassandra/pull/682#issuecomment-676597748
close the PR since it is merged.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c commented on a change in pull request #682: CASSANDRA-15945 verify sstable components on startup
Posted by GitBox <gi...@apache.org>.
yifan-c commented on a change in pull request #682:
URL: https://github.com/apache/cassandra/pull/682#discussion_r456724803
##########
File path: test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
##########
@@ -780,4 +787,65 @@ public void testGetApproximateKeyCount() throws InterruptedException
assertEquals(50, SSTableReader.getApproximateKeyCount(sstables));
}
}
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Test
+ public void testVerifyCompressionInfoExistenceThrows()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+
+ // delete the compression info, so it is corrupted.
+ File compressionInfoFile = new File(desc.filenameFor(Component.COMPRESSION_INFO));
+ compressionInfoFile.delete();
+ assertFalse("CompressionInfo file should not exist", compressionInfoFile.exists());
+
+ // discovert the components on disk after deletion
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+
+ expectedException.expect(CorruptSSTableException.class);
+ expectedException.expectMessage("CompressionInfo.db");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistencePasses()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+
+ // mark the toc file not readable in order to trigger the FSReadError
+ File tocFile = new File(desc.filenameFor(Component.TOC));
+ tocFile.setReadable(false);
+
+ expectedException.expect(FSReadError.class);
+ expectedException.expectMessage("TOC.txt");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistenceWhenTOCUnableToOpen()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ private Descriptor setUpForTestVerfiyCompressionInfoExistence()
Review comment:
It is a mistake. I somehow swapped the test name between `testVerifyCompressionInfoExistenceWhenTOCUnableToOpen` and `testVerifyCompressionInfoExistencePasses`.
If TOC exist but cannot be opened, it throws `FSReadError`.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #682: CASSANDRA-15945 verify sstable components on startup
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #682:
URL: https://github.com/apache/cassandra/pull/682#discussion_r457620205
##########
File path: test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
##########
@@ -780,4 +787,65 @@ public void testGetApproximateKeyCount() throws InterruptedException
assertEquals(50, SSTableReader.getApproximateKeyCount(sstables));
}
}
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Test
+ public void testVerifyCompressionInfoExistenceThrows()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+
+ // delete the compression info, so it is corrupted.
+ File compressionInfoFile = new File(desc.filenameFor(Component.COMPRESSION_INFO));
+ compressionInfoFile.delete();
+ assertFalse("CompressionInfo file should not exist", compressionInfoFile.exists());
+
+ // discovert the components on disk after deletion
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+
+ expectedException.expect(CorruptSSTableException.class);
+ expectedException.expectMessage("CompressionInfo.db");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistencePasses()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+
+ // mark the toc file not readable in order to trigger the FSReadError
+ File tocFile = new File(desc.filenameFor(Component.TOC));
+ tocFile.setReadable(false);
+
+ expectedException.expect(FSReadError.class);
+ expectedException.expectMessage("TOC.txt");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistenceWhenTOCUnableToOpen()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ private Descriptor setUpForTestVerfiyCompressionInfoExistence()
+ {
+ Keyspace keyspace = Keyspace.open(KEYSPACE1);
+ ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(CF_COMPRESSED);
+ SSTableReader sstable = getNewSSTable(cfs);
+ cfs.clearUnsafe();
+ sstable.selfRef().release();
+ Descriptor desc = sstable.descriptor;
+
+ File compressionInfoFile = new File(desc.filenameFor(Component.COMPRESSION_INFO));
+ File tocFile = new File(desc.filenameFor(Component.TOC));
+ assertTrue("CompressionInfo file should exist", compressionInfoFile.exists());
Review comment:
Thanks, by removing the release these should stick around; so should resolve the issue above.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #682: CASSANDRA-15945 verify sstable components on startup
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #682:
URL: https://github.com/apache/cassandra/pull/682#discussion_r456723461
##########
File path: test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
##########
@@ -780,4 +787,65 @@ public void testGetApproximateKeyCount() throws InterruptedException
assertEquals(50, SSTableReader.getApproximateKeyCount(sstables));
}
}
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Test
+ public void testVerifyCompressionInfoExistenceThrows()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+
+ // delete the compression info, so it is corrupted.
+ File compressionInfoFile = new File(desc.filenameFor(Component.COMPRESSION_INFO));
+ compressionInfoFile.delete();
+ assertFalse("CompressionInfo file should not exist", compressionInfoFile.exists());
+
+ // discovert the components on disk after deletion
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+
+ expectedException.expect(CorruptSSTableException.class);
+ expectedException.expectMessage("CompressionInfo.db");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistencePasses()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+
+ // mark the toc file not readable in order to trigger the FSReadError
+ File tocFile = new File(desc.filenameFor(Component.TOC));
+ tocFile.setReadable(false);
+
+ expectedException.expect(FSReadError.class);
+ expectedException.expectMessage("TOC.txt");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistenceWhenTOCUnableToOpen()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ private Descriptor setUpForTestVerfiyCompressionInfoExistence()
+ {
+ Keyspace keyspace = Keyspace.open(KEYSPACE1);
+ ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(CF_COMPRESSED);
+ SSTableReader sstable = getNewSSTable(cfs);
+ cfs.clearUnsafe();
+ sstable.selfRef().release();
+ Descriptor desc = sstable.descriptor;
+
+ File compressionInfoFile = new File(desc.filenameFor(Component.COMPRESSION_INFO));
+ File tocFile = new File(desc.filenameFor(Component.TOC));
+ assertTrue("CompressionInfo file should exist", compressionInfoFile.exists());
Review comment:
To make this safe, you would need to create a `readOrdering` barrier
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #682: CASSANDRA-15945 verify sstable components on startup
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #682:
URL: https://github.com/apache/cassandra/pull/682#discussion_r456723285
##########
File path: test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
##########
@@ -780,4 +787,65 @@ public void testGetApproximateKeyCount() throws InterruptedException
assertEquals(50, SSTableReader.getApproximateKeyCount(sstables));
}
}
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Test
+ public void testVerifyCompressionInfoExistenceThrows()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+
+ // delete the compression info, so it is corrupted.
+ File compressionInfoFile = new File(desc.filenameFor(Component.COMPRESSION_INFO));
+ compressionInfoFile.delete();
+ assertFalse("CompressionInfo file should not exist", compressionInfoFile.exists());
+
+ // discovert the components on disk after deletion
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+
+ expectedException.expect(CorruptSSTableException.class);
+ expectedException.expectMessage("CompressionInfo.db");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistencePasses()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+
+ // mark the toc file not readable in order to trigger the FSReadError
+ File tocFile = new File(desc.filenameFor(Component.TOC));
+ tocFile.setReadable(false);
+
+ expectedException.expect(FSReadError.class);
+ expectedException.expectMessage("TOC.txt");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistenceWhenTOCUnableToOpen()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ private Descriptor setUpForTestVerfiyCompressionInfoExistence()
+ {
+ Keyspace keyspace = Keyspace.open(KEYSPACE1);
+ ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(CF_COMPRESSED);
+ SSTableReader sstable = getNewSSTable(cfs);
+ cfs.clearUnsafe();
+ sstable.selfRef().release();
+ Descriptor desc = sstable.descriptor;
+
+ File compressionInfoFile = new File(desc.filenameFor(Component.COMPRESSION_INFO));
+ File tocFile = new File(desc.filenameFor(Component.TOC));
+ assertTrue("CompressionInfo file should exist", compressionInfoFile.exists());
Review comment:
this will be flaky, deleting sstables is async
```
# org.apache.cassandra.io.sstable.format.SSTableReader.InstanceTidier#tidy
ScheduledExecutors.nonPeriodicTasks.execute(new Runnable()
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] dcapwell commented on a change in pull request #682: CASSANDRA-15945 verify sstable components on startup
Posted by GitBox <gi...@apache.org>.
dcapwell commented on a change in pull request #682:
URL: https://github.com/apache/cassandra/pull/682#discussion_r456724327
##########
File path: test/unit/org/apache/cassandra/io/sstable/SSTableReaderTest.java
##########
@@ -780,4 +787,65 @@ public void testGetApproximateKeyCount() throws InterruptedException
assertEquals(50, SSTableReader.getApproximateKeyCount(sstables));
}
}
+
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
+ @Test
+ public void testVerifyCompressionInfoExistenceThrows()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+
+ // delete the compression info, so it is corrupted.
+ File compressionInfoFile = new File(desc.filenameFor(Component.COMPRESSION_INFO));
+ compressionInfoFile.delete();
+ assertFalse("CompressionInfo file should not exist", compressionInfoFile.exists());
+
+ // discovert the components on disk after deletion
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+
+ expectedException.expect(CorruptSSTableException.class);
+ expectedException.expectMessage("CompressionInfo.db");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistencePasses()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+
+ // mark the toc file not readable in order to trigger the FSReadError
+ File tocFile = new File(desc.filenameFor(Component.TOC));
+ tocFile.setReadable(false);
+
+ expectedException.expect(FSReadError.class);
+ expectedException.expectMessage("TOC.txt");
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ @Test
+ public void testVerifyCompressionInfoExistenceWhenTOCUnableToOpen()
+ {
+ Descriptor desc = setUpForTestVerfiyCompressionInfoExistence();
+ Set<Component> components = SSTable.discoverComponentsFor(desc);
+ SSTableReader.verifyCompressionInfoExistenceIfApplicable(desc, components);
+ }
+
+ private Descriptor setUpForTestVerfiyCompressionInfoExistence()
Review comment:
I don't understand why you delete the SSTable. verify will run IFF TOC.txt exists, and that is deleted async. Given the name (Verify CompressionInfo Existence When TOC Unable To Open) it sounds like you want to delete TOC and verify that verify no-ops? If so you should prob have the function return a boolean to say if it did anything, and make sure that it returns that it didn't.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] yifan-c closed pull request #682: CASSANDRA-15945 verify sstable components on startup
Posted by GitBox <gi...@apache.org>.
yifan-c closed pull request #682:
URL: https://github.com/apache/cassandra/pull/682
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org