-
Notifications
You must be signed in to change notification settings - Fork 15.4k
KAFKA-13603: Allow the empty active segment to have missing offset index during recovery #11345
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
Changes from 4 commits
ecb8692
9904830
ac6e9cd
a2c51d9
82913f0
b915fe2
4df64d8
0f2f0a8
c50bff2
603627f
56d3f06
425a7a7
f345831
069ac5b
049c856
56d64cb
7bb37b8
40506eb
6f70af3
23314ea
d775f14
4d88092
099cb42
229a537
14239c0
8b7e0c9
0c8e48b
25821db
47ba1d5
885db15
5bb1a47
e00906a
bd21bb2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1505,7 +1505,7 @@ class UnifiedLog(@volatile var logStartOffset: Long, | |
| /** | ||
| * Flush all local log segments | ||
| */ | ||
| def flush(): Unit = flush(logEndOffset) | ||
| def flush(): Unit = flush(logEndOffset + 1) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we add a comment why we need to flush to logEndOffset + 1? Also, could we have a test that verifies the index file is present after an empty segment is rolled and the broker is shut down?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the comment. Will write it later. Was too busy yesterday and didn't have time to do so. |
||
|
|
||
| /** | ||
| * Flush local log segments for all offsets up to offset-1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add unit test coverage for this change in
UnifiedLogTest.scala?