clolov commented on code in PR #14161:
URL: https://github.com/apache/kafka/pull/14161#discussion_r1290120601


##########
core/src/main/scala/kafka/server/ConfigHandler.scala:
##########
@@ -62,6 +62,12 @@ class TopicConfigHandler(private val logManager: LogManager, 
kafkaConfig: KafkaC
     topicConfig.asScala.forKeyValue { (key, value) =>
       if (!configNamesToExclude.contains(key)) props.put(key, value)
     }
+
+    if (!kafkaConfig.remoteLogManagerConfig.enableRemoteStorageSystem()
+      && topicConfig.getProperty(TopicConfig.REMOTE_LOG_STORAGE_ENABLE_CONFIG) 
!= null) {

Review Comment:
   My reasoning was the following:
   * When KIP-950 is reviewed the behaviour from chaning 
`enable.remote.storage=true` to `enable.remote.storage=false` will be defined 
and a retention/deletion policy will be in effect.
   * Right now (and for 3.6) if I change `enable.remote.storage` from `true` to 
`false` the topic will stop being archived, but the data which is already in 
remote storage won't be deleted.
   * If a person now disables tiered storage on a cluster-level there is 
nothing to let them know that they might have forgotten uncleaned data in 
remote storage.
   * If we forbid a topic to have the property then we draw the attention of 
the user to both situations where topics are still actively using remote 
storage and where topics have been disabled but remote data might still be 
present.
   * The way for them to express their certainty that they would like to 
proceed is to either completely remove the property or delete the topic.



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to