junrao commented on code in PR #19371:
URL: https://github.com/apache/kafka/pull/19371#discussion_r2073985790
##########
core/src/test/scala/unit/kafka/server/ControllerConfigurationValidatorTest.scala:
##########
@@ -73,15 +74,15 @@ class ControllerConfigurationValidatorTest {
def testValidTopicConfig(): Unit = {
val config = new util.TreeMap[String, String]()
config.put(SEGMENT_JITTER_MS_CONFIG, "1000")
- config.put(SEGMENT_BYTES_CONFIG, "67108864")
+ config.put(LogConfig.INTERNAL_SEGMENT_BYTES_CONFIG, "67108864")
Review Comment:
This change is unnecessary, right? Ditto below.
##########
core/src/test/scala/kafka/raft/KafkaMetadataLogTest.scala:
##########
@@ -1103,7 +1090,7 @@ object KafkaMetadataLogTest {
UnifiedLog.logDirName(KafkaRaftServer.MetadataPartition)
)
- val metadataLog = KafkaMetadataLog(
+ val metadataLog = KafkaMetadataLog.apply(
Review Comment:
Is this change needed?
##########
core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala:
##########
@@ -101,27 +101,27 @@ class DynamicConfigChangeTest extends
KafkaServerTestHarness {
val tp = new TopicPartition("test", 0)
val oldSegmentSize = 1000
val logProps = new Properties()
- logProps.put(TopicConfig.SEGMENT_BYTES_CONFIG, oldSegmentSize.toString)
+ logProps.put(LogConfig.INTERNAL_SEGMENT_BYTES_CONFIG,
oldSegmentSize.toString)
Review Comment:
Is this change needed since we don't need a small segment size? Ditto below.
##########
streams/src/test/java/org/apache/kafka/streams/StreamsConfigTest.java:
##########
@@ -248,7 +247,7 @@ public void
consumerConfigMustContainStreamPartitionAssignorConfig() {
props.put(StreamsConfig.PROBING_REBALANCE_INTERVAL_MS_CONFIG, 99_999L);
props.put(StreamsConfig.WINDOW_STORE_CHANGE_LOG_ADDITIONAL_RETENTION_MS_CONFIG,
7L);
props.put(StreamsConfig.APPLICATION_SERVER_CONFIG, "dummy:host");
- props.put(StreamsConfig.topicPrefix(TopicConfig.SEGMENT_BYTES_CONFIG),
100);
+ props.put(StreamsConfig.topicPrefix("internal.segment.bytes"), 100);
Review Comment:
Could we use the constant val ? Ditto below.
##########
metadata/src/test/java/org/apache/kafka/controller/ReplicationControlManagerTest.java:
##########
@@ -905,7 +904,7 @@ public void testCreateTopicsWithPolicy() {
null, Map.of()),
new CreateTopicPolicy.RequestMetadata("baz", null, null,
Map.of(0, List.of(2, 1, 0)),
- Map.of(SEGMENT_BYTES_CONFIG, "12300000")),
+ Map.of("internal.segment.bytes", "12300000")),
Review Comment:
Is this change needed? Ditto below.
##########
share-coordinator/src/test/java/org/apache/kafka/coordinator/share/ShareCoordinatorServiceTest.java:
##########
@@ -1999,7 +1999,7 @@ public void testShareStateTopicConfigs() {
List<String> propNames = List.of(
TopicConfig.CLEANUP_POLICY_CONFIG,
TopicConfig.COMPRESSION_TYPE_CONFIG,
- TopicConfig.SEGMENT_BYTES_CONFIG,
+ "internal.segment.bytes",
Review Comment:
Hmm, this doesn't look right. service.shareGroupStateTopicConfigs() doesn't
include this internal config.
##########
raft/src/main/java/org/apache/kafka/raft/MetadataLogConfig.java:
##########
@@ -112,15 +110,14 @@ public class MetadataLogConfig {
* @param deleteDelayMillis The amount of time to wait before deleting a
file from the filesystem
*/
public MetadataLogConfig(int logSegmentBytes,
- int logSegmentMinBytes,
long logSegmentMillis,
long retentionMaxBytes,
long retentionMillis,
int maxBatchSizeInBytes,
int maxFetchSizeInBytes,
long deleteDelayMillis) {
this.logSegmentBytes = logSegmentBytes;
- this.logSegmentMinBytes = logSegmentMinBytes;
+ this.internalLogSegmentBytes = logSegmentBytes;
Review Comment:
Could we add a comment that this constructor is for test only? Could we also
rename logSegmentBytes to internalLogSegmentBytes?
##########
streams/integration-tests/src/test/java/org/apache/kafka/streams/integration/PurgeRepartitionTopicIntegrationTest.java:
##########
@@ -114,7 +114,7 @@ public final boolean conditionMet() {
.get();
return
config.get(TopicConfig.CLEANUP_POLICY_CONFIG).value().equals(TopicConfig.CLEANUP_POLICY_DELETE)
&&
config.get(TopicConfig.SEGMENT_MS_CONFIG).value().equals(PURGE_INTERVAL_MS.toString())
- &&
config.get(TopicConfig.SEGMENT_BYTES_CONFIG).value().equals(PURGE_SEGMENT_BYTES.toString());
+ &&
config.get("internal.segment.bytes").value().equals(PURGE_SEGMENT_BYTES.toString());
Review Comment:
Could we use the constant val ? Ditto below.
--
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
