[Fork]KAFKA-19167: Move RemoteLogManagerConfig out of KafkaConfig by apalan60 · Pull Request #2 · apalan60/kafka

apalan60 · 2025-07-15T10:08:32Z

PR Type

Enhancement

Description

Refactor RemoteLogManagerConfig to use singleton pattern
Replace direct constructor usage with factory method
Update all references across codebase to use new pattern
Add dynamic configuration update capability

Changes diagram

flowchart LR
  A["KafkaConfig"] --> B["RemoteLogManagerConfig.of()"]
  B --> C["Singleton Instance"]
  C --> D["Dynamic Updates"]
  E["Constructor Usage"] -.-> F["Factory Method"]
  F --> C

Changes walkthrough 📝

Relevant files

Enhancement

10 files

RemoteLogManagerConfig.java `Convert to singleton with factory method`	+58/-41
KafkaApisBuilder.java `Update to use factory method`	+2/-1
ReplicaManagerBuilder.java `Update to use factory method`	+2/-1
LogManager.scala `Update to use factory method`	+3/-2
BrokerServer.scala `Update to use factory method`	+6/-5
ConfigHandler.scala `Update to use factory method`	+2/-1
ControllerConfigurationValidator.scala `Update to use factory method`	+2/-1
DynamicBrokerConfig.scala `Update to use factory method`	+11/-13
KafkaConfig.scala `Remove direct RemoteLogManagerConfig field`	+2/-4
ReplicaManager.scala `Update to use factory method`	+5/-5

Tests

9 files

RemoteLogManagerConfigTest.java `Add singleton behavior tests`	+20/-1
RemoteLogManagerTest.java `Update test to use factory method`	+1/-1
RemoteLogOffsetReaderTest.java `Update test to use factory method`	+1/-1
LogConfigTest.scala `Update tests to use factory method`	+14/-14
LogLoaderTest.scala `Update test to use factory method`	+2/-1
UnifiedLogTest.scala `Update tests to use factory method`	+2/-2
DynamicBrokerConfigTest.scala `Update tests to use factory method`	+37/-35
KafkaConfigTest.scala `Update test to use factory method`	+2/-2
ReplicaManagerTest.scala `Update tests to use factory method`	+5/-5

Need help?
Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
Check out the documentation for more information.

gemini-code-assist

Summary of Changes

Hello @apalan60, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request refactors the RemoteLogManagerConfig class by extracting its instantiation and management logic from KafkaConfig. The goal is to improve modularity and enable more flexible, dynamic configuration management for remote log features by introducing a singleton pattern for RemoteLogManagerConfig.

Highlights

Refactoring RemoteLogManagerConfig: The RemoteLogManagerConfig class has been refactored to be a singleton, accessible via a new static factory method RemoteLogManagerConfig.of(config). It now internally manages its AbstractConfig reference, allowing for dynamic updates to its configuration.
Decoupling from KafkaConfig: The direct dependency of KafkaConfig on RemoteLogManagerConfig has been removed. KafkaConfig no longer instantiates or holds a RemoteLogManagerConfig object, simplifying its structure.
Widespread Usage Update: All existing code paths that previously accessed config.remoteLogManagerConfig have been updated to use the new RemoteLogManagerConfig.of(config) static factory method. This change propagates across various server components, log management, and configuration validation logic.
Enhanced Dynamic Configuration Support: The new design for RemoteLogManagerConfig explicitly supports dynamic updates to its underlying configuration, which is validated by new dedicated test cases.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

qodo-code-review · 2025-07-15T10:09:20Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Thread Safety The singleton pattern implementation uses AtomicReference but the updateAndGet operation with complex logic may not be atomic. The comparison `existing.configRef.get() != config` and subsequent update could lead to race conditions where multiple threads create different instances or miss updates. return INSTANCE_REF.updateAndGet(existing -> { if (existing == null) { return new RemoteLogManagerConfig(config); } if (existing.configRef.get() != config) { existing.update(config); } return existing; }); } Singleton Violation The singleton pattern can be violated when different AbstractConfig instances are passed to `of()` method. The current implementation may create multiple instances or fail to update properly when configs differ, breaking the singleton guarantee. public static RemoteLogManagerConfig of(final AbstractConfig config) { return INSTANCE_REF.updateAndGet(existing -> { if (existing == null) { return new RemoteLogManagerConfig(config); } if (existing.configRef.get() != config) { existing.update(config); } return existing; }); } void update(final AbstractConfig newConfig) { this.configRef.set(newConfig); } Syntax Error Missing dot operator between method calls in multiple test methods. The code shows `RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled` which should be `RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled`. LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) }

