MINOR: set replication.factor to 1 to make StreamsBrokerCompatibility… #10673
Conversation
…Service work with old broker
| properties = {streams_property.STATE_DIR: self.PERSISTENT_ROOT, | ||
| streams_property.KAFKA_SERVERS: self.kafka.bootstrap_servers(), | ||
| # the old broker (< 2.4) does not support configuration replication.factor=-1 | ||
| "replication.factor": 1} |
There was a problem hiding this comment.
Should we set it to 3 instead? IIRC, we run all system tests with 3 brokers? Wondering why the change broke the system tests, as they should have overwritten the default to 3 anyway?
There was a problem hiding this comment.
Sounds like we don't override the default after all? Or we have at least one test where that slipped through 🤷♀️
There was a problem hiding this comment.
I guess the logic scattered between Python and Java code... And maybe we need 3 only for EOS tests? Not sure either.
Also ok with me to just merge this PR as-is? Or should we update Java code instead?
There was a problem hiding this comment.
Ah, yeah, I bet that's it: we only set it for the EOS tests. Might be better to just set it in the Java code instead as it's easier to find and read, and I believe most other configs are set there. I think this test runs the StreamsSmokeTest?
There was a problem hiding this comment.
Should we set it to 3 instead? IIRC, we run all system tests with 3 brokers?
not really. streams_broker_compatibility_test.py run test with single broker ( https://github.com/apache/kafka/blob/trunk/tests/kafkatest/tests/streams/streams_broker_compatibility_test.py#L45)
Might be better to just set it in the Java code instead as it's easier to find and read, and I believe most other configs are set there.
I prefer to change python code rather than java code since the number of brokers is connected to replication refactor. If we add hardcode (i.e replication refactor = 1) in the java class, it is hard to change both of them in python.
I think this test runs the StreamsSmokeTest?
BrokerCompatibilityTest (https://github.com/apache/kafka/blob/trunk/tests/kafkatest/services/streams.py#L466)
There was a problem hiding this comment.
Fine with me either way. Thanks for the fix
There was a problem hiding this comment.
Fine with me, too. Feel free to merge.
…e-allocations-lz4 * apache-github/trunk: (155 commits) KAFKA-12728: Upgrade gradle to 7.0.2 and shadow to 7.0.0 (apache#10606) KAFKA-12778: Fix QuorumController request timeouts and electLeaders (apache#10688) KAFKA-12754: Improve endOffsets for TaskMetadata (apache#10634) Rework on KAFKA-3968: fsync the parent directory of a segment file when the file is created (apache#10680) MINOR: set replication.factor to 1 to make StreamsBrokerCompatibilityService work with old broker (apache#10673) MINOR: prevent cleanup() from being called while Streams is still shutting down (apache#10666) KAFKA-8326: Introduce List Serde (apache#6592) KAFKA-12697: Add Global Topic and Partition count metrics to the Quorum Controller (apache#10679) KAFKA-12648: MINOR - Add TopologyMetadata.Subtopology class for subtopology metadata (apache#10676) MINOR: Update jacoco to 0.8.7 for JDK 16 support (apache#10654) MINOR: exclude all `src/generated` and `src/generated-test` (apache#10671) KAFKA-12772: Move all transaction state transition rules into their states (apache#10667) KAFKA-12758 Added `server-common` module to have server side common classes. (apache#10638) MINOR Removed copying storage libraries specifically as they are already copied. (apache#10647) KAFKA-5876: KIP-216 Part 4, Apply InvalidStateStorePartitionException for Interactive Queries (apache#10657) KAFKA-12747: Fix flakiness in shouldReturnUUIDsWithStringPrefix (apache#10643) MINOR: remove unnecessary placeholder from WorkerSourceTask#recordSent (apache#10659) MINOR: Remove unused `scalatest` definition from `dependencies.gradle` (apache#10655) MINOR: checkstyle version upgrade: 8.20 -> 8.36.2 (apache#10656) KAFKA-12464: minor code cleanup and additional logging in constrained sticky assignment (apache#10645) ...
related to #10532
the default value of
replication.factorwas changed from1to-1. The old broker (< 2.4) does not support such configuration so this PR sets thereplication.factorto1to fixstreams_broker_compatibility_test.py.Committer Checklist (excluded from commit message)