-
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 8 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 |
|---|---|---|
|
|
@@ -655,6 +655,7 @@ class UnifiedLog(@volatile var logStartOffset: Long, | |
| def close(): Unit = { | ||
| debug("Closing log") | ||
| lock synchronized { | ||
| flush(logEndOffset + 1) | ||
| maybeFlushMetadataFile() | ||
| localLog.checkIfMemoryMappedBufferClosed() | ||
| producerExpireCheck.cancel(true) | ||
|
|
@@ -1504,6 +1505,8 @@ class UnifiedLog(@volatile var logStartOffset: Long, | |
|
|
||
| /** | ||
| * Flush all local log segments | ||
| * We have to pass logEngOffset + 1 to the `def flush(offset: Long): Unit` function to flush empty | ||
|
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. This comment is in the wrong place. |
||
| * active segments, which is important to make sure we don't lose logEndOffset during shutdown. | ||
| */ | ||
| def flush(): Unit = flush(logEndOffset) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1630,6 +1630,20 @@ class UnifiedLogTest { | |
| assertThrows(classOf[OffsetOutOfRangeException], () => LogTestUtils.readLog(log, 1026, 1000)) | ||
| } | ||
|
|
||
| @Test | ||
| def testFlushingEmptyActiveSegments(): Unit = { | ||
|
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. Is it possible to add another test in
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. Done with other comments. Will work on this one next week.
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. Sounds good. |
||
| /* create a multipart log with 100 messages */ | ||
| val logConfig = LogTestUtils.createLogConfig(segmentBytes = 100) | ||
| val log = createLog(logDir, logConfig) | ||
| val numMessages = 2 | ||
| val messageSets = (0 until numMessages).map(i => TestUtils.singletonRecords(value = i.toString.getBytes, | ||
| timestamp = mockTime.milliseconds)) | ||
| messageSets.foreach(log.appendAsLeader(_, leaderEpoch = 0)) | ||
| log.roll() | ||
| log.close() | ||
| assertEquals(numMessages + 1, logDir.listFiles(_.getName.endsWith(".index")).length) | ||
| } | ||
|
|
||
| /** | ||
| * Test that covers reads and writes on a multisegment log. This test appends a bunch of messages | ||
| * and then reads them all back and checks that the message read and offset matches what was appended. | ||
|
|
||
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.
This seems to have the same problem that the log recovery point could be moved to logEndOffset + 1, which is a bit weird?
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.
Actually, this doesn't work. I am trying to figure out a proper solution.