maedhroz commented on code in PR #2460:
URL: https://github.com/apache/cassandra/pull/2460#discussion_r1253478976
##########
src/java/org/apache/cassandra/index/sai/disk/io/IndexFileUtils.java:
##########
@@ -77,7 +77,7 @@ public IndexInput openBlockingInput(File file)
public interface ChecksumWriter
{
- long getChecksum();
+ long getChecksum() throws IOException;
}
static class IncrementalChecksumSequentialWriter extends SequentialWriter
implements ChecksumWriter
Review Comment:
I'm actually not sure why we have the `ChecksumWriter` interface at all
here. (CC @mike-tr-adamson) Also not sure what's incremental about this now,
given we just write it on flush.
There is the class `ChecksummedSequentialWriter` in `o.a.c.io.util`, and
that even shares essentially the same `flushData()` method (but it writes to a
concrete class called `ChecksumWriter` which has an incremental component as
well as its own `DataOutput` to another file).
Perhaps we could address the `flushData()` duplication by having a
`SequentialWriter` subclass w/ an abstract method that updates the checksum in
a subclass-appropriate way. Then you'd have two subclasses, one strictly in
memory for this case, and the other essentially doing what
`o.a.c.io.util.ChecksummedSequentialWriter` does now. This might not be the
only way to address this. (Naming would be interesting, as usual.
`InMemoryChecksumSequentialWriter` is awful, but sort of describes what the one
in this class does...)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
