[
https://issues.apache.org/jira/browse/CASSANDRA-13698?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16438984#comment-16438984
]
Lerh Chuan Low commented on CASSANDRA-13698:
--------------------------------------------
{quote}The patch looks good and the failures look unrelated but I just noticed
that there are a bunch of other commented-out tests on CompactionsTest like
testEchoedRow, testRangeTombstones,
testUncheckedTombstoneSizeTieredCompaction,etc that are like this since
CASSANDRA-8099. Even though this was not in the original ticket scope, I think
we should also triage those tests and either remove or restore them. WDYT?
{quote}
Seconded :) Being relatively new to the project it unnerves me about the
current state of tests.
I've reattached the patches and reverted {{stcs}} for 3.0 and 3.X.
Here are the branches again if they float your boat more:
[https://github.com/apache/cassandra/compare/trunk...juiceblender:cassandra-13698]
[https://github.com/apache/cassandra/compare/cassandra-3.11...juiceblender:cassandra-13698-3.11]
[https://github.com/apache/cassandra/compare/cassandra-3.0...juiceblender:cassandra-13698-3.0]
[https://circleci.com/gh/juiceblender/cassandra/56] (Trunk)
[https://circleci.com/gh/juiceblender/cassandra/60] (3.0)
[https://circleci.com/gh/juiceblender/cassandra/61] (3.11)
I get the feeling 3.0 and 3.11 are going to fail again (albeit with failures
already happening on 3.11.2 and 3.0.16).
Just to draw your attention to a few things (which I'm not overly sure of):
1. I got rid of {{testEchoedRow}} and {{testCompactionLog}} - it has been a
long time since we had {{EchoedRow}} in the codebase and we no longer have
compaction history stored in system tables.
2. I changed some of the {{superCFMDs}} to {{standardCFMDs}} because
- It doesn't look like we use super columns/they have been deprecated for some
time? The method call seems to create a nameless column "" and also expects a
{{Map}} type, which for the purposes of the tests isn't required.
- Basing against trunk, {{superCFMD}} calls {{standardCFMD}} so...
3. {{testDontPurgeAccidentally}} is a little bit weird. I originally attempted
to assert {{partition.isEmpty()}}, turns out that is false. According to my
sstabledump it looks like that 'makes sense', because there is still a cell,
it's just no longer live.
{code:java}
{
"partition" : {
"key" : [ "test1" ],
"position" : 0
},
"rows" : [
{
"type" : "row",
"position" : 28,
"clustering" : [ "c" ],
"liveness_info" : { "tstamp" : "1970-01-01T00:00:00.000002Z" },
"cells" : [
{ "name" : "val", "deletion_info" : { "local_delete_time" :
"2018-04-15T23:56:48Z" }
}
]
}
]
}
{code}
So I attempted to check if the tombstone exists instead:
{code:java}
ImmutableBTreePartition partition =
Util.getOnlyPartitionUnfiltered(Util.cmd(cfs, key).build());
assertTrue(!partition.isEmpty());
RowUpdateBuilder deleteRowBuilder = new RowUpdateBuilder(table, 2, key);
deleteRowBuilder.clustering("c").delete("val");
deleteRowBuilder.build().applyUnsafe();
// Remove key
partition = Util.getOnlyPartitionUnfiltered(Util.cmd(cfs, key).build());
assertTrue(partition.iterator().next().cells().iterator().next().isTombstone());
// Sleep one second so that the removal is indeed purgeable even with
gcgrace == 0
Thread.sleep(1000);
cfs.forceBlockingFlush();
Collection<SSTableReader> sstablesAfter = cfs.getLiveSSTables();
Collection<SSTableReader> toCompact = new ArrayList<SSTableReader>();
for (SSTableReader sstable : sstablesAfter)
if (!sstablesBefore.contains(sstable))
toCompact.add(sstable);
Util.compact(cfs, toCompact);
partition = Util.getOnlyPartitionUnfiltered(Util.cmd(cfs, key).build());
assertTrue(partition.iterator().next().cells().iterator().next().isTombstone());
<----- This never works for gc_grace = 0
{code}
But it never works for the final assert for the CF with gc_grace = 0. I also
tried using
{{partition.getRow(Clustering.make(ByteBufferUtil.bytes("c"))).isEmpty()}}.
Doesn't work either - it also always fails at that final assert for gc_grace =
0. So I eventually settled for just checking {{row.size()}}. It seems that,
when debugging, the partition really does not have any 'cells' in the sense
that it has only an empty {{BTreeRow}} in it, and nothing else. But having that
is not enough to be 'isEmpty', it has to have nothing in there at all
(BTree::EMPTY_LEAF, which is just {{new Object[1];}}.
Not sure if you are able to explain why that is so, but otherwise I think the
test (as I have it) is fair.
> Reinstate or get rid of unit tests with multiple compaction strategies
> ----------------------------------------------------------------------
>
> Key: CASSANDRA-13698
> URL: https://issues.apache.org/jira/browse/CASSANDRA-13698
> Project: Cassandra
> Issue Type: Test
> Components: Testing
> Reporter: Paulo Motta
> Assignee: Lerh Chuan Low
> Priority: Minor
> Labels: lhf
> Attachments: 13698-3.0.txt, 13698-3.11.txt, 13698-trunk.txt
>
>
> At some point there were (anti-)compaction tests with multiple compaction
> strategy classes, but now it's only tested with {{STCS}}:
> *
> [AnticompactionTest|https://github.com/apache/cassandra/blob/8b3a60b9a7dbefeecc06bace617279612ec7092d/test/unit/org/apache/cassandra/db/compaction/AntiCompactionTest.java#L247]
> *
> [CompactionsTest|https://github.com/apache/cassandra/blob/8b3a60b9a7dbefeecc06bace617279612ec7092d/test/unit/org/apache/cassandra/db/compaction/CompactionsTest.java#L85]
> We should either reinstate these tests or decide they are not important and
> remove the unused parameter.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
