jiafu1115 commented on code in PR #20811:
URL: https://github.com/apache/kafka/pull/20811#discussion_r2488523880
##########
storage/src/main/java/org/apache/kafka/server/log/remote/metadata/storage/TopicBasedRemoteLogMetadataManagerConfig.java:
##########
@@ -46,13 +46,15 @@ public final class TopicBasedRemoteLogMetadataManagerConfig
{
public static final String
REMOTE_LOG_METADATA_TOPIC_REPLICATION_FACTOR_PROP =
"remote.log.metadata.topic.replication.factor";
public static final String REMOTE_LOG_METADATA_TOPIC_PARTITIONS_PROP =
"remote.log.metadata.topic.num.partitions";
public static final String REMOTE_LOG_METADATA_TOPIC_RETENTION_MS_PROP =
"remote.log.metadata.topic.retention.ms";
+ public static final String REMOTE_LOG_METADATA_TOPIC_MIN_ISR_PROP =
"remote.log.metadata.topic.min.isr";
Review Comment:
@AndrewJSchofield Hi. I checked the code about the "internal" topic and
summarised the result as follows:
<img width="1014" height="711" alt="image"
src="https://github.com/user-attachments/assets/d7bdd150-e255-448c-b967-799f399bbc42";
/>
It seems that __remote_log_metadata isn’t a purely internal topic, since it
can be configured as a normal topic in the remote Kafka cluster — even though
its name can’t be changed.
```
org.apache.kafka.server.log.remote.storage.RemoteLogManager#configAndWrapRlmmPlugin
private Plugin<RemoteLogMetadataManager>
configAndWrapRlmmPlugin(RemoteLogMetadataManager rlmm) {
final Map<String, Object> rlmmProps = new HashMap<>();
endpoint.ifPresent(e -> {
rlmmProps.put(REMOTE_LOG_METADATA_COMMON_CLIENT_PREFIX +
"bootstrap.servers", e.host() + ":" + e.port());
rlmmProps.put(REMOTE_LOG_METADATA_COMMON_CLIENT_PREFIX +
"security.protocol", e.securityProtocol().name);
});
// update the remoteLogMetadataProps here to override endpoint config if
any
rlmmProps.putAll(rlmConfig.remoteLogMetadataManagerProps());
// ...
}
```
Besides, the producer clientId for __remote_log_metadata will conflict with
```
//KafkaApis.scala
val internalTopicsAllowed = request.header.clientId == "__admin_client"
```
So maybe we shouldn’t make it an “internal” topic in this KIP when we’re not
sure it’s matched with current design, AND it will also help to keep this KIP
stay simple and clear without any concern. WDTY?
also cc @chia7712 @kamalcph for more thoughts.
Thanks!
--
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]
