Skip to content

KAFKA-14471: Move IndexEntry and related to storage module#12993

Merged
ijuma merged 1 commit intoapache:trunkfrom
ijuma:kafka-14471-move-index-entry-to-storage
Dec 17, 2022
Merged

KAFKA-14471: Move IndexEntry and related to storage module#12993
ijuma merged 1 commit intoapache:trunkfrom
ijuma:kafka-14471-move-index-entry-to-storage

Conversation

@ijuma
Copy link
Copy Markdown
Member

@ijuma ijuma commented Dec 14, 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 requested a review from dengziming December 14, 2022 21:55
@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Dec 14, 2022

@dengziming do you happen to have cycles to review this PR?

@dengziming
Copy link
Copy Markdown
Member

@dengziming do you happen to have cycles to review this PR?

Sure, I have added it to my review backlog.

Copy link
Copy Markdown
Member

@dengziming dengziming left a comment

Choose a reason for hiding this comment

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

LGTM, the 2 comments are only trivial questions.


package kafka.log

class CorruptIndexException(message: String) extends RuntimeException(message)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's weird that git thinks CorruptIndexException is rename to IndexEntry.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, I agree. Not sure we can do anything about this though.

}

@Override
public int hashCode() {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Scala case class will generated same methods, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Correct, case classes automatically generate toString, hashCode and equals and hence why I did the same here. However, the underlying implementation is not the same, but that's not an issue for us.

@ijuma
Copy link
Copy Markdown
Member Author

ijuma commented Dec 17, 2022

JDK 8 build passed while there was 1 unrelated failure for the each of the other JDKs:

Build / JDK 11 and Scala 2.13 / testBumpTransactionalEpoch(String).quorum=kraft – kafka.api.TransactionsTest
1m 18s
Build / JDK 17 and Scala 2.13 / testTransactionAfterTransactionIdExpiresButProducerIdRemains(String).quorum=zk – kafka.api.ProducerIdExpirationTest

Merging to trunk.

@ijuma ijuma merged commit 95dc9d9 into apache:trunk Dec 17, 2022
@ijuma ijuma deleted the kafka-14471-move-index-entry-to-storage branch December 17, 2022 18:07
mimaison pushed a commit that referenced this pull request Dec 17, 2022
…on` (#13008)

#11390 and #12993 were merged in relatively quick succession, which resulted in compiler errors that weren't present when each change was on top of trunk.

Reviewers: Mickael Maison <mickael.maison@gmail.com>
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: dengziming <dengziming1993@gmail.com>
guozhangwang pushed a commit to guozhangwang/kafka that referenced this pull request Jan 25, 2023
…on` (apache#13008)

apache#11390 and apache#12993 were merged in relatively quick succession, which resulted in compiler errors that weren't present when each change was on top of trunk.

Reviewers: Mickael Maison <mickael.maison@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants