v3 attached. It's a bit of two steps forward, one step back:
- renames RowStats -> ColumnStats; separates row size computation from column/tombstone counts, moves ColumnStats computation out of serializer and into AbstractCompactedRow
- I switched from checking instanceof ExipiringColumn, to instanceof DeletedColumn, since an ExpiringColumn just means it will expire eventually (at which point it turns into a DeletedColumn), whereas a DeletedColumn is a tombstone that will be eligible for dropping after gc_grace_seconds. A common use case for TTL is to expire all data in a row after N days; if we're just going by "this column has a TTL" we'll compact these sstables daily even if none of the data has actually expired yet. Switching to checking for DC instead mitigates this a little.
However, the more I think about it, the more I think what we really want to track is a histogram of when tombstones are eligible to be dropped, relative to the sstable creation time. So, if I had a column that expired after 30 days, and a gc_grace_seconds of 10 days, I'd add an entry for 40 days to the histogram. If I had a new manual delete, I'd add an entry for 10 days.
This would allow us to have a good estimate of how much of the sstable could actually be cleaned out by compaction, and we could drop the single_compaction_interval code entirely.
What do you think?
Minor note: the new test seems fairly involved – what would we lose by just testing compaction of a single sstable w/ tombstones?