KAFKA-9373: Improve shutdown performance via lazy accessing the offset and time indices by efeg · Pull Request #7900 · apache/kafka

efeg · 2020-01-07T02:48:02Z

KAFKA-7283 enabled lazy mmap on index files by initializing indices on-demand rather than performing costly disk/memory operations when creating all indices on broker startup. This helped reducing the startup time of brokers. However, segment indices are still created on closing segments, regardless of whether they need to be closed or not.

This patch:

Improves shutdown performance via lazy accessing the offset and time indices.
Eliminates redundant disk accesses and memory mapped operations while deleting or renaming files that back segment indices.
Prevents illegal accesses to underlying indices of a closed segment, which would lead to memory leaks due to recreation of the underlying memory mapped objects.

In our evaluations in a cluster with 31 brokers, where each broker has 13K to 20K segments, we observed up to 2 orders of magnitude faster LogManager shutdown times with this patch -- i.e. dropping the LogManager shutdown time of each broker from 10s of seconds to 100s of milliseconds.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

…ndices. KAFKA-7283 enabled lazy mmap on index files by initializing indices on-demand rather than performing costly disk/memory operations when creating all indices on broker startup. This helped reducing the startup time of brokers. However, segment indices are still created on closing segments, regardless of whether they needs to be closed or not. This patch: * Improves shutdown performance via lazy accessing the offset and time indices. * Eliminates redundant disk accesses and memory mapped operations while deleting or renaming files that back segment indices. * Prevents illegal accesses to underlying indices of a closed segment, which would lead to memory leaks due to recreation of the underlying memory mapped objects.

ijuma · 2020-01-07T09:11:40Z

core/src/main/scala/kafka/log/LazyIndex.scala

+  * leaks due to recreation of underlying memory mapped object.
  *
-  * Methods of this class are thread safe. Make sure to check `AbstractIndex` subclasses documentation
-  * to establish their thread safety.


Why has the thread safety comment been removed?

ijuma · 2020-01-07T09:14:29Z

Nice optimization. I think we should consider delegating to the underlying implementations instead of adding logic to the wrapper.

ijuma · 2020-01-28T14:05:36Z

@efeg Any thoughts on the above?

efeg · 2020-01-29T00:50:56Z

@ijuma Thanks for the review and sorry for the delay on addressing the comments! -- I will try to update the PR by next week.

ijuma · 2020-01-29T05:52:16Z

@efeg thanks, looking forward to it.

ijuma · 2020-03-25T14:53:14Z

@efeg Seems like you have been busy. Since this is a very useful change, I went ahead and submitted a new pull request with the approach I suggested (with you as co-author):

#8346

@efeg

…ndexes KAFKA-7283 enabled lazy mmap on index files by initializing indices on-demand rather than performing costly disk/memory operations when creating all indices on broker startup. This helped reducing the startup time of brokers. However, segment indices are still created on closing segments, regardless of whether they need to be closed or not. This is a cleaned up version of apache#7900, which was submitted by @efeg. It eliminates unnecessary disk accesses and memory map operations while deleting, renaming or closing offset and time indexes. In a cluster with 31 brokers, where each broker has 13K to 20K segments, @efeg and team observed up to 2 orders of magnitude faster LogManager shutdown times - i.e. dropping the LogManager shutdown time of each broker from 10s of seconds to 100s of milliseconds. To avoid confusion between `renameTo` and `setFile`, I replaced the latter with the more restricted updateParentDir` (it turns out that's all we need). Co-authored-by: Adem Efe Gencer <[email protected]> Co-authored-by: Ismael Juma <[email protected]>

@efeg

