[
https://issues.apache.org/jira/browse/CASSANDRA-20829?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18013635#comment-18013635
]
Stefan Miklosovic edited comment on CASSANDRA-20829 at 8/13/25 9:30 AM:
------------------------------------------------------------------------
> In other words it would kill all benefits of whole-sstable-expiration.
Not always, only when table has an index, right? Also we might further narrow
this down e.g. it would do it like that only in case it does not have any SAI
indexes etc, just old 2i implementations.
> SAI (which will be fine just dropping the files without being notified of
> expirations) for one would be badly and unnecessarily hurt by this
Yes, we can leave out SAI from this, no problems.
> For the other cases, a simpler walk over the file, listing all data as
> deleted, will be a much better performing solution, and we should only apply
> this if the secondary index requests it
I am not completely sure what you mean by this. The primary reason this patch
exists is that when you have a custom 2i implementation (e.g. Stratio Lucene
plugin), then when SSTables are fully expired, because it does not integrate
with IndexGCTransaction, these custom implementations are never notified about
rows expired. It is like that index implementation is completely blind to the
fact that SSTables expired in their entirety. This in turn means that we can
not use TWCS on tables because in this index the data will never be purged
which will make that index which has its own data structures which handle
indexed data to grow beyond any limit.
Basically it is like we have API which indexes are meant to integrate with but
on server side we never cover that case for fully expired sstables. This is
clearly Cassandra's fault.
We are particularly interested in patching this in 4.1 and I do not think that
such a refactorisation you are suggesting would be justifiable given these are
just older maintenance branches not a lot of effort should go into anymore.
was (Author: smiklosovic):
> In other words it would kill all benefits of whole-sstable-expiration.
Not always, only when table has an index, right? Also we might further narrow
this down e.g. it would do it like that only in case it does not have any SAI
indexes etc, just old 2i implementations.
> SAI (which will be fine just dropping the files without being notified of
> expirations) for one would be badly and unnecessarily hurt by this
Yes, we can leave out SAI from this, no problems.
> For the other cases, a simpler walk over the file, listing all data as
> deleted, will be a much better performing solution, and we should only apply
> this if the secondary index requests it
I am not completely sure what you mean by this. The primary reason this patch
exists is that when you have a custom 2i implementation (e.g. Stratio Lucene
plugin), then when SSTables are fully expired, because it does not integrate
with IndexGCTransaction, these custom implementations are never notified about
rows expired. It is like that index implementation is completely blind to the
fact that SSTables expired in their entirety. This in turn means that we can
not use TWCS on tables because in this index the data will never be purged
which will make that index which has its own data structures which handle
indexed data to grow beyond any limit.
We are particularly interested in patching this in 4.1 and I do not think that
such a refactorisation you are suggesting would be justifiable given these are
just older maintenance branches not a lot of effort should go into anymore.
> Secondary index implementations do not integrate with IndexGCTransaction when
> compaction contains fully expired SSTables
> ------------------------------------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-20829
> URL: https://issues.apache.org/jira/browse/CASSANDRA-20829
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Feature/2i Index, Local/Compaction, Local/Compaction/TWCS
> Reporter: Stefan Miklosovic
> Assignee: Stefan Miklosovic
> Priority: Normal
> Fix For: 4.0.x, 4.1.x, 5.0.x, 5.x
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> There is a test (1) which ensures that when data are TTLed and compacted,
> IndexGCTransaction is aware of that and it will invoke Indexer.removeRow()
> method eventually.
> However, this is not working properly when we have fully expired SSTables,
> e.g. as the result of a table being on TWCS and having TTL on that.
> The reason is that in CompactionTask, we are filtering out fully expired ones
> (2). These then do not go to the compaction process and then they are not
> reacted on in listener() (3) which contains this logic (4). Eventually,
> onRowMerge in IndexGCTransaction will make the diff and in its commit
> indexer.removeRow(row); will notify 2i about its removal.
>
> This integration is missing and it is quite a big problem because if there
> are custom secondary index implementations the fact that SSTables were fully
> expired is not propagated to them which means that data are never removed
> from whatever backend they use.
> The solution is to go to the compaction with fully expired SSTables as well
> _but only if we detected that respective column family has some indexes_
>
> (1)
> [https://github.com/apache/cassandra/blob/cassandra-4.1/test/unit/org/apache/cassandra/index/CustomIndexTest.java#L583-L607]
> (2)
> [https://github.com/apache/cassandra/blob/cassandra-4.1/src/java/org/apache/cassandra/db/compaction/CompactionTask.java#L174]
> (3)
> [https://github.com/apache/cassandra/blob/cassandra-4.1/src/java/org/apache/cassandra/db/compaction/CompactionIterator.java#L130]
> (4)
> [https://github.com/apache/cassandra/blob/cassandra-4.1/src/java/org/apache/cassandra/db/compaction/CompactionIterator.java#L235-L252]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
