Open
Conversation
add logging statement for network injection code
fanminshi
added a commit
that referenced
this pull request
Jul 27, 2017
This pr adds a check in compaction to ensure that setting keep to nil only if this is the latest compaction in queue and merges the keep from prev ongoing compaction to this scheduled compaction allows HashKV to computes Hash from previous keep. Scenario #1 Suppose that HashKV() starts after compact(A) and compact(B) has scheduled and after completion of compact(A) but before the completion of compact(B). Before this PR: Completion of compact(A) sets keep to nil to indicate that there are no ongoing compactions. This assumption causes HashKV to hash all mvcc keys including keys that hasn't been deleted by compact(B) up to a rev. Hence, HashKV again after compact(B) returns a different Hash that during compact(B). After this PR: Completion of compact(A) sets keep to nil if current compactRev == compactRev(compact(A)). Since calling compact(B) changes current compactRev, then current compactRev != compactRev(compact(A)). Then HashKV will base on the keep set during compact(B) instead of nil which return the correct hash during compact(B). Scenario etcd-io#2 Suppose that HashKV() starts after compact(A) and compact(B) has scheduled and before the completion of compact(A) and compact(B). Before this PR: The start of compact(B) sets keep to compact(B) and overrides the keep from compact(A). Since compact(A) hasn't finished yet, HashKV needs to hash revision from keep of compact(A) but it is overridden. Hence HashKV misses revision from keep of compact(A) would result hash difference. After this PR: The start of compact(B) merges keep from compact(B) and compact(A). Even though compact(A) hasn't finished, HashKV can retrieve from keep of compact(A). Hence HashKV doesn't miss any revisions and will compute Hash correctly.
fanminshi
added a commit
that referenced
this pull request
Jul 27, 2017
This pr adds a check in compaction to ensure that setting keep to nil only if this is the latest compaction in queue and merges the keep from prev ongoing compaction to this scheduled compaction allows HashKV to computes Hash from previous keep. Scenario #1 Suppose that HashKV() starts after compact(A) and compact(B) has scheduled and after completion of compact(A) but before the completion of compact(B). Before this PR: Completion of compact(A) sets keep to nil to indicate that there are no ongoing compactions. This assumption causes HashKV to hash all mvcc keys including keys that hasn't been deleted by compact(B) up to a rev. Hence, HashKV again after compact(B) returns a different Hash that during compact(B). After this PR: Completion of compact(A) sets keep to nil if current compactRev == compactRev(compact(A)). Since calling compact(B) changes current compactRev, then current compactRev != compactRev(compact(A)). Then HashKV will base on the keep set during compact(B) instead of nil which return the correct hash during compact(B). Scenario etcd-io#2 Suppose that HashKV() starts after compact(A) and compact(B) has scheduled and before the completion of compact(A) and compact(B). Before this PR: The start of compact(B) sets keep to compact(B) and overrides the keep from compact(A). Since compact(A) hasn't finished yet, HashKV needs to hash revision from keep of compact(A) but it is overridden. Hence HashKV misses revision from keep of compact(A) would result hash difference. After this PR: The start of compact(B) merges keep from compact(B) and compact(A). Even though compact(A) hasn't finished, HashKV can retrieve from keep of compact(A). Hence HashKV doesn't miss any revisions and will compute Hash correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
add logging statement for network injection code