[Pulsar Transaction]Transaction client transaction state check #9776
Conversation
congbobo184
changed the title
congbobo184
changed the title
| OPEN, | ||
| CLOSE |
There was a problem hiding this comment.
Why not keep the state consistent with the transaction state? Open/Committing/Aborting/Committed/Aborted ?
There was a problem hiding this comment.
some situation we can't get the real state fro this transaction, example: the transaction timeout, it will be aborted but client commit.
There was a problem hiding this comment.
So the state should be committing -> closed ?
There was a problem hiding this comment.
commit or abort finish, the state will become closed. we don't need to change the state to committing or aborting, it seem not useful. When we need committing or aborting sate we can add.
|
/pulsarbot run-failure-checks |
1 similar comment
|
/pulsarbot run-failure-checks |
| } | ||
| tcClient.abortAsync(new TxnID(txnIdMostBits, txnIdLeastBits)).whenComplete((vx, ex) -> { | ||
| return checkIfOpen().thenCompose(value -> { | ||
| CompletableFuture<Void> abortFuture = new CompletableFuture<>(); |
There was a problem hiding this comment.
Change to aborting first?
| .withTransactionTimeout(1, TimeUnit.SECONDS) | ||
| .build().get(); | ||
|
|
||
| Thread.sleep(2000); |
There was a problem hiding this comment.
Please avoid use sleep.
| return FutureUtil.failedFuture(new InvalidTxnStatusException("[" + txnIdMostBits + ":" | ||
| + txnIdLeastBits + "] Transaction in " + state.name() + " state!")); |
There was a problem hiding this comment.
print the expected transaction state and the current transaction state?
|
@gaoran10 Please help review this PR |
codelipenghui
requested review from
sijie,
315157973,
hangc0276,
jiazhai,
merlimat and
zymap
|
/pulsarbot run-failure-checks |
Motivation
Now pulsar client don't check the transaction state, so when it committed or aborted, it also can produce or ack message. It should be prevented.
implement
Add the State :
when open we can do transaction operation, it is close we can't do transaction operation.
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: (yes)
The rest endpoints: (no)
The admin cli options: (no)
Anything that affects deployment: (no)