Skip to content

KAFKA-14462; [14/N] Add PartitionWriter#13675

Merged
dajac merged 11 commits intoapache:trunkfrom
dajac:KAFKA-14462-14
Jun 6, 2023
Merged

KAFKA-14462; [14/N] Add PartitionWriter#13675
dajac merged 11 commits intoapache:trunkfrom
dajac:KAFKA-14462-14

Conversation

@dajac
Copy link
Copy Markdown
Member

@dajac dajac commented May 4, 2023

This patch introduces the PartitionWriter interface in the group-coordinator module. The ReplicaManager resides in the core module and it is thus not accessible from the group-coordinator one. The PartitionWriterImpl is basically an implementation of the interface residing in core which interfaces with the ReplicaManager.

One notable difference from the usual produce path is that the PartitionWriter returns the offset following the written records. This is then used by the coordinator runtime (coming later) to track when the request associated with the write can be completed.

Committer Checklist (excluded from commit message)

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

@dajac dajac added the KIP-848 The Next Generation of the Consumer Rebalance Protocol label May 4, 2023
@dajac dajac changed the title KAFKA-14462; [14/N]; Add PartitionWriter KAFKA-14462; [14/N] Add PartitionWriter May 4, 2023
Comment thread core/src/main/scala/kafka/coordinator/group/PartitionWriterImpl.scala Outdated
Comment thread core/src/main/scala/kafka/server/ReplicaManager.scala Outdated
Comment thread core/src/test/scala/unit/kafka/coordinator/group/PartitionWriterImplTest.scala Outdated
@dajac dajac requested a review from jolshan June 2, 2023 09:32
Comment thread core/src/main/scala/kafka/coordinator/group/PartitionWriterImpl.scala Outdated
@jolshan
Copy link
Copy Markdown
Member

jolshan commented Jun 2, 2023

Hey David -- whenever we update appendRecords we have to modify a ton of tests that mock the method. (Basically add a ton of any(), since the mocker doesn't get default args). There are 283 failing now 😅

See here for an example of a PR that updates them: https://github.com/apache/kafka/pull/13391/files

@dajac
Copy link
Copy Markdown
Member Author

dajac commented Jun 2, 2023

Yeah, I have noticed all the failed tests. I will fix them.

Comment thread core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala Outdated
Comment thread core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala Outdated
Comment thread core/src/main/scala/kafka/server/ReplicaManager.scala Outdated
Comment thread core/src/main/scala/kafka/server/ReplicaManager.scala
@jolshan
Copy link
Copy Markdown
Member

jolshan commented Jun 5, 2023

These connect and kraft tests are a little concerning. They seem unrelated but maybe we should file a jira.

Comment thread core/src/main/scala/kafka/coordinator/group/CoordinatorPartitionWriter.scala Outdated
Copy link
Copy Markdown
Contributor

@jeffkbkim jeffkbkim left a comment

Choose a reason for hiding this comment

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

LGTM!

@dajac dajac merged commit 7d147cf into apache:trunk Jun 6, 2023
@dajac dajac deleted the KAFKA-14462-14 branch June 6, 2023 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

KIP-848 The Next Generation of the Consumer Rebalance Protocol

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants