cmccabe commented on code in PR #14208: URL: https://github.com/apache/kafka/pull/14208#discussion_r1295091800
########## metadata/src/main/java/org/apache/kafka/image/loader/MetadataBatchLoader.java: ########## @@ -0,0 +1,257 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.kafka.image.loader; + +import org.apache.kafka.common.metadata.MetadataRecordType; +import org.apache.kafka.common.utils.LogContext; +import org.apache.kafka.common.utils.Time; +import org.apache.kafka.image.MetadataDelta; +import org.apache.kafka.image.MetadataImage; +import org.apache.kafka.image.MetadataProvenance; +import org.apache.kafka.raft.Batch; +import org.apache.kafka.raft.LeaderAndEpoch; +import org.apache.kafka.server.common.ApiMessageAndVersion; +import org.apache.kafka.server.fault.FaultHandler; +import org.slf4j.Logger; + +import java.util.function.Supplier; + +import static java.util.concurrent.TimeUnit.NANOSECONDS; + +/** + * Loads batches of metadata updates from Raft commits into MetadataDelta-s. Multiple batches from a commit + * are buffered into a MetadataDelta to achieve batching of records and reduce the number of times + * MetadataPublishers must be updated. This class also supports metadata transactions (KIP-866). + * + * + */ +public class MetadataBatchLoader { + + enum TransactionState { + NO_TRANSACTION, + STARTED_TRANSACTION, + CONTINUED_TRANSACTION, + ENDED_TRANSACTION, + ABORTED_TRANSACTION; + } + + @FunctionalInterface + public interface MetadataUpdater { + void update(MetadataDelta delta, MetadataImage image, LogDeltaManifest manifest); + } + + private final Logger log; + private final Time time; + private final FaultHandler faultHandler; + private final Supplier<LeaderAndEpoch> leaderAndEpochSupplier; + private final MetadataUpdater callback; + + private MetadataImage image; + private MetadataDelta delta; + private long lastOffset; + private int lastEpoch; + private long lastContainedLogTimeMs; + private long numBytes; + private int numBatches; + private long totalBatchElapsedNs; + private TransactionState transactionState; + + public MetadataBatchLoader( + LogContext logContext, + Time time, + FaultHandler faultHandler, + Supplier<LeaderAndEpoch> leaderAndEpochSupplier, + MetadataUpdater callback + ) { + this.log = logContext.logger(MetadataBatchLoader.class); + this.time = time; + this.faultHandler = faultHandler; + this.leaderAndEpochSupplier = leaderAndEpochSupplier; + this.callback = callback; + } + + /** + * Reset the state of this batch loader to the given image. Any un-flushed state will be + * discarded. + * + * @param image Metadata image to reset this batch loader's state to. + */ + public void resetToImage(MetadataImage image) { + this.image = image; + this.delta = new MetadataDelta.Builder().setImage(image).build(); + this.transactionState = TransactionState.NO_TRANSACTION; + this.lastOffset = image.provenance().lastContainedOffset(); + this.lastEpoch = image.provenance().lastContainedEpoch(); + this.lastContainedLogTimeMs = image.provenance().lastContainedLogTimeMs(); + this.numBytes = 0; + this.numBatches = 0; + this.totalBatchElapsedNs = 0; + } + + /** + * Load a batch of records from the log. We have to do some bookkeeping here to + * translate between batch offsets and record offsets, and track the number of bytes we + * have read. Additionally, there is the chance that one of the records is a metadata + * version change which needs to be handled differently. + * </p> + * If this batch starts a transaction, any records preceding the transaction in this + * batch will be implicitly added to the transaction. + * + * @param batch The reader which yields the batches. + * @return The time in nanoseconds that elapsed while loading this batch + */ + + public long loadBatch(Batch<ApiMessageAndVersion> batch) { + long startNs = time.nanoseconds(); + int indexWithinBatch = 0; + + lastContainedLogTimeMs = batch.appendTimestamp(); + lastEpoch = batch.epoch(); + + for (ApiMessageAndVersion record : batch.records()) { + try { + replay(record); + } catch (Throwable e) { + faultHandler.handleFault("Error loading metadata log record from offset " + + batch.baseOffset() + indexWithinBatch, e); + } + + // Emit the accumulated delta if a new transaction has been started and one of the following is true + // 1) this is not the first record in this batch + // 2) this is not the first batch since last emitting a delta + if (transactionState == TransactionState.STARTED_TRANSACTION && (indexWithinBatch > 0 || numBatches > 0)) { + MetadataProvenance provenance = new MetadataProvenance(lastOffset, lastEpoch, lastContainedLogTimeMs); + LogDeltaManifest manifest = LogDeltaManifest.newBuilder() + .provenance(provenance) + .leaderAndEpoch(leaderAndEpochSupplier.get()) Review Comment: I don't think we need the `leaderAndEpoch` supplier here since we can just pass in `MetadataLoader.currentLeaderAndEpoch` to this function. The metadata loader already tracks this and it's cheaper than taking locks and contacting the raft layer -- 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
