BewareMyPower commented on code in PR #21406: URL: https://github.com/apache/pulsar/pull/21406#discussion_r1691068034
########## pulsar-broker/src/test/java/org/apache/pulsar/broker/transaction/buffer/TopicTransactionBufferTest.java: ########## @@ -405,4 +412,168 @@ private void assertGetLastMessageId(Consumer<?> consumer, MessageIdImpl expected assertEquals(expected.getLedgerId(), actual.getLedgerId()); } + /** + * This test verifies the state changes of a TransactionBuffer within a topic under different conditions. + * Initially, the TransactionBuffer is in a NoSnapshot state upon topic creation. + * It remains in the NoSnapshot state even after a normal message is sent. + * The state changes to Ready only after a transactional message is sent. + * The test also ensures that the TransactionBuffer can be correctly recovered after the topic is unloaded. + */ + @Test + public void testWriteSnapshotWhenFirstTxnMessageSend() throws Exception { + // 1. Prepare test environment. + String topic = "persistent://" + NAMESPACE1 + "/testWriteSnapshotWhenFirstTxnMessageSend"; + String txnMsg = "transaction message"; + String normalMsg = "normal message"; + admin.topics().createNonPartitionedTopic(topic); + PersistentTopic persistentTopic = (PersistentTopic) pulsarServiceList.get(0).getBrokerService() + .getTopic(topic, false) + .get() + .get(); + @Cleanup + Producer<String> producer = pulsarClient.newProducer(Schema.STRING) + .topic(topic) + .create(); + @Cleanup + Consumer<String> consumer = pulsarClient.newConsumer(Schema.STRING) + .topic(topic) + .subscriptionName("my-sub") + .subscribe(); + // 2. Test the state of transaction buffer after building producer with no new messages. + // The TransactionBuffer should be in NoSnapshot state before transaction message sent. + TopicTransactionBuffer topicTransactionBuffer = (TopicTransactionBuffer) persistentTopic.getTransactionBuffer(); + Awaitility.await().untilAsserted(() -> { + Assert.assertEquals(topicTransactionBuffer.getState(), TopicTransactionBufferState.State.NoSnapshot); + }); + // 3. Test the state of transaction buffer after sending normal messages. + // The TransactionBuffer should still be in NoSnapshot state after a normal message is sent. + producer.newMessage().value(normalMsg).send(); + Assert.assertEquals(topicTransactionBuffer.getState(), TopicTransactionBufferState.State.NoSnapshot); + // 4. Test the state of transaction buffer after sending transaction messages. + // The transaction buffer should be in Ready state at this time. + Transaction transaction = pulsarClient.newTransaction() + .withTransactionTimeout(5, TimeUnit.HOURS) + .build() + .get(); + producer.newMessage(transaction).value(txnMsg).send(); + Assert.assertEquals(topicTransactionBuffer.getState(), TopicTransactionBufferState.State.Ready); + // 5. Test transaction buffer can be recovered correctly. + // There are 4 message sent to this topic, 2 normal message and 2 transaction message |m1|m2-txn1|m3-txn1|m4|. + // Aborting the transaction and unload the topic and then redelivering unacked messages, + // only normal messages can be received. + transaction.abort().get(5, TimeUnit.SECONDS); + producer.newMessage().value(normalMsg).send(); + admin.topics().unload(topic); + PersistentTopic persistentTopic2 = (PersistentTopic) pulsarServiceList.get(0).getBrokerService() + .getTopic(topic, false) + .get() + .get(); + TopicTransactionBuffer topicTransactionBuffer2 = (TopicTransactionBuffer) persistentTopic2 + .getTransactionBuffer(); + Awaitility.await().untilAsserted(() -> { + Assert.assertEquals(topicTransactionBuffer2.getState(), TopicTransactionBufferState.State.Ready); + }); + consumer.redeliverUnacknowledgedMessages(); + for (int i = 0; i < 2; i++) { + Message<String> message = consumer.receive(5, TimeUnit.SECONDS); + Assert.assertEquals(message.getValue(), normalMsg); + } + Message<String> message = consumer.receive(5, TimeUnit.SECONDS); + Assert.assertNull(message); + } + + /** + * Send some messages before transaction buffer ready and then send some messages after transaction buffer ready, + * these messages should be received in order. + */ + @Test + public void testMessagePublishInOrder() throws Exception { + // 1. Prepare test environment. + String topic = "persistent://" + NAMESPACE1 + "/testMessagePublishInOrder" + RandomUtils.nextLong(); + admin.topics().createNonPartitionedTopic(topic); + PersistentTopic persistentTopic = (PersistentTopic) pulsarServiceList.get(0).getBrokerService() + .getTopic(topic, false) + .get() + .get(); + @Cleanup + Producer<Integer> producer = pulsarClient.newProducer(Schema.INT32) + .topic(topic) + .create(); + @Cleanup + Consumer<Integer> consumer = pulsarClient.newConsumer(Schema.INT32) + .topic(topic) + .subscriptionName("sub") + .subscribe(); + Transaction transaction = pulsarClient.newTransaction() + .withTransactionTimeout(5, TimeUnit.HOURS) + .build().get(); + // 2. Set a new future in transaction buffer as `transactionBufferFuture` to stimulate whether the + // transaction buffer recover completely. + TopicTransactionBuffer topicTransactionBuffer = (TopicTransactionBuffer) persistentTopic.getTransactionBuffer(); + CompletableFuture<Void> completableFuture = new CompletableFuture<>(); + Whitebox.setInternalState(topicTransactionBuffer, "transactionBufferFuture", completableFuture); + Field stateField = TopicTransactionBufferState.class.getDeclaredField("state"); + stateField.setAccessible(true); + stateField.set(topicTransactionBuffer, TopicTransactionBufferState.State.Ready); + // Register this topic to the transaction in advance to avoid the sending request pending here. + ((TransactionImpl) transaction).registerProducedTopic(topic).get(5, TimeUnit.SECONDS); Review Comment: It's not good to just introduce one more dependency for just one API. I also don't recommend modifying a final field via reflection. It would be better to mock methods rather than fields. See https://stackoverflow.com/questions/23629766/how-to-mock-final-field-mockito-powermock A proper way could be implementing a customized `TransactionBuffer` and call `PulsarService#setTransactionBufferProvider` to modify the TB provider. In this case, you can modify the internal stats like `transactionBufferFuture` and `state` without any reflection. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org