Add optional CRC per log entry. #454
Conversation
| uint32_t get_crc32() const { | ||
| return crc32_; | ||
| } | ||
|
|
||
| void set_crc32(uint32_t crc) { | ||
| crc32_ = crc; | ||
| } |
There was a problem hiding this comment.
For backward compatibility (for the previous logs that do not have CRC, and they can be mixed with new logs with CRC), we should have one more field indicating if CRC exists or not, let's say has_crc32_.
Otherwise, we cannot distinguish non-existing CRC and CRC=0. Also we need an API returning that flag:
bool has_crc32() const () { ... }that will be used in asio layer (replacing crc32 != 0).
src/asio_service.cxx
Outdated
| if (impl_->get_options().crc_on_payload_) { | ||
| ss.put_u32(le->get_crc32()); | ||
| } |
There was a problem hiding this comment.
Similarly, we need to ship the existence flag as well.
if (impl_->get_options().crc_on_payload_) {
ss.put_u8(le->has_crc32() ? 1 : 0);
ss.put_u32(le->get_crc32());
}and
LOG_ENTRY_SIZE += 5;I don't have better idea rather than shipping 5 bytes.
include/libnuraft/log_entry.hxx
Outdated
| uint64_t log_timestamp = 0) | ||
| uint64_t log_timestamp = 0, | ||
| uint32_t crc32 = 0, | ||
| bool compute_crc = true) |
There was a problem hiding this comment.
Can this be false by default?
There was a problem hiding this comment.
This is to cope with conf logs. There are more than 1 place where conf logs are constructed. Either we need to copy the CRC computation code (or this true flag) everywhere (and I need to make sure I found every place), or we bite the performance overhead and lean towards safety.
include/libnuraft/log_entry.hxx
Outdated
| @@ -23,8 +23,10 @@ limitations under the License. | |||
|
|
|||
| #include "basic_types.hxx" | |||
| #include "buffer.hxx" | |||
| #include "crc32.hxx" | |||
There was a problem hiding this comment.
Another thing I just found: crc32.hxx is not a public header, but it is included in log_entry.hxx which is the library's public header. Since we don't want to expose our custom CRC implementation, we need a separate log_entry.cxx file in src.
| * should not be any member running with the old version before supporting | ||
| * this flag. | ||
| */ | ||
| bool crc_on_payload_; |
There was a problem hiding this comment.
Initialization in the constructor is missing.
Add an option to replicate and verify per-log-entry CRC checksums.
The log entry's checksum is computed when the entry is first created on the leader, and then stored in the log store and replicated over the network. On receive, if the checksum is sent, it is verified.