CNDB-13745: Fix checksum for IncrementalChecksumSequentialWriter edge case

[michaeljmarshall]

What is the issue

Fixes: https://github.com/riptano/cndb/issues/13745

What does this PR fix and why was it fixed

In the IncrementalChecksumSequentialWriter, we call writeMostSignificantBytes, and that has a minor "bug" in the implementation in the BufferedDataOutputStreamPlus class. In the following logic:

    @Override
    public void writeMostSignificantBytes(long register, int bytes) throws IOException
    {
        assert buffer != null : "Attempt to use a closed data output";
        if (buffer.remaining() < Long.BYTES)
        {
            super.writeMostSignificantBytes(register, bytes);
        }
        else
        {
            int pos = buffer.position();
            buffer.putLong(pos, register);
            buffer.position(pos + bytes);
        }
    }

when we call super.writeMostSignificantBytes, we accidentally double count the next bytes in the checksum. This is in part due to the hybrid solution. Instead, I propose that we remove the naive solution (that is likely slower) and instead just do the bit math that calls the appropriate writeBytes, writeShorte, etc. methods.

[github-actions]

Checklist before you submit for review

• Make sure there is a PR in the CNDB project updating the Converged Cassandra version
• Use NoSpamLogger for log lines that may appear frequently in the logs
• Verify test results on Butler
• Test coverage for new/modified code is > 80%
• Proper code formatting
• Proper title for each commit staring with the project-issue number, like CNDB-1234
• Each commit has a meaningful description
• Each commit is not very long and contains related changes
• Renames, moves and reformatting are in distinct commits
• All new files should contain the DataStax copyright header instead of the Apache License one

[blambov]

Because it's involved in writing vints, writeMostSignificantBytes may be the most heavily used method in the serialization code. I'd be quite wary of dropping the fast path in BufferedDataOutputStreamPlus without proving that it does not hurt us. Can you please check the results of VintCodingBench, especially testWriteRandomLongDOP?

If you are interested, take a look at CASSANDRA-15215 where this was introduced.

[michaeljmarshall]

Thank you for the reference @blambov. It looks like we don't even use the IncrementalChecksumSequentialWriter in https://github.com/apache/cassandra/blob/cassandra-5.0/src/java/org/apache/cassandra/index/sai/disk/io/IndexFileUtils.java, so perhaps there is a different direction we should go @djatnieks?

[djatnieks]

perhaps there is a different direction we should go @djatnieks?

Interesting, ok I will take a look