prateekm commented on a change in pull request #1190: SAMZA-2352: Use min.compaction.lag.ms to avoid compacting the Kafka changelog topic URL: https://github.com/apache/samza/pull/1190#discussion_r336139886
########## File path: samza-core/src/main/java/org/apache/samza/config/StorageConfig.java ########## @@ -52,6 +52,9 @@ public static final boolean DEFAULT_DISALLOW_LARGE_MESSAGES = false; public static final String DROP_LARGE_MESSAGES = STORE_PREFIX + "%s.drop.large.messages"; public static final boolean DEFAULT_DROP_LARGE_MESSAGES = false; + // The log compaction lag time for transactional state change log + public static final String CHANGELOG_MIN_COMPACTION_LAG_MS = STORE_PREFIX + "%s.changelog.kafka.min.compaction.lag.ms"; Review comment: Oh I see, that's quite confusing. Even the configuration reference documents the changelog.kafka.* properties incorrectly (provided example is incorrect) and does not clarify its precedence. Looks like the convention is: if it's a known property that the framework cares about, set it explicitly using the changelog.xyz convention. If it's an arbitrary Kafka property, set it using changelog.kafka.xyz convention. Following this convention, and for consistency with existing behavior class, I'd still prefer to rename this to changelog.min.compaction.lag.ms. Seems like long term fix would be to give higher precedence to the explicitly specified properties over the kafka.* specified properties, although that's backwards incompatible. We can create a ticket to track that separately. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
