[
https://issues.apache.org/jira/browse/CASSANDRA-9530?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15295913#comment-15295913
]
Stefania commented on CASSANDRA-9530:
-------------------------------------
Agreed on testing corruption for early opened tables, let's leave it for now
since it is quite complex to write and it doesn't add much to what the new
corruption tests already do.
The patch looks good to me now.
I've fixed a couple of nits, edited the text in CHANGES.txt a bit, and I've
introduced a field in {{AbstractType}}, to avoid calling a method every time we
read a value. Perhaps this is not necessary if the jit compiler optimizes this
static method call away, however there is an ongoing effort to limit the
dependencies on {{DatabaseDescriptor}}, at least in some packages, so it is not
a bad thing to move the dependency at least to the constructor. Unfortunately,
due to all the type singleton instances, this field cannot be final as it needs
to be changed for testing. Feel free to see if you can figure out a better way
to end up with types that have a different max value size by passing it to the
constructor. Basically, once the types are in the CFS metadata we are good for
our tests, however even if we create the CFM with those types, when the
keyspace is created we end up with new types because we use the CQL string of
the type. I did not want to change that part of the code for this simple patch.
You can find my changes
[here|https://github.com/stef1927/cassandra/commit/2ab8950ae17ea74c7690f83dfa2eec0aa66290cc].
If you are +1 on them, let's rebase and re-run the tests.
> SSTable corruption can trigger OOM
> ----------------------------------
>
> Key: CASSANDRA-9530
> URL: https://issues.apache.org/jira/browse/CASSANDRA-9530
> Project: Cassandra
> Issue Type: Bug
> Reporter: Sylvain Lebresne
> Assignee: Alex Petrov
>
> If a sstable is corrupted so that the length of a given is bogus, we'll still
> happily try to allocate a buffer of that bogus size to read the value, which
> can easily lead to an OOM.
> We should probably protect against this. In practice, a given value can be so
> big since it's limited by the protocol frame size in the first place. Maybe
> we could add a max_value_size_in_mb setting and we'd considered a sstable
> corrupted if it was containing a value bigger than that.
> I'll note that this ticket would be a good occasion to improve
> {{BlacklistingCompactionsTest}}. Typically, it currently generate empty
> values which makes it pretty much impossible to get the problem described
> here. And as described in CASSANDRA-9478, it also doesn't test properly for
> thing like early opening of compaction results. We could try to randomize as
> much of the parameters of this test as possible to make it more likely to
> catch any type of corruption that could happen.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
