[Transaction] Fix transaction problem when sub delete.#11305
[Transaction] Fix transaction problem when sub delete.#11305congbobo184 wants to merge 2 commits into
Conversation
congbobo184
added
cherry-picked/branch-2.8
eolivelli
left a comment
eolivelli
left a comment
There was a problem hiding this comment.
I don't understand why we are modifying the tests this way.
can you please explain ?
| ServerError.ServiceNotReady, | ||
| "Topic " + optionalTopic.get().getName() | ||
| + " subscription " + subName + " is not exist.")); | ||
| log.warn("Topic {} subscription {} is not exist.", optionalTopic.get().getName(), subName); |
There was a problem hiding this comment.
we can use this opportunity to fix the typo and use this message: Topic {} subscription {} does not exist
| tbClient.abortTxnOnSubscription(topic + "_abort_sub", "test", 1L, 1L, -1L).get(); | ||
| tbClient.commitTxnOnSubscription(topic + "_commit_sub", "test", 1L, 1L, -1L).get(); | ||
|
|
||
| Awaitility.await().until(() -> { |
There was a problem hiding this comment.
why do we have to use this loop ?
in this test the command is supposed to work well
which kind of error may happen ?
There was a problem hiding this comment.
waiting pending ack init.
There was a problem hiding this comment.
can we explicitly wait for that initialisation ?
I have a question about how users are supposed to deal with this problem.
I know that TransactionBufferClient is an internal API.
but when we think about the user of "Transaction#commit" (and abort)...
does the PulsarClient wait automatically for this initialisation to complete or will Transaction#commit fail ?
| } | ||
|
|
||
| @Test | ||
| public void testTransactionBufferOpFail() throws InterruptedException, ExecutionException { |
There was a problem hiding this comment.
why are we dropping this test ?
|
This patch looks like #11304 |
codelipenghui
added
release/2.8.1
and removed
cherry-picked/branch-2.8
|
@congbobo184 many thanks for your doc label! |
|
@congbobo184 Would you please address the comments by eolivelli ? |
|
#11304 have solve this. so close this pr. |
5 participants
Motivation
now, when sub has been deleted the end txn sub will fail.
implement
when the sub has been deleted the end txn sub op will success.
Verifying this change
Add the tests for it
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)