Skip to content

KAFKA-16770; [2/2] Coalesce records into bigger batches#16215

Merged
dajac merged 13 commits intoapache:trunkfrom
dajac:KAFKA-16770-2
Jun 12, 2024
Merged

KAFKA-16770; [2/2] Coalesce records into bigger batches#16215
dajac merged 13 commits intoapache:trunkfrom
dajac:KAFKA-16770-2

Conversation

@dajac
Copy link
Copy Markdown
Member

@dajac dajac commented Jun 5, 2024

This patch is the continuation of #15964. It introduces the records coalescing to the CoordinatorRuntime. It also introduces a new configuration group.coordinator.append.linger.ms which allows administrators to chose the linger time or disable it with zero. The new configuration defaults to 10ms.

I am still working on the tests so I keep the PR as a draft for now. However, all the existing tests that uses no linger are all passing. I will push the new tests asap.

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 5, 2024
@dajac dajac changed the title KAFKA-16770; [1/2] Coalesce records into bigger batches KAFKA-16770; [2/2] Coalesce records into bigger batches Jun 5, 2024
@dajac dajac requested a review from jolshan June 5, 2024 15:18
jeffkbkim
jeffkbkim reviewed Jun 5, 2024
View reviewed changes
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java Outdated
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java Outdated
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java Outdated
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java Outdated
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java Outdated
@dajac dajac marked this pull request as ready for review June 6, 2024 14:03
@dajac
Copy link
Copy Markdown
Member Author

dajac commented Jun 6, 2024

@jeffkbkim @jolshan Thanks for your comments. I addressed them. I pushed the tests and a few fixes too.

@dajac dajac requested review from jeffkbkim and jolshan June 6, 2024 14:55
jeffkbkim
jeffkbkim reviewed Jun 6, 2024
View reviewed changes
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java Outdated
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
dajac
dajac commented Jun 7, 2024
View reviewed changes
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java Outdated
jeffkbkim
jeffkbkim reviewed Jun 7, 2024
View reviewed changes
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...coordinator/src/main/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntime.java
Comment thread ...dinator/src/test/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntimeTest.java
Comment thread ...dinator/src/test/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntimeTest.java
Comment thread ...dinator/src/test/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntimeTest.java
Comment thread ...dinator/src/test/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntimeTest.java Outdated
Comment thread ...dinator/src/test/java/org/apache/kafka/coordinator/group/runtime/CoordinatorRuntimeTest.java
@jolshan
Copy link
Copy Markdown
Member

jolshan commented Jun 10, 2024

Looks pretty good. I was wondering -- do we want to test the behavior of when we commit before a batch is flushed? (Is this possible)

@dajac
Copy link
Copy Markdown
Member Author

dajac commented Jun 10, 2024

@jolshan Committing offsets above the last written offset is not allowed so it is probably not necessary. We already have tests to validate the logic to commit offsets up to the last written offset.

@jolshan
Copy link
Copy Markdown
Member

jolshan commented Jun 10, 2024

@dajac makes sense. I guess I just didn't know if there was a case where we had one in a batch so it wasn't written to the log yet, but we tried to commit offsets.

Thanks for clarifying.

@dajac dajac requested a review from jolshan June 11, 2024 07:58
jolshan
jolshan approved these changes Jun 11, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@jolshan jolshan left a comment

Choose a reason for hiding this comment

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

thanks David!

