MINOR: BrokerMetadataSnapshotter must avoid exceeding batch size #12486
Conversation
BrokerMetadataSnapshotter should split up record lists that exceed the batch size.
There was a problem hiding this comment.
Thanks @cmccabe, just one main question -- is it okay to break up an atomic batch into two Lists when writing to a snapshot? Do we ever rely on reading an atomic batch of messages as one List when reading snapshots? E.g., what happens if we're dealing with a topic with 2000 partitions.
| * you also change the equivalent constant in the Raft layer, and have considered the | ||
| * compatibility issues. | ||
| */ | ||
| private val maxBatchSize = 1024 |
There was a problem hiding this comment.
Can we reference a constant for this special value?
There was a problem hiding this comment.
Good point. I added some more documentation here
| trait SnapshotWriterBuilder { | ||
| def build(committedOffset: Long, | ||
| committedEpoch: Int, | ||
| lastContainedLogTime: Long): Option[SnapshotWriter[ApiMessageAndVersion]] | ||
| } | ||
|
|
||
| class RecordListConsumer( |
There was a problem hiding this comment.
Maybe add a quick javadoc of what this class does? Also, maybe a more descriptive name (BatchingRecordListConsumer?)
No, we do not rely on this in the snapshot. The decision of what batches to use in the snapshot doesn't change semantics. In the log (as opposed to snapshot) we care about batching because batches are atomically committed, so if the controller fails midway into writing a multibatch operation we have to think about what happens next (probably a partial commit of whatever was going on) |
…(10 August 2022) Trivial conflict in gradle/dependencies.gradle due to the newer Netty version in confluentinc/kafka. * apache-github/trunk: MINOR: Upgrade gradle to 7.5.1 and bump other build/test dependencies (apache#12495) KAFKA-14140: Ensure an offline or in-controlled-shutdown replica is not eligible to join ISR in ZK mode (apache#12487) KAFKA-14114: Add Metadata Error Related Metrics MINOR: BrokerMetadataSnapshotter must avoid exceeding batch size (apache#12486) MINOR: Upgrade mockito test dependencies (apache#12460) KAFKA-14144:; Compare AlterPartition LeaderAndIsr before fencing partition epoch (apache#12489) KAFKA-14134: Replace EasyMock with Mockito for WorkerConnectorTest (apache#12472) MINOR: Update scala version in bin scripts to 2.13.8 (apache#12477) KAFKA-14104; Add CRC validation when iterating over Metadata Log Records (apache#12457) MINOR: add :server-common test dependency to :storage (apache#12488) KAFKA-14107: Upgrade Jetty version for CVE fixes (apache#12440) KAFKA-14124: improve quorum controller fault handling (apache#12447)
* apache-github/trunk: (447 commits) KAFKA-13959: Controller should unfence Broker with busy metadata log (apache#12274) KAFKA-10199: Expose read only task from state updater (apache#12497) KAFKA-14154; Return NOT_CONTROLLER from AlterPartition if leader is ahead of controller (apache#12506) KAFKA-13986; Brokers should include node.id in fetches to metadata quorum (apache#12498) KAFKA-14163; Retry compilation after zinc compile cache error (apache#12507) Remove duplicate common.message.* from clients:test jar file (apache#12407) KAFKA-13060: Replace EasyMock and PowerMock with Mockito in WorkerGroupMemberTest.java (apache#12484) Fix the rate window size calculation for edge cases (apache#12184) MINOR: Upgrade gradle to 7.5.1 and bump other build/test dependencies (apache#12495) KAFKA-14140: Ensure an offline or in-controlled-shutdown replica is not eligible to join ISR in ZK mode (apache#12487) KAFKA-14114: Add Metadata Error Related Metrics MINOR: BrokerMetadataSnapshotter must avoid exceeding batch size (apache#12486) MINOR: Upgrade mockito test dependencies (apache#12460) KAFKA-14144:; Compare AlterPartition LeaderAndIsr before fencing partition epoch (apache#12489) KAFKA-14134: Replace EasyMock with Mockito for WorkerConnectorTest (apache#12472) MINOR: Update scala version in bin scripts to 2.13.8 (apache#12477) KAFKA-14104; Add CRC validation when iterating over Metadata Log Records (apache#12457) MINOR: add :server-common test dependency to :storage (apache#12488) KAFKA-14107: Upgrade Jetty version for CVE fixes (apache#12440) KAFKA-14124: improve quorum controller fault handling (apache#12447) ...
BrokerMetadataSnapshotter should split up record lists that exceed the batch size.