Jonathan, thanks for the feedback.
I need a bit of clarification for a newbie hacking on the code...
Looks like this only updates the CQL path? We'd want to make the Thrift path cf-ttl-aware as well. I think this just means updating RowMutation + CF addColumn methods.
I actually thought the opposite. Part of the code I changed was in CFMetaData's toThrift and fromThrift methods. Perhaps I'm reading too much into the method names?
But I took a look at ColumnFamily's addColumn method, but it already performs the conditional based on the TTL value.
Nit: we could simplify getTTL a bit by adding assert ttl > 0.
Sorry, I'm not sure to which part of the code you're referring Can you elaborate?
I got it backwards: we want max(cf ttl, column ttl) to be able to reason about the live-ness of CF data w/o looking at individual rows
I cleaned up the CFMetaData.getTimeToLive method, which is now simply:
public int getTimeToLive(int timeToLive)
return Math.max(defaultTimeToLive, timeToLive);
We can break the compaction optimizations into another ticket. It really needs a separate compaction Strategy; the idea is if we have an sstable A older than CF ttl, then all the data in the file is dead and we can just delete the file without looking at it row-by-row. However, there's a lot of tension there with the goal of normal compaction, which wants to merge different versions of the same row, so we're going to churn a lot with a low chance of ever having an sstable last the full TTL without being merged, effectively restarting our timer. So, I think we're best served by a ArchivingCompactionStrategy that doesn't merge sstables at all, just drops obsolete ones, and let people use that for append-only insert workloads. Which is a common enough case that it's worth the trouble... probably.
Either way is fine. Would love to contribute.