@dajac dajac merged commit 638844f into apache:trunk Jun 12, 2024
@dajac dajac deleted the KAFKA-16770-2 branch June 12, 2024 06:29
dajac added a commit that referenced this pull request Jun 12, 2024
@dajac
KAFKA-16770; [2/2] Coalesce records into bigger batches (#16215)
6016b15 
This patch is the continuation of #15964. It introduces the records coalescing to the CoordinatorRuntime. It also introduces a new configuration `group.coordinator.append.linger.ms` which allows administrators to chose the linger time or disable it with zero. The new configuration defaults to 10ms.

Reviewers: Jeff Kim <jeff.kim@confluent.io>, Justine Olshan <jolshan@confluent.io>
@dajac
Copy link
Copy Markdown
Member Author

dajac commented Jun 12, 2024

Merged to trunk and to 3.8.

gongxuanzhang pushed a commit to gongxuanzhang/kafka that referenced this pull request Jun 12, 2024
@dajac @gongxuanzhang
KAFKA-16770; [2/2] Coalesce records into bigger batches (apache#16215)
ef3158c 
This patch is the continuation of apache#15964. It introduces the records coalescing to the CoordinatorRuntime. It also introduces a new configuration `group.coordinator.append.linger.ms` which allows administrators to chose the linger time or disable it with zero. The new configuration defaults to 10ms.

Reviewers: Jeff Kim <jeff.kim@confluent.io>, Justine Olshan <jolshan@confluent.io>
apourchet added a commit to apourchet/kafka that referenced this pull request Jun 12, 2024
@apourchet
Squashed commit of the following:
4ba836d 
commit 9368ef8
Author: Gantigmaa Selenge <39860586+tinaselenge@users.noreply.github.com>
Date:   Wed Jun 12 16:04:24 2024 +0100

    KAFKA-16865: Add IncludeTopicAuthorizedOperations option for DescribeTopicPartitionsRequest (apache#16136)

    Reviewers: Mickael Maison <mickael.maison@gmail.com>, Chia-Ping Tsai <chia7712@gmail.com>, Calvin Liu <caliu@confluent.io>, Andrew Schofield <andrew_schofield@live.com>, Apoorv Mittal <amittal@confluent.io>

commit 46eb081
Author: gongxuanzhang <gongxuanzhang@foxmail.com>
Date:   Wed Jun 12 22:23:39 2024 +0800

    KAFKA-10787 Apply spotless to log4j-appender, trogdor, jmh-benchmarks, examples, shell and generator (apache#16296)

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

commit 79b9c44
Author: gongxuanzhang <gongxuanzhang@foxmail.com>
Date:   Wed Jun 12 22:19:47 2024 +0800

    KAFKA-10787 Apply spotless to connect module (apache#16299)

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

commit b5fb654
Author: Abhijeet Kumar <abhijeet.cse.kgp@gmail.com>
Date:   Wed Jun 12 19:47:46 2024 +0530

    KAFKA-15265: Dynamic broker configs for remote fetch/copy quotas (apache#16078)

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

commit faee6a4
Author: Dmitry Werner <grimekillah@gmail.com>
Date:   Wed Jun 12 15:44:11 2024 +0500

    MINOR: Use predetermined dir IDs in ReplicationQuotasTest

    Use predetermined directory IDs instead of Uuid.randomUuid() in ReplicationQuotasTest.

    Reviewers: Igor Soarez <soarez@apple.com>

commit 638844f
Author: David Jacot <djacot@confluent.io>
Date:   Wed Jun 12 08:29:50 2024 +0200

    KAFKA-16770; [2/2] Coalesce records into bigger batches (apache#16215)

    This patch is the continuation of apache#15964. It introduces the records coalescing to the CoordinatorRuntime. It also introduces a new configuration `group.coordinator.append.linger.ms` which allows administrators to chose the linger time or disable it with zero. The new configuration defaults to 10ms.

    Reviewers: Jeff Kim <jeff.kim@confluent.io>, Justine Olshan <jolshan@confluent.io>

commit 39ffdea
Author: Bruno Cadonna <cadonna@apache.org>
Date:   Wed Jun 12 07:51:38 2024 +0200

    KAFKA-10199: Enable state updater by default (apache#16107)

    We have already enabled the state updater by default once.
    However, we ran into issues that forced us to disable it again.
    We think that we fixed those issues. So we want to enable the
    state updater again by default.

    Reviewers: Lucas Brutschy <lbrutschy@confluent.io>, Matthias J. Sax <matthias@confluent.io>

commit 0782232
Author: Antoine Pourchet <antoine@responsive.dev>
Date:   Tue Jun 11 22:31:43 2024 -0600

    KAFKA-15045: (KIP-924 pt. 22) Add RackAwareOptimizationParams and other minor TaskAssignmentUtils changes (apache#16294)

    We now provide a way to more easily customize the rack aware
    optimizations that we provide by way of a configuration class called
    RackAwareOptimizationParams.

    We also simplified the APIs for the optimizeXYZ utility functions since
    they were mutating the inputs anyway.

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

commit 226ac5e
Author: Murali Basani <muralidhar.basani@aiven.io>
Date:   Wed Jun 12 05:38:50 2024 +0200

    KAFKA-16922 Adding unit tests for NewTopic  (apache#16255)

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

commit 23fe71d
Author: Abhijeet Kumar <abhijeet.cse.kgp@gmail.com>
Date:   Wed Jun 12 06:27:02 2024 +0530

    KAFKA-15265: Integrate RLMQuotaManager for throttling copies to remote storage (apache#15820)

    - Added the integration of the quota manager to throttle copy requests to the remote storage. Reference KIP-956
    - Added unit-tests for the copy throttling logic.

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

commit 2fa2c72
Author: Chris Egerton <chrise@aiven.io>
Date:   Tue Jun 11 23:15:07 2024 +0200

    MINOR: Wait for embedded clusters to start before using them in Connect OffsetsApiIntegrationTest (apache#16286)

    Reviewers: Greg Harris <greg.harris@aiven.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeffkbkim jeffkbkim jeffkbkim left review comments

@jolshan jolshan jolshan approved these changes

Assignees

No one assigned

Labels

KIP-848 The Next Generation of the Consumer Rebalance Protocol

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dajac @jolshan @jeffkbkim