[Transaction]No TransactionCoordinatorNotFound, but automatic reconnect #13135

liangyepianzhou · 2021-12-04T13:06:37Z

Motivation and Modification

We should not throw the following exceptions to the user to deal with.

TransactionCoordinatorNotFound or ManagerLedgerFenceException
--- we should retry the operation and reconnect to TC
TransactionMetaStoreHandler was connecting
---- add the operation into pendingRequests, and executed the requests in pendingRequests when the connected completely.
The complexity of concurrent operations is too high. For operations in a TransactionMetaStoreHandler, consider using single-threaded operations
--- use internalPinnedExecutor

Verifying this change

Make sure that the change passes the CI checks.

(Please pick either of the following options)

This change is a trivial rework / code cleanup without any test coverage.

(or)

This change is already covered by existing tests, such as (please describe tests).

(or)

This change added tests and can be verified as follows:

(example:)

Added integration tests for end-to-end deployment with large payloads (10MB)
Extended integration test for recovery after broker failure

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): (yes / no)
The public API: (yes / no)
The schema: (yes / no / don't know)
The default values of configurations: (yes / no)
The wire protocol: (yes / no)
The rest endpoints: (yes / no)
The admin cli options: (yes / no)
Anything that affects deployment: (yes / no / don't know)

Documentation

Check the box below and label this PR (if you have committer privilege).

Need to update docs?

doc-required

(If you need help on updating docs, create a doc issue)
no-need-doc

(Please explain why)
doc

(If this PR contains doc changes)

…ction ### Motivation we should not throw a TransactionCoordinatorNotFound to client. Beacuse that exception is a normal behavior. ### Modification Try the operation again when the client receives a TransactionCoordinatorNotFound.

2. add requestArgs 3. remove semaphore.release 4. add backoff in op

2. Split four opCallBack

congbobo184

Maybe we should handle ManagedLedgerFenceException as the same as TCNotFoundException, because this exception, customer also don't need to know.

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java

...-broker/src/test/java/org/apache/pulsar/broker/transaction/TransactionClientConnectTest.java

2. optimize recycle, safeRelease, cnx(), backoff 3. optimize test

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java

…need to be try again. 2. modify Result.h, Result.cc, ClientConnection.cc

2. Improve code reuse

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java

pulsar-client-cpp/include/pulsar/Result.h

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java

1. backoff 2. code reuse

1. optimize log 2. code reuse 3. fix Result.h by adding ,

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java

2. clear map and add send check for connecting and cnx 3. optimize test

eolivelli

This work looks great.

But it is a big patch and also in includes a little wire protocol change.
What about the compatibility with 2.9 clients ?

would you like to discuss about this work on dev@pulsar.apache.org ?
this way the community will be more informed and everybody will be able to contribute to the review and the discussion

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java

pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TransactionRetryTest.java

liangyepianzhou · 2021-12-12T12:55:53Z

/pulsarbot run-failure-checks

pulsar-broker/src/main/java/org/apache/pulsar/broker/TransactionMetadataStoreService.java

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java

2. move TransactionClientConnectTest to the package: rg.apache.pulsar.client.impl

2. remove the judge of cnx = null 3. add canSendRequest

2. remove the judge of cnx = null 3. Extract the data of the deposit response

2. details

…dSendRequest

…ct (apache#13135) ### Motivation and Modification We should not throw the following exceptions to the user to deal with. 1. `TransactionCoordinatorNotFound` or `ManagerLedgerFenceException` --- we should retry the operation and reconnect to TC 2. `TransactionMetaStoreHandler` was connecting ---- add the operation into `pendingRequests`, and executed the requests in `pendingRequests` when the connected completely. 3. The complexity of concurrent operations is too high. For operations in a TransactionMetaStoreHandler, consider using single-threaded operations --- use `internalPinnedExecutor`

…ct (#13135) ### Motivation and Modification We should not throw the following exceptions to the user to deal with. 1. `TransactionCoordinatorNotFound` or `ManagerLedgerFenceException` --- we should retry the operation and reconnect to TC 2. `TransactionMetaStoreHandler` was connecting ---- add the operation into `pendingRequests`, and executed the requests in `pendingRequests` when the connected completely. 3. The complexity of concurrent operations is too high. For operations in a TransactionMetaStoreHandler, consider using single-threaded operations --- use `internalPinnedExecutor` (cherry picked from commit 56323e4)

github-actions bot assigned liangyepianzhou Dec 4, 2021

github-actions bot added the doc-not-needed Your PR changes do not impact docs label Dec 4, 2021

liangyepianzhou added 2 commits December 7, 2021 13:35

1. use timer

e233fe4

2. add requestArgs 3. remove semaphore.release 4. add backoff in op

1. use timer from client

cbe3367

2. Split four opCallBack

congbobo184 reviewed Dec 7, 2021

View reviewed changes

1. handle ManagedLedgerFencedException

85ad312

2. optimize recycle, safeRelease, cnx(), backoff 3. optimize test

codelipenghui reviewed Dec 7, 2021

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java Outdated Show resolved Hide resolved

liangyepianzhou added 6 commits December 8, 2021 14:58

1. add handle TransactionMetadataStoreStateExceptionv and Connecting

63d9660

listener header

e9d17fd

1. add a TcOperationRetryException to represent the exceptions which …

cd56997

…need to be try again. 2. modify Result.h, Result.cc, ClientConnection.cc

1. modify PulsarClientException, ClientCnx

1c92668

1. Add the operation to a ConcurrentLongHashMap when state is connecting

e3cc397

2. Improve code reuse

fix lock with try finally

eebd568

congbobo184 reviewed Dec 9, 2021

View reviewed changes

liangyepianzhou added 4 commits December 10, 2021 10:38

Detail optimization：

7d1caa1

1. backoff 2. code reuse

Detail optimization：

ebfd47c

1. optimize log 2. code reuse 3. fix Result.h by adding ,

RuntimeException

56154be

Optimize call logic

d545001

congbobo184 reviewed Dec 10, 2021

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java Outdated Show resolved Hide resolved

congbobo184 reviewed Dec 11, 2021

View reviewed changes

pulsar-client/src/main/java/org/apache/pulsar/client/impl/TransactionMetaStoreHandler.java Outdated Show resolved Hide resolved

1. optimize log and naming

404d1b0

2. clear map and add send check for connecting and cnx 3. optimize test

eolivelli reviewed Dec 11, 2021

View reviewed changes

pulsar-broker/src/main/java/org/apache/pulsar/broker/service/ServerCnx.java Outdated Show resolved Hide resolved

pulsar-broker/src/test/java/org/apache/pulsar/client/impl/TransactionRetryTest.java Outdated Show resolved Hide resolved

liangyepianzhou added 3 commits December 12, 2021 19:18

delete the RetryException and relate test

85ac0d4

update

6c8b907

update

22e5811

liangyepianzhou added 2 commits December 12, 2021 21:22

delete tcFenced

4e3916a

Merge remote-tracking branch 'apache/master' into TC_not_found

1667e77

codelipenghui reviewed Dec 13, 2021

View reviewed changes

1. change TransactionMetaStoreHandler single thread

5a18ca6

2. move TransactionClientConnectTest to the package: rg.apache.pulsar.client.impl

codelipenghui approved these changes Dec 14, 2021

View reviewed changes

codelipenghui added this to the 2.10.0 milestone Dec 14, 2021

liangyepianzhou added 6 commits December 14, 2021 10:59

1. make the connect be single thread

2d06428

2. remove the judge of cnx = null 3. add canSendRequest

1. make the connect be single thread

2fd3b7f

2. remove the judge of cnx = null 3. Extract the data of the deposit response

1. Add callback timeout check

3d2d45e

2. details

take out the operation of adding op to pendingRequest in checkStateAn…

e19e3e6

…dSendRequest

take out the operation of adding op to pendingRequest in checkStateAn…

4a92069

…dSendRequest

Merge remote-tracking branch 'apache/master' into TC_not_found

4d5a792

congbobo184 merged commit 56323e4 into apache:master Dec 14, 2021

codelipenghui added the release/2.9.2 label Dec 17, 2021

codelipenghui added the cherry-picked/branch-2.9 Archived: 2.9 is end of life label Dec 21, 2021

gaoran10 added the area/transaction label Jan 25, 2022

Jason918 mentioned this pull request Feb 6, 2022

[Client][Transaction] Fix OpBase.callback is not called in TransactionMetaStoreHandler#failPendingRequest #14133

Merged

2 tasks

Genuineh mentioned this pull request Nov 8, 2023

Add Transaction supported on 2.9+ fsprojects/pulsar-client-dotnet#237

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Transaction]No TransactionCoordinatorNotFound, but automatic reconnect #13135

[Transaction]No TransactionCoordinatorNotFound, but automatic reconnect #13135

liangyepianzhou commented Dec 4, 2021 •

edited

Loading

congbobo184 left a comment

eolivelli left a comment •

edited

Loading

liangyepianzhou commented Dec 12, 2021

[Transaction]No TransactionCoordinatorNotFound, but automatic reconnect #13135

[Transaction]No TransactionCoordinatorNotFound, but automatic reconnect #13135

Conversation

liangyepianzhou commented Dec 4, 2021 • edited Loading

Motivation and Modification

Verifying this change

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

Documentation

congbobo184 left a comment

Choose a reason for hiding this comment

eolivelli left a comment • edited Loading

Choose a reason for hiding this comment

liangyepianzhou commented Dec 12, 2021

liangyepianzhou commented Dec 4, 2021 •

edited

Loading

eolivelli left a comment •

edited

Loading