Skip to content

Conversation

@congbobo184
Copy link
Contributor

@congbobo184 congbobo184 commented Jul 11, 2022

Motivation

In the actual scenario, because the transaction will timeout, the transaction cannot block sending messages all the time. So we send messages with txn need to throw TimeoutException

Modifications

send messages with txn don't need set producer config .sendTimeout(0, TimeUnit.SECONDS)

Verifying this change

add send messages with txn throw TimeoutException test

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API: (no)
  • The schema: (no)
  • The default values of configurations: (no)
  • The wire protocol: (no)
  • The rest endpoints: (no)
  • The admin cli options: (no)
  • Anything that affects deployment: (no)

Documentation

  • Does this pull request introduces a new feature? (yes)

  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

  • If a feature is not applicable for documentation, explain why?

  • If a feature is not documented yet in this PR, please create a follow-up issue for adding the documentation

  • doc-not-needed

@congbobo184 congbobo184 added area/transaction doc-not-needed Your PR changes do not impact docs labels Jul 11, 2022
@congbobo184 congbobo184 added this to the 2.11.0 milestone Jul 11, 2022
@congbobo184 congbobo184 self-assigned this Jul 11, 2022
Copy link
Contributor

@gaoran10 gaoran10 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions github-actions bot added doc-label-missing and removed doc-not-needed Your PR changes do not impact docs labels Jul 11, 2022
@github-actions
Copy link

@congbobo184 Please provide a correct documentation label for your PR.
Instructions see Pulsar Documentation Label Guide.

@congbobo184 congbobo184 added the doc-not-needed Your PR changes do not impact docs label Jul 11, 2022
@codelipenghui codelipenghui changed the title [change][txn] Send message with txn can throw TimeoutException [fix][txn] Allow producer enable send timeout in transaction Jul 11, 2022
@codelipenghui codelipenghui added the type/bug The PR fixed a bug or issue reported a bug label Jul 11, 2022
@codelipenghui codelipenghui merged commit bbf2a47 into apache:master Jul 11, 2022
codelipenghui pushed a commit that referenced this pull request Jul 11, 2022
codelipenghui pushed a commit that referenced this pull request Jul 11, 2022
nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Jul 11, 2022
@nicoloboschi
Copy link
Contributor

@codelipenghui There's a test failing on branch-2.10 related to this pull

java.lang.AssertionError: expected [true] but found [false]
	at org.testng.Assert.fail(Assert.java:99)
	at org.testng.Assert.failNotEquals(Assert.java:1037)
	at org.testng.Assert.assertTrue(Assert.java:45)
	at org.testng.Assert.assertTrue(Assert.java:55)
	at org.apache.pulsar.client.impl.TransactionEndToEndTest.testSendTxnMessageTimeout(TransactionEndToEndTest.java:1099)

Could you verify ?

@nicoloboschi
Copy link
Contributor

I found the reason, it's related to the test itself. Will send a fix soon

congbobo184 pushed a commit that referenced this pull request Jul 14, 2022
…MessageTimeout (only release branches) (#16570)

### Motivation

`TransactionEndToEndTest#testSendTxnMessageTimeout` fails on releases branch after #16519 has been cherry-picked. 

```
java.lang.AssertionError: expected [true] but found [false]
	at org.testng.Assert.fail(Assert.java:99)
	at org.testng.Assert.failNotEquals(Assert.java:1037)
	at org.testng.Assert.assertTrue(Assert.java:45)
	at org.testng.Assert.assertTrue(Assert.java:55)
	at org.apache.pulsar.client.impl.TransactionEndToEndTest.testSendTxnMessageTimeout(TransactionEndToEndTest.java:1099)
```

The reason is that the mock setup must be slightly different since the `ProducerImpl` is not exactly the same between master and branch-2.10.
wuxuanqicn pushed a commit to wuxuanqicn/pulsar that referenced this pull request Jul 14, 2022
mattisonchao pushed a commit that referenced this pull request Jul 15, 2022
…MessageTimeout (only release branches) (#16570)

### Motivation

`TransactionEndToEndTest#testSendTxnMessageTimeout` fails on releases branch after #16519 has been cherry-picked. 

```
java.lang.AssertionError: expected [true] but found [false]
	at org.testng.Assert.fail(Assert.java:99)
	at org.testng.Assert.failNotEquals(Assert.java:1037)
	at org.testng.Assert.assertTrue(Assert.java:45)
	at org.testng.Assert.assertTrue(Assert.java:55)
	at org.apache.pulsar.client.impl.TransactionEndToEndTest.testSendTxnMessageTimeout(TransactionEndToEndTest.java:1099)
```

The reason is that the mock setup must be slightly different since the `ProducerImpl` is not exactly the same between master and branch-2.10.
nicoloboschi added a commit to datastax/pulsar that referenced this pull request Jul 15, 2022
…MessageTimeout (only release branches) (apache#16570)

### Motivation

`TransactionEndToEndTest#testSendTxnMessageTimeout` fails on releases branch after apache#16519 has been cherry-picked.

```
java.lang.AssertionError: expected [true] but found [false]
	at org.testng.Assert.fail(Assert.java:99)
	at org.testng.Assert.failNotEquals(Assert.java:1037)
	at org.testng.Assert.assertTrue(Assert.java:45)
	at org.testng.Assert.assertTrue(Assert.java:55)
	at org.apache.pulsar.client.impl.TransactionEndToEndTest.testSendTxnMessageTimeout(TransactionEndToEndTest.java:1099)
```

The reason is that the mock setup must be slightly different since the `ProducerImpl` is not exactly the same between master and branch-2.10.

(cherry picked from commit c83bede)
nicoloboschi added a commit to datastax/pulsar that referenced this pull request Jul 18, 2022
…MessageTimeout (only release branches) (apache#16570)

### Motivation

`TransactionEndToEndTest#testSendTxnMessageTimeout` fails on releases branch after apache#16519 has been cherry-picked.

```
java.lang.AssertionError: expected [true] but found [false]
	at org.testng.Assert.fail(Assert.java:99)
	at org.testng.Assert.failNotEquals(Assert.java:1037)
	at org.testng.Assert.assertTrue(Assert.java:45)
	at org.testng.Assert.assertTrue(Assert.java:55)
	at org.apache.pulsar.client.impl.TransactionEndToEndTest.testSendTxnMessageTimeout(TransactionEndToEndTest.java:1099)
```

The reason is that the mock setup must be slightly different since the `ProducerImpl` is not exactly the same between master and branch-2.10.

(cherry picked from commit c83bede)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/transaction cherry-picked/branch-2.9 Archived: 2.9 is end of life cherry-picked/branch-2.10 doc-not-needed Your PR changes do not impact docs release/2.9.4 release/2.10.2 type/bug The PR fixed a bug or issue reported a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants