Skip to content

KAFKA-14701; Move PartitionAssignor to new group-coordinator-api module#16198

Merged
dajac merged 11 commits intoapache:trunkfrom
dajac:group-coordinator-api-module-2
Jun 6, 2024
Merged

KAFKA-14701; Move PartitionAssignor to new group-coordinator-api module#16198
dajac merged 11 commits intoapache:trunkfrom
dajac:group-coordinator-api-module-2

Conversation

@dajac
Copy link
Copy Markdown
Member

@dajac dajac commented Jun 4, 2024

This patch moves the PartitionAssignor interface and all the related classes to a newly created group-coordinator/api module, following the pattern used by the storage and tools modules.

I'll update the KIP when this PR is merged.

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 Jun 4, 2024
@dajac dajac requested a review from chia7712 June 4, 2024 19:16
Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@dajac nice patch! two small comments are left, and please take a look at build error

package org.apache.kafka.coordinator.group.assignor;
package org.apache.kafka.coordinator.group.api.assignor;

import org.apache.kafka.common.Uuid;
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.

I noticed that not all interfaces have @InterfaceStability.Unstable. Could you share the context to me?

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.

For another, GroupSpec#MemberSubscriptionSpec return the interface but GroupSpec#memberAssignment return a map struct. If it is public API now, maybe return MemberAssignment is more flexible if we want to enrich it in the future?

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.

I noticed that not all interfaces have @InterfaceStability.Unstable. Could you share the context to me?

Fixed. Thanks.

For another, GroupSpec#MemberSubscriptionSpec return the interface but GroupSpec#memberAssignment return a map struct. If it is public API now, maybe return MemberAssignment is more flexible if we want to enrich it in the future?

Good question. Reusing MemberAssignment is not ideal because we want to have the ability to pass internal objects. In this case, it could be backed by the Assignment. So, we could think of using an interface though.

On thing to consider is whether the MemberAssignment returned by the assignor would always be the same as the one provided to the assignor. The actual assignment (the map) is for sure. What do you think?

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.

So, we could think of using an interface though.

+1

On thing to consider is whether the MemberAssignment returned by the assignor would always be the same as the one provided to the assignor. The actual assignment (the map) is for sure. What do you think?

not sure whether I have got the point. The member assignment in GroupAssignment returned from PartitionAssignor#assign is the new assignment. Hence, it should be different to assignment in GroupSpec.

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.

You suggested to use MemberAssignment in your previous comment. MemberAssignment is already used in GroupAssignment returned by PartitionAssignor#assign. I am not sure if you meant to reuse the same MemberAssignment in both places.

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.

You suggested to use MemberAssignment in your previous comment. MemberAssignment is already used in GroupAssignment returned by PartitionAssignor#assign. I am not sure if you meant to reuse the same MemberAssignment in both places.

ummm, I guess I did not notice MemberAssignment is used by GroupAssignment before :_

Anyway, having a interface to replace Map struct is good way to me. Or we can keep current version if the tag "unstable" give the room to modify those public stuff :)

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.

I am playing with this. I will suggest something a bit later today.

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.

@chia7712 I gave it a go. You can check the last commit to see how it looks like. I also moved some classes between internal packages while here.

@dajac
Copy link
Copy Markdown
Member Author

dajac commented Jun 5, 2024

I fixed the build issue.

Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

@dajac thanks for updated PR

