Skip to content
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

KAFKA-14474: Move OffsetIndex to storage module #13009

Merged
merged 1 commit into from Dec 20, 2022

Conversation

ijuma
Copy link
Contributor

@ijuma ijuma commented Dec 17, 2022

For broader context on this change, please check:

Committer Checklist (excluded from commit message)

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

@ijuma ijuma force-pushed the kafka-14474-move-offset-index-to-storage branch 4 times, most recently from 8c169ab to bfc38de Compare December 19, 2022 20:01
@ijuma ijuma marked this pull request as ready for review December 19, 2022 20:02
@ijuma ijuma force-pushed the kafka-14474-move-offset-index-to-storage branch from bfc38de to b68f9c6 Compare December 20, 2022 03:39
For broader context on this change, please check:

KAFKA-14470: Move log layer to storage module
@ijuma ijuma force-pushed the kafka-14474-move-offset-index-to-storage branch from b68f9c6 to 3fd84ef Compare December 20, 2022 03:47
@ijuma ijuma requested a review from junrao December 20, 2022 04:06
@ijuma
Copy link
Contributor Author

ijuma commented Dec 20, 2022

Test failures look unrelated, re-running just in case.

Copy link
Member

@satishd satishd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ijuma : Thanks for the PR, LGTM.

Copy link
Contributor

@junrao junrao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ijuma : Thanks for the PR. LGTM. Just a minor comment.

}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra newline

Copy link
Contributor Author

@ijuma ijuma Dec 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@junrao I think most editors expect a new line at the end. For example, IntelliJ adds it automatically after I delete the new line and save the file (I tried it just now). And vim doesn't even show that there is a new line. So, I think we should probably leave it in.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. Sounds good.

@ijuma
Copy link
Contributor Author

ijuma commented Dec 20, 2022

Failures are unrelated.

@ijuma ijuma merged commit 82c9216 into apache:trunk Dec 20, 2022
@ijuma ijuma deleted the kafka-14474-move-offset-index-to-storage branch December 20, 2022 19:45
guozhangwang pushed a commit to guozhangwang/kafka that referenced this pull request Jan 25, 2023
For broader context on this change, please check:

* KAFKA-14470: Move log layer to storage module

Reviewers: Jun Rao <junrao@gmail.com>, Satish Duggana <satishd@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants