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

abhijeetk88 · 2024-04-27T05:16:05Z

In this PR, I have 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.

Committer Checklist (excluded from commit message)

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

showuon

Had an early review, left some comments. Will check again after #15625 merged. Thanks.

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java

core/src/main/java/kafka/log/remote/RemoteLogManager.java

satishd · 2024-05-31T01:08:51Z

@abhijeetk88 Can you resolve the conflicts?

showuon

LGTM! Thanks.

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java

core/src/main/java/kafka/log/remote/RemoteLogManager.java

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java

satishd

Thanks @abhijeetk88 for the PR, overall LGTM. Left a minor comment and +1 on the comment regarding the refactoring of the simpler test condition.

satishd · 2024-06-03T04:19:23Z

core/src/main/java/kafka/log/remote/RemoteLogManager.java

+     * Returns the timeout for the RLM Tasks to wait for the quota to be available
+     */
+    Duration quotaTimeout() {
+        return Duration.ofSeconds(1);


Can you use the declared constant value?

I want to override this method to have a smaller timeout for tests. That is the reason I did not use a constant value.

kamalcph

Thanks for the update! Left one comment to address.

kamalcph · 2024-06-04T03:28:26Z

core/src/main/java/kafka/log/remote/RemoteLogManager.java

+                                    // If the thread gets interrupted while waiting, the InterruptedException is thrown
+                                    // back to the caller. It's important to note that the task being executed is already
+                                    // cancelled before the executing thread is interrupted. The caller is responsible
+                                    // for handling the exception gracefully by checking if the task is already cancelled.


Could you cover shutdown when the quota gets breached as unit test?

showuon · 2024-06-06T03:25:53Z

@abhijeetk88 , any update to this PR?

abhijeetk88 · 2024-06-06T09:13:31Z

Hi @showuon. I have responded to your comment here. Please take a look.

abhijeetk88 · 2024-06-08T22:37:24Z

@kamalcph I have addressed your comments, please take a look.

kamalcph

Overall LGTM, left one comment to address!

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java

kamalcph

LGTM, thanks for patch!

satishd · 2024-06-12T00:54:21Z

Thanks @abhijeetk88 for addressing the review comments. LGTM.

satishd · 2024-06-12T00:55:12Z

There are a few unrelated test failures, merging it to trunk and 3.8.

…e storage (#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 <[email protected]>, Luke Chen <[email protected]>, Kamal Chandraprakash<[email protected]>

…e 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 <[email protected]>, Luke Chen <[email protected]>, Kamal Chandraprakash<[email protected]>

commit 9368ef8 Author: Gantigmaa Selenge <[email protected]> Date: Wed Jun 12 16:04:24 2024 +0100 KAFKA-16865: Add IncludeTopicAuthorizedOperations option for DescribeTopicPartitionsRequest (apache#16136) Reviewers: Mickael Maison <[email protected]>, Chia-Ping Tsai <[email protected]>, Calvin Liu <[email protected]>, Andrew Schofield <[email protected]>, Apoorv Mittal <[email protected]> commit 46eb081 Author: gongxuanzhang <[email protected]> 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 <[email protected]> commit 79b9c44 Author: gongxuanzhang <[email protected]> Date: Wed Jun 12 22:19:47 2024 +0800 KAFKA-10787 Apply spotless to connect module (apache#16299) Reviewers: Chia-Ping Tsai <[email protected]> commit b5fb654 Author: Abhijeet Kumar <[email protected]> Date: Wed Jun 12 19:47:46 2024 +0530 KAFKA-15265: Dynamic broker configs for remote fetch/copy quotas (apache#16078) Reviewers: Kamal Chandraprakash<[email protected]>, Satish Duggana <[email protected]> commit faee6a4 Author: Dmitry Werner <[email protected]> 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 <[email protected]> commit 638844f Author: David Jacot <[email protected]> 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 <[email protected]>, Justine Olshan <[email protected]> commit 39ffdea Author: Bruno Cadonna <[email protected]> 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 <[email protected]>, Matthias J. Sax <[email protected]> commit 0782232 Author: Antoine Pourchet <[email protected]> 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 <[email protected]> commit 226ac5e Author: Murali Basani <[email protected]> Date: Wed Jun 12 05:38:50 2024 +0200 KAFKA-16922 Adding unit tests for NewTopic (apache#16255) Reviewers: Chia-Ping Tsai <[email protected]> commit 23fe71d Author: Abhijeet Kumar <[email protected]> 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 <[email protected]>, Luke Chen <[email protected]>, Kamal Chandraprakash<[email protected]> commit 2fa2c72 Author: Chris Egerton <[email protected]> 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 <[email protected]>

abhijeetk88 marked this pull request as draft April 27, 2024 05:19

abhijeetk88 force-pushed the KAFKA-15434_copy_quota branch 2 times, most recently from 3593def to 645ab37 Compare May 24, 2024 08:42

abhijeetk88 changed the title ~~[WIP] Kafka 15434: Integrate RLMQuotaManager for throttling copies to remote storage~~ Kafka 15434: Integrate RLMQuotaManager for throttling copies to remote storage May 24, 2024

abhijeetk88 changed the title ~~Kafka 15434: Integrate RLMQuotaManager for throttling copies to remote storage~~ Kafka-15625: Integrate RLMQuotaManager for throttling copies to remote storage May 24, 2024

abhijeetk88 marked this pull request as ready for review May 24, 2024 08:51

abhijeetk88 changed the title ~~Kafka-15625: Integrate RLMQuotaManager for throttling copies to remote storage~~ Kafka-15265: Integrate RLMQuotaManager for throttling copies to remote storage May 24, 2024

abhijeetk88 changed the title ~~Kafka-15265: Integrate RLMQuotaManager for throttling copies to remote storage~~ KAFKA-15265: Integrate RLMQuotaManager for throttling copies to remote storage May 24, 2024

abhijeetk88 force-pushed the KAFKA-15434_copy_quota branch 3 times, most recently from b56b448 to 22a7ae5 Compare May 26, 2024 12:59

satishd requested review from kamalcph, satishd, junrao and showuon May 27, 2024 04:45

satishd added the tiered-storage Related to the Tiered Storage feature label May 27, 2024

showuon reviewed May 27, 2024

View reviewed changes

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java Outdated Show resolved Hide resolved

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java Outdated Show resolved Hide resolved

kamalcph reviewed May 28, 2024

View reviewed changes

core/src/main/java/kafka/log/remote/RemoteLogManager.java Show resolved Hide resolved

core/src/main/java/kafka/log/remote/RemoteLogManager.java Show resolved Hide resolved

abhijeetk88 force-pushed the KAFKA-15434_copy_quota branch 2 times, most recently from 52c34f6 to aa32909 Compare May 30, 2024 18:34

abhijeetk88 force-pushed the KAFKA-15434_copy_quota branch from aa32909 to 68412eb Compare May 31, 2024 04:06

showuon approved these changes May 31, 2024

View reviewed changes

showuon reviewed May 31, 2024

View reviewed changes

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java Outdated Show resolved Hide resolved

kamalcph reviewed May 31, 2024

View reviewed changes

core/src/main/java/kafka/log/remote/RemoteLogManager.java Outdated Show resolved Hide resolved

kamalcph reviewed May 31, 2024

View reviewed changes

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java Outdated Show resolved Hide resolved

kamalcph reviewed May 31, 2024

View reviewed changes

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java Outdated Show resolved Hide resolved

abhijeetk88 commented May 31, 2024

View reviewed changes

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java Outdated Show resolved Hide resolved

satishd reviewed Jun 3, 2024

View reviewed changes

abhijeetk88 force-pushed the KAFKA-15434_copy_quota branch from 68412eb to 5440667 Compare June 3, 2024 08:58

kamalcph reviewed Jun 4, 2024

View reviewed changes

Integrate RLMQuotaManager for throttling copies to remote storage

43e34d5

abhijeetk88 force-pushed the KAFKA-15434_copy_quota branch from 5440667 to 5c1ef3c Compare June 8, 2024 22:35

abhijeetk88 force-pushed the KAFKA-15434_copy_quota branch from 5c1ef3c to 154ae9a Compare June 8, 2024 22:50

abhijeetk88 mentioned this pull request Jun 9, 2024

KAFKA-17069: Remote copy throttle metrics #16086

Merged

3 tasks

kamalcph reviewed Jun 9, 2024

View reviewed changes

core/src/test/java/kafka/log/remote/RemoteLogManagerTest.java Outdated Show resolved Hide resolved

Address review comments

22016f2

abhijeetk88 force-pushed the KAFKA-15434_copy_quota branch from 154ae9a to 22016f2 Compare June 10, 2024 10:53

kamalcph approved these changes Jun 11, 2024

View reviewed changes

satishd merged commit 23fe71d into apache:trunk Jun 12, 2024
1 check failed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

abhijeetk88 commented Apr 27, 2024 •

edited

Loading

showuon left a comment •

edited

Loading

satishd commented May 31, 2024

showuon left a comment

satishd left a comment

satishd Jun 3, 2024

abhijeetk88 Jun 3, 2024

kamalcph left a comment

kamalcph Jun 4, 2024

abhijeetk88 Jun 6, 2024

abhijeetk88 Jun 8, 2024

showuon commented Jun 6, 2024

abhijeetk88 commented Jun 6, 2024

abhijeetk88 commented Jun 8, 2024

kamalcph left a comment

kamalcph left a comment

satishd commented Jun 12, 2024

satishd commented Jun 12, 2024

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

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

Conversation

abhijeetk88 commented Apr 27, 2024 • edited Loading

Committer Checklist (excluded from commit message)

showuon left a comment • edited Loading

Choose a reason for hiding this comment

satishd commented May 31, 2024

showuon left a comment

Choose a reason for hiding this comment

satishd left a comment

Choose a reason for hiding this comment

satishd Jun 3, 2024

Choose a reason for hiding this comment

abhijeetk88 Jun 3, 2024

Choose a reason for hiding this comment

kamalcph left a comment

Choose a reason for hiding this comment

kamalcph Jun 4, 2024

Choose a reason for hiding this comment

abhijeetk88 Jun 6, 2024

Choose a reason for hiding this comment

abhijeetk88 Jun 8, 2024

Choose a reason for hiding this comment

showuon commented Jun 6, 2024

abhijeetk88 commented Jun 6, 2024

abhijeetk88 commented Jun 8, 2024

kamalcph left a comment

Choose a reason for hiding this comment

kamalcph left a comment

Choose a reason for hiding this comment

satishd commented Jun 12, 2024

satishd commented Jun 12, 2024

abhijeetk88 commented Apr 27, 2024 •

edited

Loading

showuon left a comment •

edited

Loading