qodo-code-review · 2025-07-15T10:10:51Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	✅ ~~Fix singleton race condition~~ Suggestion Impact: The commit implemented a different but valid solution to the race condition. Instead of using synchronized method as suggested, it used double-checked locking pattern with volatile field and synchronized block, which also fixes the race condition. code diff: + private static volatile RemoteLogManagerConfig instance; - private final AtomicReference<AbstractConfig> configRef; - - private RemoteLogManagerConfig(final AbstractConfig config) { - this.configRef = new AtomicReference<>(config); + private final AbstractConfig config; + + private RemoteLogManagerConfig(AbstractConfig config) { + this.config = config; } public static RemoteLogManagerConfig of(final AbstractConfig config) { - return INSTANCE_REF.updateAndGet(existing -> { - if (existing == null) { - return new RemoteLogManagerConfig(config); + if (instance == null) { + synchronized (RemoteLogManagerConfig.class) { + if (instance == null) { + instance = new RemoteLogManagerConfig(config); + } } - if (existing.configRef.get() != config) { - existing.update(config); - } - return existing; - }); - } - - void update(final AbstractConfig newConfig) { - this.configRef.set(newConfig); - } + } + return instance; + } The singleton pattern implementation has a race condition. Multiple threads could create different instances if they call `of()` simultaneously with different configs. Use proper synchronization or consider if singleton pattern is necessary for this use case. storage/src/main/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfig.java [45-55] -public static RemoteLogManagerConfig of(final AbstractConfig config) { - return INSTANCE_REF.updateAndGet(existing -> { - if (existing == null) { - return new RemoteLogManagerConfig(config); - } - if (existing.configRef.get() != config) { - existing.update(config); - } - return existing; - }); +public static synchronized RemoteLogManagerConfig of(final AbstractConfig config) { + RemoteLogManagerConfig existing = INSTANCE_REF.get(); + if (existing == null) { + existing = new RemoteLogManagerConfig(config); + INSTANCE_REF.set(existing); + } else if (existing.configRef.get() != config) { + existing.update(config); + } + return existing; } `[Suggestion processed]` Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a race condition due to a non-side-effect-free lambda in `updateAndGet`, which is a critical bug in a concurrent environment.	High
Possible issue	✅ ~~Fix missing dot operator~~ Suggestion Impact: The suggestion was directly implemented - all instances of the missing dot operator between RemoteLogManagerConfig.of(kafkaConfig) and isRemoteStorageSystemEnabled were fixed throughout the test file code diff: - LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) + LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) } logProps.put(TopicConfig.CLEANUP_POLICY_CONFIG, TopicConfig.CLEANUP_POLICY_DELETE) logProps.put(TopicConfig.REMOTE_LOG_STORAGE_ENABLE_CONFIG, "true") @@ -311,10 +311,10 @@ val logProps = new Properties() logProps.put(TopicConfig.REMOTE_LOG_STORAGE_ENABLE_CONFIG, "true") if (sysRemoteStorageEnabled) { - LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) + LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) } else { val message = assertThrows(classOf[ConfigException], - () => LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled)) + () => LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled)) assertTrue(message.getMessage.contains("Tiered Storage functionality is disabled in the broker")) } } @@ -331,7 +331,7 @@ if (wasRemoteStorageEnabled) { val message = assertThrows(classOf[InvalidConfigurationException], () => LogConfig.validate(util.Map.of(TopicConfig.REMOTE_LOG_STORAGE_ENABLE_CONFIG, "true"), - logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled)) + logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled)) assertTrue(message.getMessage.contains("It is invalid to disable remote storage without deleting remote data. " + "If you want to keep the remote data and turn to read only, please set `remote.storage.enable=true,remote.log.copy.disable=true`. " + "If you want to disable remote storage and delete all remote data, please set `remote.storage.enable=false,remote.log.delete.on.disable=true`.")) @@ -340,11 +340,11 @@ // It should be able to disable the remote log storage when delete on disable is set to true logProps.put(TopicConfig.REMOTE_LOG_DELETE_ON_DISABLE_CONFIG, "true") LogConfig.validate(util.Map.of(TopicConfig.REMOTE_LOG_STORAGE_ENABLE_CONFIG, "true"), - logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) + logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) } else { - LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) + LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) LogConfig.validate(util.Map.of(TopicConfig.REMOTE_LOG_STORAGE_ENABLE_CONFIG, "false"), logProps, - kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) + kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) } } @@ -364,11 +364,11 @@ if (sysRemoteStorageEnabled) { val message = assertThrows(classOf[ConfigException], () => LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, - RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled)) + RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled)) assertTrue(message.getMessage.contains(TopicConfig.LOCAL_LOG_RETENTION_MS_CONFIG)) } else { LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, - RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) + RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) } } @@ -388,11 +388,11 @@ if (sysRemoteStorageEnabled) { val message = assertThrows(classOf[ConfigException], () => LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, - RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled)) + RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled)) assertTrue(message.getMessage.contains(TopicConfig.LOCAL_LOG_RETENTION_BYTES_CONFIG)) } else { LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, - RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) + RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) } } @@ -407,10 +407,10 @@ if (sysRemoteStorageEnabled) { val message = assertThrows(classOf[ConfigException], - () => LogConfig.validateBrokerLogConfigValues(kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled)) + () => LogConfig.validateBrokerLogConfigValues(kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled)) assertTrue(message.getMessage.contains(TopicConfig.LOCAL_LOG_RETENTION_BYTES_CONFIG)) } else { - LogConfig.validateBrokerLogConfigValues(kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) + LogConfig.validateBrokerLogConfigValues(kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) Missing dot operator between method call and property access. This will cause a compilation error. core/src/test/scala/unit/kafka/log/LogConfigTest.scala [287] -LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled) +LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled) `[Suggestion processed]` Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a missing dot or space in the Scala syntax, which is a compilation error that needs to be fixed for the tests to run.	Medium
General	Fix singleton test assertion The test uses `assertEquals` to compare object references, but `RemoteLogManagerConfig` doesn't override `equals()` method. This test will fail because it compares object identity rather than logical equality. Use `assertSame` for reference equality or implement proper `equals()` method. storage/src/test/java/org/apache/kafka/server/log/remote/storage/RemoteLogManagerConfigTest.java [94-102] @Test void testSingletonBehavior() { var config = new RLMTestConfig(Map.of(RemoteLogManagerConfig.REMOTE_LOG_INDEX_FILE_CACHE_TOTAL_SIZE_BYTES_PROP, 1024L)); var rlmConfig1 = RemoteLogManagerConfig.of(config); var rlmConfig2 = RemoteLogManagerConfig.of(config); - assertEquals(rlmConfig1, rlmConfig2); + assertSame(rlmConfig1, rlmConfig2); assertEquals(1024L, rlmConfig1.remoteLogIndexFileCacheTotalSizeBytes()); } Apply / Chat Suggestion importance[1-10]: 4 __ Why: While the suggestion's claim that the test will fail is incorrect because the default `equals` checks reference identity, using `assertSame` is better practice and more clearly expresses the intent of the test.	Low
Update

gemini-code-assist

Code Review

This pull request refactors RemoteLogManagerConfig to use a singleton pattern with a factory method, which is a good improvement for managing this configuration centrally. The changes are applied consistently across the codebase.

I've found a few issues:

A critical compilation error in LogConfigTest.scala due to a typo.
An opportunity to improve the implementation of the singleton factory method in RemoteLogManagerConfig.java to make it more readable and avoid side effects in an atomic operation.
A section in DynamicBrokerConfig.scala with repetitive code that could be cleaned up for better readability and minor performance improvement.

Overall, the changes are in the right direction. Addressing these points will improve the quality and robustness of the code.

gemini-code-assist · 2025-07-15T10:11:39Z

    val logProps = new Properties()
    def validateCleanupPolicy(): Unit = {
-      LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, kafkaConfig.remoteLogManagerConfig.isRemoteStorageSystemEnabled)
+      LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled)


There seems to be a typo here. A dot . is missing between the method call RemoteLogManagerConfig.of(kafkaConfig) and the property access isRemoteStorageSystemEnabled. This will cause a compilation error.

This issue is repeated on lines 314, 317, 334, 343, 345, 347, 367, 371, 391, 395, 410, and 413. Please fix all occurrences.

Suggested change

LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled)

LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled)

gemini-code-assist · 2025-07-15T10:11:39Z