…ndexes (#8346) KAFKA-7283 enabled lazy mmap on index files by initializing indices on-demand rather than performing costly disk/memory operations when creating all indices on broker startup. This helped reducing the startup time of brokers. However, segment indices are still created on closing segments, regardless of whether they need to be closed or not. This is a cleaned up version of #7900, which was submitted by @efeg. It eliminates unnecessary disk accesses and memory map operations while deleting, renaming or closing offset and time indexes. In a cluster with 31 brokers, where each broker has 13K to 20K segments, @efeg and team observed up to 2 orders of magnitude faster LogManager shutdown times - i.e. dropping the LogManager shutdown time of each broker from 10s of seconds to 100s of milliseconds. To avoid confusion between `renameTo` and `setFile`, I replaced the latter with the more restricted updateParentDir` (it turns out that's all we need). Reviewers: Jun Rao <[email protected]>, Andrew Choi <[email protected]> Co-authored-by: Adem Efe Gencer <[email protected]> Co-authored-by: Ismael Juma <[email protected]>

ijuma · 2020-03-26T11:29:58Z

Thanks for the PR, we merged it via #8346.

efeg · 2020-03-26T16:40:48Z

Thanks for the updated PR @ijuma!

@efeg

…ng unnecessary loading of indexes (apache#8346) TICKET = LI_DESCRIPTION = KAFKA-7283 enabled lazy mmap on index files by initializing indices on-demand rather than performing costly disk/memory operations when creating all indices on broker startup. This helped reducing the startup time of brokers. However, segment indices are still created on closing segments, regardless of whether they need to be closed or not. This is a cleaned up version of apache#7900, which was submitted by @efeg. It eliminates unnecessary disk accesses and memory map operations while deleting, renaming or closing offset and time indexes. In a cluster with 31 brokers, where each broker has 13K to 20K segments, @efeg and team observed up to 2 orders of magnitude faster LogManager shutdown times - i.e. dropping the LogManager shutdown time of each broker from 10s of seconds to 100s of milliseconds. To avoid confusion between `renameTo` and `setFile`, I replaced the latter with the more restricted updateParentDir` (it turns out that's all we need). Reviewers: Jun Rao <[email protected]>, Andrew Choi <[email protected]> Co-authored-by: Adem Efe Gencer <[email protected]> Co-authored-by: Ismael Juma <[email protected]> EXIT_CRITERIA = HASH [222726d]

@efeg

…ng unnecessary loading of indexes (apache#8346) TICKET = LI_DESCRIPTION = KAFKA-7283 enabled lazy mmap on index files by initializing indices on-demand rather than performing costly disk/memory operations when creating all indices on broker startup. This helped reducing the startup time of brokers. However, segment indices are still created on closing segments, regardless of whether they need to be closed or not. This is a cleaned up version of apache#7900, which was submitted by @efeg. It eliminates unnecessary disk accesses and memory map operations while deleting, renaming or closing offset and time indexes. In a cluster with 31 brokers, where each broker has 13K to 20K segments, @efeg and team observed up to 2 orders of magnitude faster LogManager shutdown times - i.e. dropping the LogManager shutdown time of each broker from 10s of seconds to 100s of milliseconds. To avoid confusion between `renameTo` and `setFile`, I replaced the latter with the more restricted updateParentDir` (it turns out that's all we need). Reviewers: Jun Rao <[email protected]>, Andrew Choi <[email protected]> Co-authored-by: Adem Efe Gencer <[email protected]> Co-authored-by: Ismael Juma <[email protected]> EXIT_CRITERIA = HASH [222726d]

@efeg

…ng unnecessary loading of indexes (apache#8346) TICKET = LI_DESCRIPTION = KAFKA-7283 enabled lazy mmap on index files by initializing indices on-demand rather than performing costly disk/memory operations when creating all indices on broker startup. This helped reducing the startup time of brokers. However, segment indices are still created on closing segments, regardless of whether they need to be closed or not. This is a cleaned up version of apache#7900, which was submitted by @efeg. It eliminates unnecessary disk accesses and memory map operations while deleting, renaming or closing offset and time indexes. In a cluster with 31 brokers, where each broker has 13K to 20K segments, @efeg and team observed up to 2 orders of magnitude faster LogManager shutdown times - i.e. dropping the LogManager shutdown time of each broker from 10s of seconds to 100s of milliseconds. To avoid confusion between `renameTo` and `setFile`, I replaced the latter with the more restricted updateParentDir` (it turns out that's all we need). Reviewers: Jun Rao <[email protected]>, Andrew Choi <[email protected]> Co-authored-by: Adem Efe Gencer <[email protected]> Co-authored-by: Ismael Juma <[email protected]> EXIT_CRITERIA = HASH [222726d]

ijuma reviewed Jan 7, 2020

View reviewed changes

Be ultra lazy in loading lazy indices during index deletion.

9984743

efeg force-pushed the fix/shutdownPerformance branch from 308c917 to 9984743 Compare February 4, 2020 00:02

ijuma mentioned this pull request Mar 25, 2020

KAFKA-9373: Reduce shutdown time by avoiding unnecessary loading of indexes #8346

Merged

3 tasks

ijuma closed this Mar 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KAFKA-9373: Improve shutdown performance via lazy accessing the offset and time indices#7900

KAFKA-9373: Improve shutdown performance via lazy accessing the offset and time indices#7900
efeg wants to merge 2 commits intoapache:trunkfrom
efeg:fix/shutdownPerformance

efeg commented Jan 7, 2020

Uh oh!

ijuma Jan 7, 2020

Uh oh!

ijuma commented Jan 7, 2020

Uh oh!

ijuma commented Jan 28, 2020

Uh oh!

efeg commented Jan 29, 2020

Uh oh!

ijuma commented Jan 29, 2020

Uh oh!

ijuma commented Mar 25, 2020

Uh oh!

ijuma commented Mar 26, 2020

Uh oh!

efeg commented Mar 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

efeg commented Jan 7, 2020

Committer Checklist (excluded from commit message)

Uh oh!

ijuma Jan 7, 2020

Choose a reason for hiding this comment

Uh oh!

ijuma commented Jan 7, 2020

Uh oh!

ijuma commented Jan 28, 2020

Uh oh!

efeg commented Jan 29, 2020

Uh oh!

ijuma commented Jan 29, 2020

Uh oh!

ijuma commented Mar 25, 2020

Uh oh!

ijuma commented Mar 26, 2020

Uh oh!

efeg commented Mar 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants