Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feat][txn]Implement transactional consumer/producer API #1002

Merged
merged 8 commits into from
May 6, 2023
Merged

Conversation

liangyepianzhou
Copy link
Contributor

Master Issue:#932

Motivation

Implement transaction coordinator client.

Modifications

  1. Implement transaction coordinator
  2. implement transactionImpl
  3. Implement transaction in producer and consumer API

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)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / GoDocs / 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 followup issue for adding the documentation

@shibd
Copy link
Member

shibd commented Apr 10, 2023

Is this the last PR about transactions? It's better to add some sample code to examples.

@liangyepianzhou
Copy link
Contributor Author

Is this the last PR about transactions? It's better to add some sample code to examples.

Yes, this is the last PR concerning transactions. In the tests I've added, I've only used the API to perform the tests, which can serve as an initial example. Adding more detailed examples to the documentation might be more appropriate?

@shibd
Copy link
Member

shibd commented Apr 10, 2023

Adding more detailed examples to the documentation might be more appropriate?

Yes, the documentation can be described step by step, and complete examples are placed in the examples folder. We can push in a later PR.

pulsar/client_impl.go Show resolved Hide resolved
pulsar/consumer_partition.go Outdated Show resolved Hide resolved
pulsar/producer_partition.go Outdated Show resolved Hide resolved
pulsar/producer_partition.go Show resolved Hide resolved
@Anonymitaet
Copy link
Member

Hi @liangyepianzhou thanks for introducing this great feature!

========================================

As we discussed just now, for the doc side:

✅ Pulsar feature matrix

I've ticked the three boxes (client, producer, consumer) of "Transactions" for Go.

✅ Pulsar client docs

  1. Could you please provide the tech inputs as below?

    Add Go code example to Transactions > Get started .

image

  1. This feature is available from Pulsar 2.9.5, can we add the doc to 4 doc versions (NEXT, 2.11.x, 2.10.x, 2.9.x) or just indicate what version it supports?

    • For 2.9.x: we can add a tip to the 2.9.x doc and indicate that this feature is available from (or in) Pulsar 2.9.5.
    • For 2.11.x and 2.10.x: please see the questions below.

image

BTW,

  • Original doc file of NEXT is here.
  • Original doc files of all versioned docs are here.

========================================

Feel free to reach out to me if you have questions. TIA!

@liangyepianzhou
Copy link
Contributor Author

Could you please retake a look? @BewareMyPower @shibd

@shibd
Copy link
Member

shibd commented May 5, 2023

Could you please retake a look? @BewareMyPower @shibd

Sorry, I'll take a look at it today.

Copy link
Member

@shibd shibd left a comment

Choose a reason for hiding this comment

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

Overall LGTM, left some small comments.

pulsar/consumer_partition.go Show resolved Hide resolved
pulsar/transaction_test.go Show resolved Hide resolved
pulsar/transaction_test.go Show resolved Hide resolved
@liangyepianzhou liangyepianzhou requested a review from shibd May 5, 2023 16:13
@shibd shibd merged commit 2437caa into master May 6, 2023
@BewareMyPower BewareMyPower deleted the txn_API branch May 6, 2023 09:45
RobertIndie added a commit to apache/pulsar-site that referenced this pull request Jul 2, 2024
## Motivation

The go client has already supported the transaction. apache/pulsar-client-go#1002
But the client feature matrix doesn't reflect this.

This PR marks the transaction support for go client feature matrix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants