Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ object TransactionCoordinator {
config.transactionRemoveExpiredTransactionalIdCleanupIntervalMs,
config.requestTimeoutMs)

val txnStateManager = new TransactionStateManager(config.brokerId, scheduler, replicaManager, txnConfig,
val txnStateManager = new TransactionStateManager(config.brokerId, scheduler, replicaManager, metadataCache, txnConfig,
time, metrics)

val logContext = new LogContext(s"[TransactionCoordinator id=${config.brokerId}] ")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ object TransactionLog {
*
* @return value payload bytes
*/
private[transaction] def valueToBytes(txnMetadata: TxnTransitMetadata): Array[Byte] = {
private[transaction] def valueToBytes(txnMetadata: TxnTransitMetadata,
usesFlexibleRecords: Boolean): Array[Byte] = {
if (txnMetadata.txnState == Empty && txnMetadata.topicPartitions.nonEmpty)
throw new IllegalStateException(s"Transaction is not expected to have any partitions since its state is ${txnMetadata.txnState}: $txnMetadata")

Expand All @@ -75,9 +76,11 @@ object TransactionLog {
.setPartitionIds(partitions.map(tp => Integer.valueOf(tp.partition)).toList.asJava)
}.toList.asJava

// Serialize with the highest supported non-flexible version
// until a tagged field is introduced or the version is bumped.
MessageUtil.toVersionPrefixedBytes(0,
// Serialize with version 0 (highest non-flexible version) until transaction.version 1 is enabled
// which enables flexible fields in records.
val version: Short =
Comment thread
jolshan marked this conversation as resolved.
if (usesFlexibleRecords) 1 else 0
MessageUtil.toVersionPrefixedBytes(version,
new TransactionLogValue()
.setProducerId(txnMetadata.producerId)
.setProducerEpoch(txnMetadata.producerEpoch)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import java.nio.ByteBuffer
import java.util.Properties
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.locks.ReentrantReadWriteLock
import kafka.server.{ReplicaManager, RequestLocal}
import kafka.server.{MetadataCache, ReplicaManager, RequestLocal}
import kafka.utils.CoreUtils.{inReadLock, inWriteLock}
import kafka.utils.{Logging, Pool}
import kafka.utils.Implicits._
Expand All @@ -36,6 +36,7 @@ import org.apache.kafka.common.requests.TransactionResult
import org.apache.kafka.common.utils.{Time, Utils}
import org.apache.kafka.common.{KafkaException, TopicPartition}
import org.apache.kafka.coordinator.transaction.{TransactionLogConfigs, TransactionStateManagerConfigs}
import org.apache.kafka.server.common.TransactionVersion
import org.apache.kafka.server.config.ServerConfigs
import org.apache.kafka.server.record.BrokerCompressionType
import org.apache.kafka.server.util.Scheduler
Expand Down Expand Up @@ -65,6 +66,7 @@ import scala.collection.mutable
class TransactionStateManager(brokerId: Int,
scheduler: Scheduler,
replicaManager: ReplicaManager,
metadataCache: MetadataCache,
config: TransactionConfig,
time: Time,
metrics: Metrics) extends Logging {
Expand Down Expand Up @@ -99,6 +101,10 @@ class TransactionStateManager(brokerId: Int,
TransactionStateManagerConfigs.METRICS_GROUP,
"The avg time it took to load the partitions in the last 30sec"), new Avg())

private def usesFlexibleRecords(): Boolean = {
metadataCache.features().finalizedFeatures().getOrDefault(TransactionVersion.FEATURE_NAME, 0.toShort) > 0
}

// visible for testing only
private[transaction] def addLoadingPartition(partitionId: Int, coordinatorEpoch: Int): Unit = {
val partitionAndLeaderEpoch = TransactionPartitionAndLeaderEpoch(partitionId, coordinatorEpoch)
Expand Down Expand Up @@ -618,7 +624,7 @@ class TransactionStateManager(brokerId: Int,

// generate the message for this transaction metadata
val keyBytes = TransactionLog.keyToBytes(transactionalId)
val valueBytes = TransactionLog.valueToBytes(newMetadata)
val valueBytes = TransactionLog.valueToBytes(newMetadata, usesFlexibleRecords())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add a test to ensure that this new logic works as expected?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functionally there is no difference now, so a test didn't come to my mind easily. I guess the best thing is to just look at the version of the record generated. I can add that.

val timestamp = time.milliseconds()

val records = MemoryRecords.withRecords(TransactionLog.EnforcedCompression, new SimpleRecord(timestamp, keyBytes, valueBytes))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public void testNoAutoStart() {

@ClusterTest
public void testDefaults(ClusterInstance clusterInstance) {
Assertions.assertEquals(MetadataVersion.IBP_4_0_IV0, clusterInstance.config().metadataVersion());
Assertions.assertEquals(MetadataVersion.IBP_4_0_IV1, clusterInstance.config().metadataVersion());
}

@ClusterTests({
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/kafka/test/annotation/ClusterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
AutoStart autoStart() default AutoStart.DEFAULT;
SecurityProtocol securityProtocol() default SecurityProtocol.PLAINTEXT;
String listener() default "";
MetadataVersion metadataVersion() default MetadataVersion.IBP_4_0_IV0;
MetadataVersion metadataVersion() default MetadataVersion.IBP_4_0_IV1;
ClusterConfigProperty[] serverProperties() default {};
// users can add tags that they want to display in test
String[] tags() default {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,13 @@ abstract class QuorumTestHarness extends Logging {
))
}

metadataRecords.add(new ApiMessageAndVersion(
new FeatureLevelRecord()
.setName(Features.TRANSACTION_VERSION.featureName)
.setFeatureLevel(Features.TRANSACTION_VERSION.latestTesting),
0.toShort
))

optionalMetadataRecords.foreach { metadataArguments =>
for (record <- metadataArguments) metadataRecords.add(record)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ object ZkMigrationIntegrationTest {
MetadataVersion.IBP_3_7_IV2,
MetadataVersion.IBP_3_7_IV4,
MetadataVersion.IBP_3_8_IV0,
MetadataVersion.IBP_4_0_IV0
MetadataVersion.IBP_4_0_IV0,
MetadataVersion.IBP_4_0_IV1
).map { mv =>
val serverProperties = new util.HashMap[String, String]()
serverProperties.put("inter.broker.listener.name", "EXTERNAL")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import org.apache.kafka.common.record.{FileRecords, MemoryRecords, RecordBatch,
import org.apache.kafka.common.requests._
import org.apache.kafka.common.utils.{LogContext, MockTime, ProducerIdAndEpoch}
import org.apache.kafka.common.{Node, TopicPartition}
import org.apache.kafka.server.common.{FinalizedFeatures, MetadataVersion, TransactionVersion}
import org.apache.kafka.storage.internals.log.{FetchDataInfo, FetchIsolation, LogConfig, LogOffsetMetadata}
import org.junit.jupiter.api.Assertions._
import org.junit.jupiter.api.{AfterEach, BeforeEach, Test}
Expand Down Expand Up @@ -75,7 +76,23 @@ class TransactionCoordinatorConcurrencyTest extends AbstractCoordinatorConcurren
when(zkClient.getTopicPartitionCount(TRANSACTION_STATE_TOPIC_NAME))
.thenReturn(Some(numPartitions))

txnStateManager = new TransactionStateManager(0, scheduler, replicaManager, txnConfig, time,
val brokerNode = new Node(0, "host", 10)
val metadataCache: MetadataCache = mock(classOf[MetadataCache])
when(metadataCache.getPartitionLeaderEndpoint(
anyString,
anyInt,
any[ListenerName])
).thenReturn(Some(brokerNode))
when(metadataCache.features()).thenReturn {
new FinalizedFeatures(
MetadataVersion.latestTesting(),
Collections.singletonMap(TransactionVersion.FEATURE_NAME, TransactionVersion.TV_2.featureLevel()),
0,
true
)
}

txnStateManager = new TransactionStateManager(0, scheduler, replicaManager, metadataCache, txnConfig, time,
new Metrics())
txnStateManager.startup(() => zkClient.getTopicPartitionCount(TRANSACTION_STATE_TOPIC_NAME).get,
enableTransactionalIdExpiration = true)
Expand All @@ -89,13 +106,6 @@ class TransactionCoordinatorConcurrencyTest extends AbstractCoordinatorConcurren
} else {
Success(producerId)
})
val brokerNode = new Node(0, "host", 10)
val metadataCache: MetadataCache = mock(classOf[MetadataCache])
when(metadataCache.getPartitionLeaderEndpoint(
anyString,
anyInt,
any[ListenerName])
).thenReturn(Some(brokerNode))
val networkClient: NetworkClient = mock(classOf[NetworkClient])
txnMarkerChannelManager = new TransactionMarkerChannelManager(
KafkaConfig.fromProps(serverProps),
Expand Down Expand Up @@ -451,10 +461,10 @@ class TransactionCoordinatorConcurrencyTest extends AbstractCoordinatorConcurren
addPartitionsOp.awaitAndVerify(txn)

val txnMetadata = transactionMetadata(txn).getOrElse(throw new IllegalStateException(s"Transaction not found $txn"))
txnRecords += new SimpleRecord(txn.txnMessageKeyBytes, TransactionLog.valueToBytes(txnMetadata.prepareNoTransit()))
txnRecords += new SimpleRecord(txn.txnMessageKeyBytes, TransactionLog.valueToBytes(txnMetadata.prepareNoTransit(), true))

txnMetadata.state = PrepareCommit
txnRecords += new SimpleRecord(txn.txnMessageKeyBytes, TransactionLog.valueToBytes(txnMetadata.prepareNoTransit()))
txnRecords += new SimpleRecord(txn.txnMessageKeyBytes, TransactionLog.valueToBytes(txnMetadata.prepareNoTransit(), true))

prepareTxnLog(partitionId)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class TransactionLogTest {
val txnMetadata = TransactionMetadata(transactionalId, producerId, producerEpoch, transactionTimeoutMs, 0)
txnMetadata.addPartitions(topicPartitions)

assertThrows(classOf[IllegalStateException], () => TransactionLog.valueToBytes(txnMetadata.prepareNoTransit()))
assertThrows(classOf[IllegalStateException], () => TransactionLog.valueToBytes(txnMetadata.prepareNoTransit(), true))
}

@Test
Expand Down Expand Up @@ -80,7 +80,7 @@ class TransactionLogTest {
txnMetadata.addPartitions(topicPartitions)

val keyBytes = TransactionLog.keyToBytes(transactionalId)
val valueBytes = TransactionLog.valueToBytes(txnMetadata.prepareNoTransit())
val valueBytes = TransactionLog.valueToBytes(txnMetadata.prepareNoTransit(), true)

new SimpleRecord(keyBytes, valueBytes)
}.toSeq
Expand Down Expand Up @@ -120,7 +120,7 @@ class TransactionLogTest {
txnMetadata.addPartitions(Set(topicPartition))

val keyBytes = TransactionLog.keyToBytes(transactionalId)
val valueBytes = TransactionLog.valueToBytes(txnMetadata.prepareNoTransit())
val valueBytes = TransactionLog.valueToBytes(txnMetadata.prepareNoTransit(), true)
val transactionMetadataRecord = TestUtils.records(Seq(
new SimpleRecord(keyBytes, valueBytes)
)).records.asScala.head
Expand All @@ -146,7 +146,7 @@ class TransactionLogTest {
@Test
def testSerializeTransactionLogValueToHighestNonFlexibleVersion(): Unit = {
val txnTransitMetadata = TxnTransitMetadata(1, 1, 1, 1, 1000, CompleteCommit, Set.empty, 500, 500)
val txnLogValueBuffer = ByteBuffer.wrap(TransactionLog.valueToBytes(txnTransitMetadata))
val txnLogValueBuffer = ByteBuffer.wrap(TransactionLog.valueToBytes(txnTransitMetadata, true))
assertEquals(0, txnLogValueBuffer.getShort)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import java.util.concurrent.CountDownLatch
import java.util.concurrent.locks.ReentrantLock
import javax.management.ObjectName
import kafka.log.UnifiedLog
import kafka.server.{ReplicaManager, RequestLocal}
import kafka.server.{MetadataCache, ReplicaManager, RequestLocal}
import kafka.utils.{Pool, TestUtils}
import kafka.zk.KafkaZkClient
import org.apache.kafka.common.TopicPartition
Expand All @@ -36,6 +36,7 @@ import org.apache.kafka.common.record._
import org.apache.kafka.common.requests.ProduceResponse.PartitionResponse
import org.apache.kafka.common.requests.TransactionResult
import org.apache.kafka.common.utils.MockTime
import org.apache.kafka.server.common.{FinalizedFeatures, MetadataVersion, TransactionVersion}
import org.apache.kafka.server.util.MockScheduler
import org.apache.kafka.storage.internals.log.{AppendOrigin, FetchDataInfo, FetchIsolation, LogConfig, LogOffsetMetadata}
import org.junit.jupiter.api.Assertions._
Expand All @@ -44,6 +45,7 @@ import org.mockito.{ArgumentCaptor, ArgumentMatchers}
import org.mockito.ArgumentMatchers.{any, anyInt, anyLong, anyShort}
import org.mockito.Mockito.{atLeastOnce, mock, reset, times, verify, when}

import java.util.Collections
import scala.collection.{Map, mutable}
import scala.jdk.CollectionConverters._

Expand All @@ -61,15 +63,25 @@ class TransactionStateManagerTest {
val scheduler = new MockScheduler(time)
val zkClient: KafkaZkClient = mock(classOf[KafkaZkClient])
val replicaManager: ReplicaManager = mock(classOf[ReplicaManager])
val metadataCache: MetadataCache = mock(classOf[MetadataCache])

when(zkClient.getTopicPartitionCount(TRANSACTION_STATE_TOPIC_NAME))
.thenReturn(Some(numPartitions))

when(metadataCache.features()).thenReturn {
new FinalizedFeatures(
MetadataVersion.latestTesting(),
Collections.singletonMap(TransactionVersion.FEATURE_NAME, TransactionVersion.TV_2.featureLevel()),
0,
true
)
}

val metrics = new Metrics()

val txnConfig = TransactionConfig()
val transactionManager: TransactionStateManager = new TransactionStateManager(0, scheduler,
replicaManager, txnConfig, time, metrics)
replicaManager, metadataCache, txnConfig, time, metrics)

val transactionalId1: String = "one"
val transactionalId2: String = "two"
Expand Down Expand Up @@ -168,7 +180,7 @@ class TransactionStateManagerTest {
new TopicPartition("topic1", 0),
new TopicPartition("topic1", 1)))
val records = MemoryRecords.withRecords(startOffset, Compression.NONE,
new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit())))
new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit(), true)))

// We create a latch which is awaited while the log is loading. This ensures that the deletion
// is triggered before the loading returns
Expand Down Expand Up @@ -212,43 +224,43 @@ class TransactionStateManagerTest {
txnMetadata1.addPartitions(Set[TopicPartition](new TopicPartition("topic1", 0),
new TopicPartition("topic1", 1)))

txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit(), true))

// pid1's transaction adds three more partitions
txnMetadata1.addPartitions(Set[TopicPartition](new TopicPartition("topic2", 0),
new TopicPartition("topic2", 1),
new TopicPartition("topic2", 2)))

txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit(), true))

// pid1's transaction is preparing to commit
txnMetadata1.state = PrepareCommit

txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit(), true))

// pid2's transaction started with three partitions
txnMetadata2.state = Ongoing
txnMetadata2.addPartitions(Set[TopicPartition](new TopicPartition("topic3", 0),
new TopicPartition("topic3", 1),
new TopicPartition("topic3", 2)))

txnRecords += new SimpleRecord(txnMessageKeyBytes2, TransactionLog.valueToBytes(txnMetadata2.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes2, TransactionLog.valueToBytes(txnMetadata2.prepareNoTransit(), true))

// pid2's transaction is preparing to abort
txnMetadata2.state = PrepareAbort

txnRecords += new SimpleRecord(txnMessageKeyBytes2, TransactionLog.valueToBytes(txnMetadata2.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes2, TransactionLog.valueToBytes(txnMetadata2.prepareNoTransit(), true))

// pid2's transaction has aborted
txnMetadata2.state = CompleteAbort

txnRecords += new SimpleRecord(txnMessageKeyBytes2, TransactionLog.valueToBytes(txnMetadata2.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes2, TransactionLog.valueToBytes(txnMetadata2.prepareNoTransit(), true))

// pid2's epoch has advanced, with no ongoing transaction yet
txnMetadata2.state = Empty
txnMetadata2.topicPartitions.clear()

txnRecords += new SimpleRecord(txnMessageKeyBytes2, TransactionLog.valueToBytes(txnMetadata2.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes2, TransactionLog.valueToBytes(txnMetadata2.prepareNoTransit(), true))

val startOffset = 15L // it should work for any start offset
val records = MemoryRecords.withRecords(startOffset, Compression.NONE, txnRecords.toArray: _*)
Expand Down Expand Up @@ -877,7 +889,7 @@ class TransactionStateManagerTest {
txnMetadata1.addPartitions(Set[TopicPartition](new TopicPartition("topic1", 0),
new TopicPartition("topic1", 1)))

txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit(), true))
val startOffset = 0L
val records = MemoryRecords.withRecords(startOffset, Compression.NONE, txnRecords.toArray: _*)

Expand Down Expand Up @@ -1040,7 +1052,7 @@ class TransactionStateManagerTest {
txnMetadata1.addPartitions(Set[TopicPartition](new TopicPartition("topic1", 0),
new TopicPartition("topic1", 1)))

txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit(), true))
val startOffset = 0L
val records = MemoryRecords.withRecords(startOffset, Compression.NONE, txnRecords.toArray: _*)

Expand Down Expand Up @@ -1146,7 +1158,7 @@ class TransactionStateManagerTest {
txnMetadata1.addPartitions(Set[TopicPartition](new TopicPartition("topic1", 1),
new TopicPartition("topic1", 1)))

txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit(), true))

val startOffset = 15L
val records = MemoryRecords.withRecords(startOffset, Compression.NONE, txnRecords.toArray: _*)
Expand All @@ -1165,7 +1177,7 @@ class TransactionStateManagerTest {
txnMetadata1.addPartitions(Set[TopicPartition](new TopicPartition("topic1", 0),
new TopicPartition("topic1", 1)))

txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit()))
txnRecords += new SimpleRecord(txnMessageKeyBytes1, TransactionLog.valueToBytes(txnMetadata1.prepareNoTransit(), true))
val startOffset = 0L

val unknownKey = new TransactionLogKey()
Expand Down
Loading