Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Log entry checksum validation #123

Merged
merged 27 commits into from
Apr 24, 2019

Conversation

killme2008
Copy link
Contributor

@killme2008 killme2008 commented Apr 21, 2019

Motivation:

LogEntry doesn't have any data checksum validation right now, it's a data integrity risk here. We should check log data's integrity after reading it from network or disk.

Modification:

  1. Adds a new log entry codec implementation LogEntryV2CodecFactory that encoding/decoding log entry by protobuf, and it adds a header to the message:
 0   1    2    3  4  5
+-+-+-+-+-+-+-+-++-+-+-+
|Magic|Version|Reserved|
+-+-+-+-+-+-+-+-++-+-+-+

Test the performance between v1 and v2 codec in LogEntryCodecPerfTest, do about 200k times to encoding/decoding log entry in 512 bytes:

V1 codec cost:148 ms.
Total log size:120540000 bytes.
V2 codec cost:302 ms.
Total log size:118014624 bytes.

The encoded log size is reduced about 1%~2% but cost more time, i think it's acceptable.

  1. Adds new methods to LogEntry:
    • hasChecksum() return true when has a checksum.
    • checksum() caculate the checksum of the log entry.
    • isCorrupted() returns true when the log entry has checksum and mismatched.

The checksum is caculated by CRC64 and xor.

  1. The LogEntryV2CodecFactory uses AutoDetectDecoder to detect the codec version automatically and chooses the right decoder to decode the log entry. We keep log codec compatibility in such way.

  2. Adds checksum validation in NodeImpl#handleAppendEntriesRequest,LogManagerImpl#getEntry and LogManagerImpl#getTermFromLogStorage, i introduce a new raft option RaftOptions#enableLogEntryChecksum to control the behaviour, it's false(disabled) by default.

  3. Deprecate LogEntry#encode and LogEntry#decode, you should use LogEntryEncoder and LogEntryDecoder instead. Also , deprecate LogStorage#getTerm(index),it can be replaced by getEntry(index).getId().getTerm(), because it don't want to do checksum validation in log storage but do it all in LogManagerImpl.

Result:

Fixes #105

@killme2008 killme2008 added this to the 1.2.6 milestone Apr 21, 2019
@killme2008 killme2008 changed the base branch from master to feature/log-entry-enhances April 21, 2019 07:22
@killme2008
Copy link
Contributor Author

冲突暂时不需要处理,等 #110 测试通过。

@killme2008 killme2008 changed the title Feature/log checksum Log entry checksum validation Apr 21, 2019
@fengjiachun
Copy link
Contributor

这个 PR 有点大,还没看完,我先歇会

@fengjiachun
Copy link
Contributor

@killme2008 看完了,提了几个意见你先看下

@killme2008
Copy link
Contributor Author

@fengjiachun 基本都改完了,你可以再看下,但是不着急合并。 V2 codec 这块性能还有改善的空间,我还是坚持用 protobuf,但是可以再想办法降低下损耗。

@fengjiachun
Copy link
Contributor

@fengjiachun 基本都改完了,你可以再看下,但是不着急合并。 V2 codec 这块性能还有改善的空间,我还是坚持用 protobuf,但是可以再想办法降低下损耗。

OK

@killme2008
Copy link
Contributor Author

killme2008 commented Apr 22, 2019

@fengjiachun 可以看下最新两个 commit ,优化后, 200 万次(并发20个线程)的 encode/decode 512 个字节数据的 log entry,三次结果为:

V1 codec cost:755 ms.
Total log size:1205400000 bytes.
V2 codec cost:1135 ms.
Total log size:1183706496 bytes.


V1 codec cost:883 ms.
Total log size:1205400000 bytes.
V2 codec cost:1184 ms.
Total log size:1183706496 bytes.


V1 codec cost:836 ms.
Total log size:1205400000 bytes.
V2 codec cost:1197 ms.
Total log size:1183706496 bytes.

这个差距我觉的暂时是可以接受的,大小缩小在 1%~2% 之间, data 是完全随机的数据。

@fengjiachun
Copy link
Contributor

赞👍 节省了一次 toBytesArray 的 memory copy

* (feat) optimize encodeV2, avoid some memory copy

* (feat) typo and clear code

* (feat) back to old code style with 'for'

* (feat) back to old code style with 'for'

* (feat) rename method
* (feat) optimize decode

* (feat) optimize decode

* (feat) rm unsafe dependency
@killme2008
Copy link
Contributor Author

这个先合并掉? @fengjiachun

@fengjiachun fengjiachun merged commit d23843b into feature/log-entry-enhances Apr 24, 2019
@fengjiachun fengjiachun deleted the feature/log-checksum branch April 24, 2019 06:04
fengjiachun pushed a commit that referenced this pull request Apr 24, 2019
* (feat) Adds JRaftServiceFactory for custom services such as LogStorage/SnapshotStorage etc. #77

* (feat) update toString in NodeOptions

* (feat) Supports custom log entry serialization, #106 (#111)

* (feat) Adds JRaftServiceFactory for custom services such as LogStorage/SnapshotStorage etc. #77

* (feat) update toString in NodeOptions

* (feat) Supports custom log entry serialization, #106

* (fix) testSaveFail

* (feat) Adds serviceFactory to BootstrapOptions and fix tests

* (fix) by code review comments

* Log entry checksum validation (#123)

* (feat) Adds JRaftServiceFactory for custom services such as LogStorage/SnapshotStorage etc. #77

* (feat) update toString in NodeOptions

* (feat) Supports custom log entry serialization, #106

* (fix) testSaveFail

* (feat) Adds serviceFactory to BootstrapOptions and fix tests

* (fix) by code review comments

* (feat) Impl log entry checksum

* (feat) remove RaftGroupService default constructor

* (feat) Checksum in iterator and readUserCommitLog

* (feat) Refactor old version 1 log entry codec

* (feat) Impl v2 codec factory supports checksum

* (feat) format code

* (feat) Adds reserved flag

* (feat) Adds test for v2 codec

* (feat) Adds log entry checksum test

* (fix) header comments

* (fix) comment

* (fix) by code review comments

* (feat) tweak v2 codec performance

* (feat) Use AsciiStringUtil.unsafeEncode replacs Utils.getBytes in peer checksum

* (feat) Use ThreadLocal to replace FastThreadLocal

* (feat) forgot CrcUtilTest

* (feat) remove unnecessary method handler (#124)

* Feat/optimize encode (#126)

* (feat) optimize encodeV2, avoid some memory copy

* (feat) typo and clear code

* (feat) back to old code style with 'for'

* (feat) back to old code style with 'for'

* (feat) rename method

* feat/optimize v2 decode (#127)

* (feat) optimize decode

* (feat) optimize decode

* (feat) rm unsafe dependency
@fengjiachun fengjiachun mentioned this pull request Aug 15, 2019
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants