unknowntpo commented on code in PR #22409:
URL: https://github.com/apache/kafka/pull/22409#discussion_r3370455603
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/LogCleaner.java:
##########
@@ -95,7 +95,7 @@
* tombstone deletion.</li>
* </ol>
*/
-public class LogCleaner implements BrokerReconfigurable {
+public class LogCleaner implements BrokerReconfigurable<AbstractConfig> {
Review Comment:
Thanks, I agree that `BrokerReconfigurable` should live in the server module
and use `AbstractKafkaConfig` directly.
I updated the structure to separate the component-level capability from the
broker integration contract:
- `DynamicConfigurable<T>` lives in `server-common` and represents a
module-neutral dynamic reconfiguration capability.
- `LogCleaner` implements `DynamicConfigurable<CleanerConfig>`, so it still
explicitly declares that it supports dynamic reconfiguration, but it only
depends on its own `CleanerConfig`.
- `BrokerReconfigurable` lives in `server` and extends
`DynamicConfigurable<AbstractKafkaConfig>`, so the broker integration contract
is server-owned and clearly uses `AbstractKafkaConfig`.
- `BrokerReconfigurableAdapter<T>` lives in `server` and bridges
`DynamicConfigurable<T>` components into `BrokerReconfigurable` by translating
`AbstractKafkaConfig` into the component-owned config type.
Compared with the generic approach, `BrokerReconfigurable` no longer needs
to be generic, and `storage` does not need to depend on the broker
reconfiguration contract.
Compared with a one-off `LogCleanerAdapter`, `LogCleaner` still declares its
dynamic reconfiguration capability in its own class signature, and we avoid
creating one adapter class per component. The server registration only declares
the config translation:
```java
BrokerReconfigurableAdapter.of(
cleaner,
brokerConfig -> new CleanerConfig(brokerConfig)
)
```
So the split is:
- components own validation/application behavior with their own config type
- server owns broker lifecycle integration and `AbstractKafkaConfig`
translation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
