clolov commented on code in PR #14161:
URL: https://github.com/apache/kafka/pull/14161#discussion_r1300216006
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/LogConfig.java:
##########
@@ -500,22 +500,29 @@ public static void validateBrokerLogConfigValues(Map<?,
?> props,
* The default values should be extracted from the KafkaConfig.
* @param props The properties to be validated
*/
- private static void validateTopicLogConfigValues(Map<?, ?> props,
- boolean
isRemoteLogStorageSystemEnabled) {
+ public static void validateTopicLogConfigValues(Map<?, ?> props,
+ boolean
isRemoteLogStorageSystemEnabled,
+ boolean
isReceivingConfigFromStore) {
validateValues(props);
boolean isRemoteLogStorageEnabled = (Boolean)
props.get(TopicConfig.REMOTE_LOG_STORAGE_ENABLE_CONFIG);
if (isRemoteLogStorageEnabled) {
-
validateRemoteStorageOnlyIfSystemEnabled(isRemoteLogStorageSystemEnabled);
- validateNoRemoteStorageForCompactedTopic(props);
- validateRemoteStorageRetentionSize(props);
- validateRemoteStorageRetentionTime(props);
+
validateRemoteStorageOnlyIfSystemEnabled(isRemoteLogStorageSystemEnabled,
isReceivingConfigFromStore);
+ if (!isReceivingConfigFromStore) {
+ validateNoRemoteStorageForCompactedTopic(props);
+ validateRemoteStorageRetentionSize(props);
+ validateRemoteStorageRetentionTime(props);
+ }
}
}
- private static void validateRemoteStorageOnlyIfSystemEnabled(boolean
isRemoteLogStorageSystemEnabled) {
+ private static void validateRemoteStorageOnlyIfSystemEnabled(boolean
isRemoteLogStorageSystemEnabled, boolean isReceivingConfigFromStore) {
if (!isRemoteLogStorageSystemEnabled) {
- throw new ConfigException("Tiered Storage functionality is
disabled in the broker. " +
- "Topic cannot be configured with remote log storage.");
+ if (isReceivingConfigFromStore) {
Review Comment:
What would your suggestion for such a message be? I left it as two separate
ones because in one situation we say "hey, you cannot disable TS on the cluster
while you have TS topics" while in the other we say "hey, you cannot enable TS
on topics while you do not have TS on the cluster", but I see value in your
suggestion. Would you like "You cannot have topics with enabled remote storage
if Tiered Storage functionality is disabled"?
--
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
