move "long rowSizeFromIndex = nextRowPositionFromIndex - currentRowPositionFromIndex;" into the IF statement where it is needed
in your log warnings, specifying the actual sstable will help with debugging
Scrub starts with a log entry to indicate which file is being scrubbed... this should be sufficient unless someone is running without INFO.
remove "this.key = key;" for both constructors
Good catch... that line was a noop in the second constructor: we don't know the key until init() is called, so we can't make it final.
*init() method should be more descriptive
Was supposed to say "Reads the key and row length from the head of the row.": fixed.
remove @param key comment from IFilter.java and SSTableSliceIterator.java
safer to updateCache(iter) AFTER appending to writer
I think the whole thing is racey, because the entire rebuilding process can fail, which might mean that the cache was updated when it shouldn't have been.
I didn't make the close() change: we talked about close a little bit offline, I think we reached the consensus that having two methods that you must always call won't be an improvement over close. I also didn't add back the scrub retry: I agree that we can do better there, but I think it will be easier to improve once we have the checksumming from 674: at the moment the reasons for doing certain things with the API would not be clear at all.
Thanks for the review!