apache / apache/bookkeeper

[improve][server]Add digest for logFileHeader and ledgersMap in entryLog file

Open
#4,658 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
2k
Forks
976
Avg merge
6d 15h
Merged PRs (30d)
7

Description

Hi, I recently read BookKeeper GC source code to figure out what situation will cause data loss in pull request [Fix the data loss issue that caused by the wrong entry log header #4607](https://github.com/apache/bookkeeper/pull/4607). See [#4607 (comment)](https://github.com/apache/bookkeeper/pull/4607#issuecomment-3228330508).
I think logFileHeader and ledgersMap are very important part of entryLog file, and these data's integrity and consistency should be guaranteed by digest.
We should add digest logFileHeader and ledgersMap in appendLedgersMap() method. I just list the original code to express my idea.
https://github.com/apache/bookkeeper/blob/e80d0318cfdebbc79f37d9120de8df28c8c1c13a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java#L139-L210
In GC thread, we should valid digest in extractEntryLogMetadataFromIndex() method, if digest validation failed, we should directly throw an IOException and fall back to extractEntryLogMetadataByScanning().
https://github.com/apache/bookkeeper/blob/e80d0318cfdebbc79f37d9120de8df28c8c1c13a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java#L1072-L1159
And, the following break should be replaced with an IOException, if ledgersMapSize <= 0, it an invalid value, we should throw an IOException and fall back into scanning, not break out the while loop, which may lead to uncertainty.
https://github.com/apache/bookkeeper/blob/e80d0318cfdebbc79f37d9120de8df28c8c1c13a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/DefaultEntryLogger.java#L1104-L1106
If we use digest validation, [Fix the data loss issue that caused by the wrong entry log header #4607](https://github.com/apache/bookkeeper/pull/4607) data loss may probably not happen.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.