+      if (RemoteLogManagerConfig.of(newConfig).remoteLogManagerCopierThreadPoolSize() != RemoteLogManagerConfig.of(oldConfig).remoteLogManagerCopierThreadPoolSize())
+        remoteLogManager.resizeCopierThreadPool(RemoteLogManagerConfig.of(newConfig).remoteLogManagerCopierThreadPoolSize())

-      if (newRLMConfig.remoteLogManagerExpirationThreadPoolSize() != oldRLMConfig.remoteLogManagerExpirationThreadPoolSize())
-        remoteLogManager.resizeExpirationThreadPool(newRLMConfig.remoteLogManagerExpirationThreadPoolSize())
+      if (RemoteLogManagerConfig.of(newConfig).remoteLogManagerExpirationThreadPoolSize() != RemoteLogManagerConfig.of(oldConfig).remoteLogManagerExpirationThreadPoolSize())
+        remoteLogManager.resizeExpirationThreadPool(RemoteLogManagerConfig.of(newConfig).remoteLogManagerExpirationThreadPoolSize())

-      if (newRLMConfig.remoteLogManagerFollowerThreadPoolSize() != oldRLMConfig.remoteLogManagerFollowerThreadPoolSize())
-        remoteLogManager.resizeFollowerThreadPool(newRLMConfig.remoteLogManagerFollowerThreadPoolSize())
+      if (RemoteLogManagerConfig.of(newConfig).remoteLogManagerFollowerThreadPoolSize() != RemoteLogManagerConfig.of(oldConfig).remoteLogManagerFollowerThreadPoolSize())
+        remoteLogManager.resizeFollowerThreadPool(RemoteLogManagerConfig.of(newConfig).remoteLogManagerFollowerThreadPoolSize())

-      if (newRLMConfig.remoteLogReaderThreads() != oldRLMConfig.remoteLogReaderThreads())
-        remoteLogManager.resizeReaderThreadPool(newRLMConfig.remoteLogReaderThreads())
+      if (RemoteLogManagerConfig.of(newConfig).remoteLogReaderThreads() != RemoteLogManagerConfig.of(oldConfig).remoteLogReaderThreads())
+        remoteLogManager.resizeReaderThreadPool(RemoteLogManagerConfig.of(newConfig).remoteLogReaderThreads())


The repeated calls to RemoteLogManagerConfig.of(newConfig) and RemoteLogManagerConfig.of(oldConfig) make the code less readable and slightly less efficient. It would be cleaner to store the results of these calls in local variables, similar to the previous implementation. This improves readability and avoids redundant method calls.

val newRLMConfig = RemoteLogManagerConfig.of(newConfig) val oldRLMConfig = RemoteLogManagerConfig.of(oldConfig) if (newRLMConfig.remoteLogManagerCopierThreadPoolSize() != oldRLMConfig.remoteLogManagerCopierThreadPoolSize()) remoteLogManager.resizeCopierThreadPool(newRLMConfig.remoteLogManagerCopierThreadPoolSize()) if (newRLMConfig.remoteLogManagerExpirationThreadPoolSize() != oldRLMConfig.remoteLogManagerExpirationThreadPoolSize()) remoteLogManager.resizeExpirationThreadPool(newRLMConfig.remoteLogManagerExpirationThreadPoolSize()) if (newRLMConfig.remoteLogManagerFollowerThreadPoolSize() != oldRLMConfig.remoteLogManagerFollowerThreadPoolSize()) remoteLogManager.resizeFollowerThreadPool(newRLMConfig.remoteLogManagerFollowerThreadPoolSize()) if (newRLMConfig.remoteLogReaderThreads() != oldRLMConfig.remoteLogReaderThreads()) remoteLogManager.resizeReaderThreadPool(newRLMConfig.remoteLogReaderThreads())

gemini-code-assist · 2025-07-15T10:11:39Z

+    public static RemoteLogManagerConfig of(final AbstractConfig config) {
+        return INSTANCE_REF.updateAndGet(existing -> {
+            if (existing == null) {
+                return new RemoteLogManagerConfig(config);
+            }
+            if (existing.configRef.get() != config) {
+                existing.update(config);
+            }
+            return existing;
+        });
+    }