@@ -88,4 +90,5 @@ public String toString() {
", memberAssignment=" + memberAssignment +
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.

toString still uses previous naming MemberSubscriptionSpecImpl

private void maybeRevokePartitions() {
for (String memberId : groupSpec.memberIds()) {
Map<Uuid, Set<Integer>> oldAssignment = groupSpec.memberAssignment(memberId);
Map<Uuid, Set<Integer>> oldAssignment = groupSpec.memberAssignment(memberId).partitions();
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.

As we have MemberAssignment interface now, could we manipulate MemberAssignment instead of raw map? Also, maybe we can remove isImmutableMap by checking the impl of MemberAssignment. For example:

            MemberAssignment oldAssignment = groupSpec.memberAssignment(memberId);
            MemberAssignment newAssignment = null;

            // The assignor expects to receive the assignment as an immutable map. It leverages
            // this knowledge in order to avoid having to copy all assignments.
            if (! (oldAssignment instanceof Assignment)) {
                throw new IllegalStateException("The assignor expect an immutable map.");
            }

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.

We can probably do further improvements but this goes beyond the scope of this PR. We can do them as follow-ups. Does it work for you?

Regarding the usage of raw maps, the assignors work with them at the moment so we cannot change it easily. We are working hard on refactoring the assignors to perform better. Once we are done with this, we can consider it. As the interface will remain unstable until 4.0, we can still do it afterwards.

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.

that makes sense to me 😄

* @param subscribedTopicIds The set of subscribed topic Ids.
* @param memberAssignment The current member assignment.
*/
public MemberSubscriptionSpecImpl(
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.

Constructs a new {@code MemberSubscriptionSpecImpl}. -> Constructs a new {@code MemberSubscriptionAndAssignmentImpl}.

(I can't add comment on line#37)

* @return The member's subscription metadata.
* @throws IllegalArgumentException If the member Id isn't found.
*/
MemberSubscriptionSpec memberSubscription(String memberId);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: why did we rename it to MemberSubscription?

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.

Now that we have MemberAssignment, MemberSubscriptionSpec looked weird. So I renamed it to align them.

@InterfaceStability.Unstable
public interface MemberAssignment {
/**
* @return Target partitions keyed by topic Ids.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Since this is a generic interface should we say assigned partitions keyed by topic Id?

* The partition assignment for a consumer group member.
*/
@InterfaceStability.Unstable
public interface MemberAssignment {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the change! I was wondering if this should be in a separate PR? Since it's not related to moving files to different modules and we're adding a new interface and making changes as a consequence?

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.

We could have done a separate PR but it is also fine doing it here as the main reviewer asked for it.

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.

oh, i neglected this PR aims to complete the migration, but I do love this change 😄

* The target partitions assigned to this member keyed by topicId.
*/
private final Map<Uuid, Set<Integer>> targetPartitions;
private final Map<Uuid, Set<Integer>> assignment;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: should we name it partitions here as well? to keep it uniform?

* topic and partition metadata for the topics that the consumer group is subscribed to.
*/
public class SubscribedTopicMetadata implements SubscribedTopicDescriber {
public class SubscribedTopicDescriberImpl implements SubscribedTopicDescriber {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nice! I like that everything is uniform with this naming

@rreddy-22
Copy link
Copy Markdown
Contributor

Thanks for the PR!

* An empty map is returned if the member Id isn't found.
* @param memberId The member Id.
* @return The member's assignment or an empty assignment if the
* member does not have one.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: i think keeping the indentation the same looks cleaner

* An immutable assignment for a member.
*/
public class Assignment {
public class Assignment implements MemberAssignment {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could you help me understand the difference between Assignment and MemberAssignmentImpl?

Also, the name seems a bit vague given that it implements MemberAssignment

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.

That's a good point. There are similar at the moment. Assignment should get extra fields in the future when we implement the client side assignor (if we ever do it) while MemberAssignment will stay as it is. We could consider merging them in the future.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So, Assignment is different from MemberAssignmentImpl in that it will also be used for client side assignors, possibly in the future, whereas MemberAssignmentImpl is only used for server side assignors. is this correct?

I think it might be worth adding a short comment or perhaps merging it now in a follow up PR.

", memberAssignment=" + memberAssignment +
')';
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: i think we can remove this

@dajac
Copy link
Copy Markdown
Member Author

dajac commented Jun 6, 2024

@chia7712 @jeffkbkim @rreddy-22 Thanks for your comments. I have addressed them.

Comment thread settings.gradle
'examples',
'generator',
'group-coordinator',
'group-coordinator:group-coordinator-api',
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.

We named other internal API modules just api, for example connect:api, storage:api. Should we stick to the same convention?

Copy link
Copy Markdown
Member

@mimaison mimaison Jun 6, 2024

Choose a reason for hiding this comment

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

I just noticed we have tools:tools-api too :(

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.

Reusing api does not work. Gradle gets confused about it and dependencies fail. There is a bug in gradle. This is why we actually use tools:tools-api and why we have project(":storage:api").name = "storage-api" later in this file. So we effectively, use module:module-api as names expect for connect.

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.

For reference: gradle/gradle#847

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.

@mimaison you raise a good discussion. I had the same question in creating tools-api module 😄

As new module, maybe storage:api can be rename to storage:storage-api for consistency.

@dajac dajac requested review from chia7712, jeffkbkim and rreddy-22 June 6, 2024 11:58
Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

there is a little question, but this patch is good enough to me :)


// private for testing
static MemberSubscriptionSpecImpl createMemberSubscriptionSpecImpl(
static MemberSubscriptionAndAssignmentImpl createMemberSubscriptionSpecImpl(
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.

Should it get renamed according to MemberSubscriptionAndAssignmentImpl? or it can be a constructor of MemberSubscriptionSpecImpl?

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.

Fixed it!

Copy link
Copy Markdown
Member

@chia7712 chia7712 left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Contributor

@rreddy-22 rreddy-22 left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good!

@dajac dajac merged commit 7d832cf into apache:trunk Jun 6, 2024
@dajac dajac deleted the group-coordinator-api-module-2 branch June 6, 2024 19:19
dajac added a commit that referenced this pull request Jun 6, 2024
…module (#16198)

This patch moves the `PartitionAssignor` interface and all the related classes to a newly created `group-coordinator/api` module, following the pattern used by the storage and tools modules.

Reviewers: Ritika Reddy <rreddy@confluent.io>, Jeff Kim <jeff.kim@confluent.io>, Chia-Ping Tsai <chia7712@gmail.com>
@dajac
Copy link
Copy Markdown
Member Author

dajac commented Jun 6, 2024

Merged to trunk and to 3.8.

apourchet added a commit to apourchet/kafka that referenced this pull request Jun 6, 2024
commit ee834d9
Author: Antoine Pourchet <antoine@responsive.dev>
Date:   Thu Jun 6 15:20:48 2024 -0600

    KAFKA-15045: (KIP-924 pt. 19) Update to new AssignmentConfigs (apache#16219)

    This PR updates all of the streams task assignment code to use the new AssignmentConfigs public class.

    Reviewers: Anna Sophie Blee-Goldman <ableegoldman@apache.org>

commit 8a2bc3a
Author: Bruno Cadonna <cadonna@apache.org>
Date:   Thu Jun 6 21:19:52 2024 +0200

    KAFKA-16903: Consider produce error of different task (apache#16222)

    A task does not know anything about a produce error thrown
    by a different task. That might lead to a InvalidTxnStateException
    when a task attempts to do a transactional operation on a producer
    that failed due to a different task.

    This commit stores the produce exception in the streams producer
    on completion of a send instead of the record collector since the
    record collector is on task level whereas the stream producer
    is on stream thread level. Since all tasks use the same streams
    producer the error should be correctly propagated across tasks
    of the same stream thread.

    For EOS alpha, this commit does not change anything because
    each task uses its own producer. The send error is still
    on task level but so is also the transaction.

    Reviewers: Matthias J. Sax <matthias@confluent.io>

commit 7d832cf
Author: David Jacot <djacot@confluent.io>
Date:   Thu Jun 6 21:19:20 2024 +0200

    KAFKA-14701; Move `PartitionAssignor` to new `group-coordinator-api` module (apache#16198)

    This patch moves the `PartitionAssignor` interface and all the related classes to a newly created `group-coordinator/api` module, following the pattern used by the storage and tools modules.

    Reviewers: Ritika Reddy <rreddy@confluent.io>, Jeff Kim <jeff.kim@confluent.io>, Chia-Ping Tsai <chia7712@gmail.com>

commit 79ea7d6
Author: Mickael Maison <mimaison@users.noreply.github.com>
Date:   Thu Jun 6 20:28:39 2024 +0200

    MINOR: Various cleanups in clients (apache#16193)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit a41f7a4
Author: Murali Basani <muralidhar.basani@aiven.io>
Date:   Thu Jun 6 18:06:25 2024 +0200

    KAFKA-16884 Refactor RemoteLogManagerConfig with AbstractConfig (apache#16199)

    Reviewers: Greg Harris <gharris1727@gmail.com>, Kamal Chandraprakash <kamal.chandraprakash@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>

commit 0ed104c
Author: Kamal Chandraprakash <kchandraprakash@uber.com>
Date:   Thu Jun 6 21:26:08 2024 +0530

    MINOR: Cleanup the storage module unit tests (apache#16202)

    - Use SystemTime instead of MockTime when time is not mocked
    - Use static assertions to reduce the line length
    - Fold the lines if it exceeds the limit
    - rename tp0 to tpId0 when it refers to TopicIdPartition

    Reviewers: Kuan-Po (Cooper) Tseng <brandboat@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>

commit f36a873
Author: Cy <yimck@uci.edu>
Date:   Thu Jun 6 08:46:49 2024 -0700

    MINOR: Added test for ClusterConfig#displayTags (apache#16110)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit 226f3c5
Author: Sanskar Jhajharia <122860866+sjhajharia@users.noreply.github.com>
Date:   Thu Jun 6 18:48:23 2024 +0530

    MINOR: Code cleanup in metadata module (apache#16065)

    Reviewers: Mickael Maison <mickael.maison@gmail.com>

commit ebe1e96
Author: Loïc GREFFIER <loic.greffier@michelin.com>
Date:   Thu Jun 6 13:40:31 2024 +0200

    KAFKA-16448: Add ProcessingExceptionHandler interface and implementations (apache#16187)

    This PR is part of KAFKA-16448 which aims to bring a ProcessingExceptionHandler to Kafka Streams in order to deal with exceptions that occur during processing.

    This PR brings ProcessingExceptionHandler interface and default implementations.

    Co-authored-by: Dabz <d.gasparina@gmail.com>
    Co-authored-by: sebastienviale <sebastien.viale@michelin.com>

    Reviewer: Bruno Cadonna <cadonna@apache.org>

commit b74b182
Author: Lianet Magrans <98415067+lianetm@users.noreply.github.com>
Date:   Thu Jun 6 09:45:36 2024 +0200

    KAFKA-16786: Remove old assignment strategy usage in new consumer (apache#16214)

    Remove usage of the partition.assignment.strategy config in the new consumer. This config is deprecated with the new consumer protocol, so the AsyncKafkaConsumer should not use or validate the property.

    Reviewers: Lucas Brutschy <lbrutschy@confluent.io>

commit f880ad6
Author: Alyssa Huang <ahuang@confluent.io>
Date:   Wed Jun 5 23:30:49 2024 -0700

    KAFKA-16530: Fix high-watermark calculation to not assume the leader is in the voter set (apache#16079)

    1. Changing log message from error to info - We may expect the HW calculation to give us a smaller result than the current HW in the case of quorum reconfiguration. We will continue to not allow the HW to actually decrease.
    2. Logic for finding the updated LeaderEndOffset for updateReplicaState is changed as well. We do not assume the leader is in the voter set and check the observer states as well.
    3. updateLocalState now accepts an additional "lastVoterSet" param which allows us to update the leader state with the last known voters. any nodes in this set but not in voterStates will be added to voterStates and removed from observerStates, any nodes not in this set but in voterStates will be removed from voterStates and added to observerStates

    Reviewers: Luke Chen <showuon@gmail.com>, José Armando García Sancio <jsancio@apache.org>

commit 3835515
Author: Okada Haruki <ocadaruma@gmail.com>
Date:   Thu Jun 6 15:10:13 2024 +0900

    KAFKA-16541 Fix potential leader-epoch checkpoint file corruption (apache#15993)

    A patch for KAFKA-15046 got rid of fsync on LeaderEpochFileCache#truncateFromStart/End for performance reason, but it turned out this could cause corrupted leader-epoch checkpoint file on ungraceful OS shutdown, i.e. OS shuts down in the middle when kernel is writing dirty pages back to the device.

    To address this problem, this PR makes below changes: (1) Revert LeaderEpochCheckpoint#write to always fsync
    (2) truncateFromStart/End now call LeaderEpochCheckpoint#write asynchronously on scheduler thread
    (3) UnifiedLog#maybeCreateLeaderEpochCache now loads epoch entries from checkpoint file only when current cache is absent

    Reviewers: Jun Rao <junrao@gmail.com>

commit 7763243
Author: Florin Akermann <florin.akermann@gmail.com>
Date:   Thu Jun 6 00:22:31 2024 +0200

    KAFKA-12317: Update FK-left-join documentation (apache#15689)

    FK left-join was changed via KIP-962. This PR updates the docs accordingly.

    Reviewers: Ayoub Omari <ayoubomari1@outlook.fr>, Matthias J. Sax <matthias@confluent.io>

commit 1134520
Author: Ayoub Omari <ayoubomari1@outlook.fr>
Date:   Thu Jun 6 00:05:04 2024 +0200

    KAFKA-16573: Specify node and store where serdes are needed (apache#15790)

    Reviewers: Matthias J. Sax <matthias@confluent.io>, Bruno Cadonna <bruno@confluent.io>, Anna Sophie Blee-Goldman <ableegoldman@apache.org>

commit 896af1b
Author: Sanskar Jhajharia <122860866+sjhajharia@users.noreply.github.com>
Date:   Thu Jun 6 01:46:59 2024 +0530

    MINOR: Raft module Cleanup (apache#16205)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit 0109a3f
Author: Antoine Pourchet <antoine@responsive.dev>
Date:   Wed Jun 5 14:09:37 2024 -0600

    KAFKA-15045: (KIP-924 pt. 17) State store computation fixed (apache#16194)

    Fixed the calculation of the store name list based on the subtopology being accessed.

    Also added a new test to make sure this new functionality works as intended.

    Reviewers: Anna Sophie Blee-Goldman <ableegoldman@apache.org>

commit 52514a8
Author: Greg Harris <greg.harris@aiven.io>
Date:   Wed Jun 5 11:35:32 2024 -0700

    KAFKA-16858: Throw DataException from validateValue on array and map schemas without inner schemas (apache#16161)

    Signed-off-by: Greg Harris <greg.harris@aiven.io>
    Reviewers: Chris Egerton <chrise@aiven.io>

commit f2aafcc
Author: Sanskar Jhajharia <122860866+sjhajharia@users.noreply.github.com>
Date:   Wed Jun 5 20:06:01 2024 +0530

    MINOR: Cleanups in Shell Module (apache#16204)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit bd9d68f
Author: Abhijeet Kumar <abhijeet.cse.kgp@gmail.com>
Date:   Wed Jun 5 19:12:25 2024 +0530

    KAFKA-15265: Integrate RLMQuotaManager for throttling fetches from remote storage (apache#16071)

    Reviewers: Kamal Chandraprakash<kamal.chandraprakash@gmail.com>, Luke Chen <showuon@gmail.com>, Satish Duggana <satishd@apache.org>

commit 62e5cce
Author: gongxuanzhang <gongxuanzhangmelt@gmail.com>
Date:   Wed Jun 5 18:57:32 2024 +0800

    KAFKA-10787 Update spotless version and remove support JDK8 (apache#16176)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit 02c794d
Author: Kamal Chandraprakash <kchandraprakash@uber.com>
Date:   Wed Jun 5 12:12:23 2024 +0530

    KAFKA-15776: Introduce remote.fetch.max.timeout.ms to configure DelayedRemoteFetch timeout (apache#14778)

    KIP-1018, part1, Introduce remote.fetch.max.timeout.ms to configure DelayedRemoteFetch timeout

    Reviewers: Luke Chen <showuon@gmail.com>

commit 7ddfa64
Author: Dongnuo Lyu <139248811+dongnuo123@users.noreply.github.com>
Date:   Wed Jun 5 02:08:38 2024 -0400

    MINOR: Adjust validateOffsetCommit/Fetch in ConsumerGroup to ensure compatibility with classic protocol members (apache#16145)

    During online migration, there could be ConsumerGroup that has members that uses the classic protocol. In the current implementation, `STALE_MEMBER_EPOCH` could be thrown in ConsumerGroup offset fetch/commit validation but it's not supported by the classic protocol. Thus this patch changed `ConsumerGroup#validateOffsetCommit` and `ConsumerGroup#validateOffsetFetch` to ensure compatibility.

    Reviewers: Jeff Kim <jeff.kim@confluent.io>, David Jacot <djacot@confluent.io>

commit 252c1ac
Author: Apoorv Mittal <apoorvmittal10@gmail.com>
Date:   Wed Jun 5 05:55:24 2024 +0100

    KAFKA-16740: Adding skeleton code for Share Fetch and Acknowledge RPC (KIP-932) (apache#16184)

    The PR adds skeleton code for Share Fetch and Acknowledge RPCs. The changes include:

    1. Defining RPCs in KafkaApis.scala
    2. Added new SharePartitionManager class which handles the RPCs handling
    3. Added SharePartition class which manages in-memory record states and for fetched data.

    Reviewers: David Jacot <djacot@confluent.io>, Andrew Schofield <aschofield@confluent.io>, Manikumar Reddy <manikumar.reddy@gmail.com>

commit b89999b
Author: PoAn Yang <payang@apache.org>
Date:   Wed Jun 5 08:02:52 2024 +0800

    KAFKA-16483: Remove preAppendErrors from createPutCacheCallback (apache#16105)

    The method createPutCacheCallback has a input argument preAppendErrors. It is used to keep the "error" happens before appending. However, it is always empty. Also, the pre-append error is handled before createPutCacheCallback by calling responseCallback. Hence, we can remove preAppendErrors.

    Signed-off-by: PoAn Yang <payang@apache.org>

    Reviewers: Luke Chen <showuon@gmail.com>

commit 01e9918
Author: Kuan-Po (Cooper) Tseng <brandboat@gmail.com>
Date:   Wed Jun 5 07:56:18 2024 +0800

    KAFKA-16814 KRaft broker cannot startup when `partition.metadata` is missing (apache#16165)

    When starting up kafka logManager, we'll check stray replicas to avoid some corner cases. But this check might cause broker unable to startup if partition.metadata is missing because when startup kafka, we load log from file, and the topicId of the log is coming from partition.metadata file. So, if partition.metadata is missing, the topicId will be None, and the LogManager#isStrayKraftReplica will fail with no topicID error.

    The partition.metadata missing could be some storage failure, or another possible path is unclean shutdown after topic is created in the replica, but before data is flushed into partition.metadata file. This is possible because we do the flush in async way here.

    When finding a log without topicID, we should treat it as a stray log and then delete it.

    Reviewers: Luke Chen <showuon@gmail.com>, Gaurav Narula <gaurav_narula2@apple.com>

commit d652f5c
Author: TingIāu "Ting" Kì <51072200+frankvicky@users.noreply.github.com>
Date:   Wed Jun 5 07:52:06 2024 +0800

    MINOR: Add topicIds and directoryIds to the return value of the toString method. (apache#16189)

    Add topicIds and directoryIds to the return value of the toString method.

    Reviewers: Luke Chen <showuon@gmail.com>

commit 7e0caad
Author: Igor Soarez <i@soarez.me>
Date:   Tue Jun 4 22:12:33 2024 +0100

    MINOR: Cleanup unused references in core (apache#16192)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit 9821aca
Author: PoAn Yang <payang@apache.org>
Date:   Wed Jun 5 05:09:04 2024 +0800

    MINOR: Upgrade gradle from 8.7 to 8.8 (apache#16190)

    Reviewers: Chia-Ping Tsai <chia7712@gmail.com>

commit 9ceed8f
Author: Colin P. McCabe <cmccabe@apache.org>
Date:   Tue Jun 4 14:04:59 2024 -0700

    KAFKA-16535: Implement AddVoter, RemoveVoter, UpdateVoter RPCs

    Implement the add voter, remove voter, and update voter RPCs for
    KIP-853. This is just adding the RPC handling; the current
    implementation in RaftManager just throws UnsupportedVersionException.

    Reviewers: Andrew Schofield <aschofield@confluent.io>, José Armando García Sancio <jsancio@apache.org>

commit 8b3c77c
Author: TingIāu "Ting" Kì <51072200+frankvicky@users.noreply.github.com>
Date:   Wed Jun 5 04:21:20 2024 +0800

    KAFKA-15305 The background thread should try to process the remaining task until the shutdown timer is expired. (apache#16156)

    Reviewers: Lianet Magrans <lianetmr@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>

commit cda2df5
Author: Kamal Chandraprakash <kchandraprakash@uber.com>
Date:   Wed Jun 5 00:41:30 2024 +0530

    KAFKA-16882 Migrate RemoteLogSegmentLifecycleTest to ClusterInstance infra (apache#16180)

    - Removed the RemoteLogSegmentLifecycleManager
    - Removed the TopicBasedRemoteLogMetadataManagerWrapper, RemoteLogMetadataCacheWrapper, TopicBasedRemoteLogMetadataManagerHarness and TopicBasedRemoteLogMetadataManagerWrapperWithHarness

    Reviewers: Kuan-Po (Cooper) Tseng <brandboat@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>

commit 2b47798
Author: Chris Egerton <chrise@aiven.io>
Date:   Tue Jun 4 21:04:34 2024 +0200

    MINOR: Fix return tag on Javadocs for consumer group-related Admin methods (apache#16197)

    Reviewers: Greg Harris <greg.harris@aiven.io>, Chia-Ping Tsai <chia7712@gmail.com>
TaiJuWu pushed a commit to TaiJuWu/kafka that referenced this pull request Jun 8, 2024
…module (apache#16198)

This patch moves the `PartitionAssignor` interface and all the related classes to a newly created `group-coordinator/api` module, following the pattern used by the storage and tools modules.

Reviewers: Ritika Reddy <rreddy@confluent.io>, Jeff Kim <jeff.kim@confluent.io>, Chia-Ping Tsai <chia7712@gmail.com>
gongxuanzhang pushed a commit to gongxuanzhang/kafka that referenced this pull request Jun 12, 2024
…module (apache#16198)

This patch moves the `PartitionAssignor` interface and all the related classes to a newly created `group-coordinator/api` module, following the pattern used by the storage and tools modules.

Reviewers: Ritika Reddy <rreddy@confluent.io>, Jeff Kim <jeff.kim@confluent.io>, Chia-Ping Tsai <chia7712@gmail.com>
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.

5 participants