Skip to content

Conversation

@TakaHiR07
Copy link
Contributor

Fixes #18923

Motivation

As described in #18923, recoverTracker.handleCommittingAndAbortingTransaction() fail when TC recover.

Modifications

when transactionLog.replayAsync() finish, complete TC future.

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

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • Anything that affects deployment

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@@ -129,7 +130,6 @@ public void replayComplete() {
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@congbobo184 I have moved future complete after changeToReadyState().

@congbobo184 congbobo184 added doc-not-needed Your PR changes do not impact docs area/transaction labels Dec 14, 2022
@congbobo184 congbobo184 added this to the 2.12.0 milestone Dec 14, 2022
@congbobo184 congbobo184 changed the title [fix][txn] fix error in recoverTracker.handleCommittingAndAbortingTransaction() [improve][txn] fix error in recoverTracker.handleCommittingAndAbortingTransaction() Dec 14, 2022
@TakaHiR07 TakaHiR07 force-pushed the fix_tc_recover_error branch from 2e29625 to 6346f64 Compare December 14, 2022 08:55
Copy link
Contributor

@liangyepianzhou liangyepianzhou left a comment

Choose a reason for hiding this comment

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

Good catch!

@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2022

Codecov Report

Merging #18924 (6346f64) into master (3180a4a) will decrease coverage by 12.39%.
The diff coverage is 49.61%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #18924       +/-   ##
=============================================
- Coverage     46.17%   33.77%   -12.40%     
+ Complexity    10359     6408     -3951     
=============================================
  Files           703      623       -80     
  Lines         68845    59104     -9741     
  Branches       7382     6146     -1236     
=============================================
- Hits          31788    19963    -11825     
- Misses        33448    36470     +3022     
+ Partials       3609     2671      -938     
Flag Coverage Δ
unittests 33.77% <49.61%> (-12.40%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../main/java/org/apache/pulsar/PulsarStandalone.java 0.00% <0.00%> (ø)
...broker/intercept/ManagedLedgerInterceptorImpl.java 0.00% <0.00%> (ø)
...va/org/apache/pulsar/broker/service/ServerCnx.java 47.17% <ø> (-0.26%) ⬇️
...er/service/persistent/GeoPersistentReplicator.java 0.00% <0.00%> (-4.71%) ⬇️
...roker/service/persistent/PersistentReplicator.java 0.00% <0.00%> (-24.06%) ⬇️
...va/org/apache/pulsar/client/impl/ConsumerImpl.java 15.09% <0.00%> (-0.04%) ⬇️
...he/pulsar/client/impl/MultiTopicsConsumerImpl.java 22.78% <0.00%> (-0.09%) ⬇️
...va/org/apache/pulsar/client/impl/ProducerImpl.java 16.83% <0.00%> (-0.17%) ⬇️
...nsaction/pendingack/impl/PendingAckHandleImpl.java 44.44% <50.00%> (-7.40%) ⬇️
.../service/SystemTopicBasedTopicPoliciesService.java 50.30% <60.86%> (-22.26%) ⬇️
... and 226 more

@congbobo184 congbobo184 merged commit a35670d into apache:master Dec 16, 2022
lifepuzzlefun pushed a commit to lifepuzzlefun/pulsar that referenced this pull request Jan 10, 2023
…gTransaction() (apache#18924)

Fixes apache#18923

### Motivation

As described in apache#18923, recoverTracker.handleCommittingAndAbortingTransaction() fail when TC recover.

### Modifications

when transactionLog.replayAsync() finish, complete TC future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/transaction doc-not-needed Your PR changes do not impact docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] [txn] TC recover fail in recoverTracker.handleCommittingAndAbortingTransaction()

4 participants