coderzc commented on code in PR #21541: URL: https://github.com/apache/pulsar/pull/21541#discussion_r1390606449
########## pip/pip-318.md: ########## @@ -0,0 +1,49 @@ +# PIP-318: Don't retain null-key messages during topic compaction + +# Background knowledge + +Apache Pulsar is supported [Topic Compaction](https://pulsar.apache.org/docs/en/concepts-topic-compaction/) which is a key-based data retention mechanism. + +# Motivation + +Currently, we retain all null-key messages during topic compaction, which I don't think is necessary because when you use topic compaction, it means that you want to retain the value according to the key, so retaining null-key messages is meaningless. + +Additionally, retaining all null-key messages will double the storage cost, and we'll never be able to clean them up since the compacted topic has not supported the retention policy yet. + +In summary, I don't think we should retain null-key messages during topic compaction. + +# Goals + +# High Level Design + +In order to avoid introducing break changes to release version, we need to add a configuration to control whether to retain null-key messages during topic compaction. +If the configuration is true, we will retain null-key messages during topic compaction, otherwise, we will not retain null-key messages during topic compaction. + +## Public-facing Changes + +### Configuration + +Add config to broker.conf +```properties +compactionRemainNullKey=false Review Comment: `topicCompactionRemainNullKey` seems to be better, as it requires cherry-picking to the old branch. ########## pip/pip-318.md: ########## @@ -0,0 +1,49 @@ +# PIP-318: Don't retain null-key messages during topic compaction + +# Background knowledge + +Apache Pulsar is supported [Topic Compaction](https://pulsar.apache.org/docs/en/concepts-topic-compaction/) which is a key-based data retention mechanism. + +# Motivation + +Currently, we retain all null-key messages during topic compaction, which I don't think is necessary because when you use topic compaction, it means that you want to retain the value according to the key, so retaining null-key messages is meaningless. + +Additionally, retaining all null-key messages will double the storage cost, and we'll never be able to clean them up since the compacted topic has not supported the retention policy yet. + +In summary, I don't think we should retain null-key messages during topic compaction. + +# Goals + +# High Level Design + +In order to avoid introducing break changes to release version, we need to add a configuration to control whether to retain null-key messages during topic compaction. +If the configuration is true, we will retain null-key messages during topic compaction, otherwise, we will not retain null-key messages during topic compaction. + +## Public-facing Changes + +### Configuration + +Add config to broker.conf +```properties +compactionRemainNullKey=false +``` + +# Backward & Forward Compatibility + +- Make `compactionRemainNullKey=false` default in the 3.2.0. +- Cherry-pick it to a branch less than 3.2.0 make `compactionRemainNullKey=true` default. +- Delete the configuration `compactionRemainNullKey` in 3.3.0 and don't supply an option to retain null-keys. + +## Revert + +Make `compactionRemainNullKey=true` in broker.conf. Review Comment: Ok. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