The implementation of the of method using updateAndGet with a lambda that has side effects (existing.update(config)) can be hard to reason about and is generally discouraged. The function passed to updateAndGet may be re-applied on contention, which could lead to unexpected behavior with side effects.

A simpler and clearer approach would be to separate the instance creation from the configuration update. You can use updateAndGet with a side-effect-free lambda for atomic initialization, and then call the update method. This makes the logic more straightforward.

public static RemoteLogManagerConfig of(final AbstractConfig config) { // Atomically create the instance if it doesn't exist. RemoteLogManagerConfig instance = INSTANCE_REF.updateAndGet(existing -> existing == null ? new RemoteLogManagerConfig(config) : existing); // Update the config. This is thread-safe as configRef is an AtomicReference. // The last writer wins, which is acceptable for dynamic config updates. instance.update(config); return instance; }

apalan60 · 2025-07-15T10:13:52Z

/review
auto_approve
--pr_reviewer.extra_instructions="
In the possible issues section, emphasize the following:

Is the code logic efficient?

"
--pr_reviewer.inline_code_comments=true
-i
--pr_reviewer.require_score_review=true
--pr_reviewer.require_can_be_split_review=true
--pr_reviewer.require_soc2_ticket=true
--pr_reviewer.num_code_suggestions="5"

qodo-code-review · 2025-07-15T10:13:57Z

Manual-approval option for PR-Agent is disabled. You can enable it via a configuration file

apalan60 added 2 commits July 15, 2025 16:58

KAFKA-19167: Move RemoteLogManagerConfig out of KafkaConfig

197609b

Merge remote-tracking branch 'upstream/trunk' into KAFKA-19167-2

a4a4c90

gemini-code-assist Bot reviewed Jul 15, 2025

View reviewed changes

qodo-code-review Bot added the Review effort 4/5 label Jul 15, 2025

gemini-code-assist Bot reviewed Jul 15, 2025

View reviewed changes

apalan60 force-pushed the KAFKA-19167-2 branch from 144396b to 04c6c44 Compare July 15, 2025 17:00

apalan60 added 2 commits July 16, 2025 01:02

Address comments based on atomic version

bb1eb65

WIP implement factory

7c95fac

apalan60 force-pushed the KAFKA-19167-2 branch from 04c6c44 to 7c95fac Compare July 15, 2025 17:03

apalan60 added 4 commits July 16, 2025 01:08

todo

3ed5498

Supplier

c701003

Add root cause explaination

037fdd4

TODO: Integration test

9854302

apalan60 force-pushed the KAFKA-19167-2 branch from c21a25e to 9854302 Compare July 17, 2025 10:22

apalan60 force-pushed the trunk branch from 8a7d0ac to 2ba30cc Compare September 2, 2025 09:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fork]KAFKA-19167: Move RemoteLogManagerConfig out of KafkaConfig#2

[Fork]KAFKA-19167: Move RemoteLogManagerConfig out of KafkaConfig#2
apalan60 wants to merge 8 commits into
trunkfrom
KAFKA-19167-2

apalan60 commented Jul 15, 2025 •

edited by qodo-code-review Bot

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

qodo-code-review Bot commented Jul 15, 2025

Uh oh!

qodo-code-review Bot commented Jul 15, 2025 •

edited

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jul 15, 2025

Uh oh!

gemini-code-assist Bot Jul 15, 2025

Uh oh!

gemini-code-assist Bot Jul 15, 2025

Uh oh!

apalan60 commented Jul 15, 2025

Uh oh!

qodo-code-review Bot commented Jul 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig)isRemoteStorageSystemEnabled)
	LogConfig.validate(util.Map.of, logProps, kafkaConfig.extractLogConfigMap, RemoteLogManagerConfig.of(kafkaConfig).isRemoteStorageSystemEnabled)

Conversation

apalan60 commented Jul 15, 2025 • edited by qodo-code-review Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Type

Description

Changes diagram

Changes walkthrough 📝

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

qodo-code-review Bot commented Jul 15, 2025

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review Bot commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

apalan60 commented Jul 15, 2025

Uh oh!

qodo-code-review Bot commented Jul 15, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

apalan60 commented Jul 15, 2025 •

edited by qodo-code-review Bot

Loading

qodo-code-review Bot commented Jul 15, 2025 •

edited

Loading