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

[improve] [pip] PIP-298 Consumer supports specifying consumption isolation level #21114

Merged
merged 4 commits into from
Oct 24, 2023

Conversation

hzh0425
Copy link
Member

@hzh0425 hzh0425 commented Sep 3, 2023

Motivation

This pip is to implement Read Committed and Read Uncommitted isolation levels for Pulsar transactions, allow consumers to configure isolation levels during the building process.
For more details, please refer to pip-298.md

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
  • The metrics
  • Anything that affects deployment

Documentation

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

Matching PR in forked repository

PR in forked repository:

@github-actions
Copy link

github-actions bot commented Sep 3, 2023

@hzh0425 Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@hzh0425 hzh0425 changed the title [improve] [pip] PIP [improve] [pip] PIP-298 Consumer supports specifying consumption isolation level Sep 3, 2023
@hzh0425 hzh0425 reopened this Sep 3, 2023
@hzh0425
Copy link
Member Author

hzh0425 commented Sep 3, 2023

@liangyepianzhou pls cc

pip/pip-298.md Outdated Show resolved Hide resolved
@liangyepianzhou
Copy link
Contributor

Leave an example to illustrate the usage scenario.

Operations: Large volume of deposit and withdrawal operations, a
small number of transfer operations.

Roles:

  • Client A1
  • Client A2
  • User Account B1
  • User Account B2
  • Request Topic C
  • Real-time Monitoring System D
  • Business Processing System E

Client Operations:

  • Withdrawal: Client A1 decreases the deposit amount from User
    Account B1 or B2.
  • Deposit: Client A1 increases the deposit amount in User Account B1 or B2.
  • Transfer: Client A2 decreases the deposit amount from User
    Account B1 and increases it in User Account B2. Or vice versa.

Real-time Monitoring System D: Obtains the latest data from
Request Topic C as quickly as possible to monitor transaction data and
changes in bank reserves in real-time. This is necessary for the
timely detection of anomalies and real-time decision-making.

Business Processing System E: Reads data from Request Topic C,
then actually operates User Accounts B1, B2.

User Scenario: Client A1 sends a large number of deposit and
withdrawal requests to Request Topic C. Client A2 writes a small
number of transfer requests to Request Topic C.

In this case, Business Processing System E needs a read-committed
isolation level to ensure operation consistency and Exactly Once
semantics. The real-time monitoring system does not care if a small
number of transfer requests are incomplete (dirty data). What it
cannot tolerate is a situation where a large number of deposit and
withdrawal requests cannot be presented in real time due to a small
number of transfer requests (the current situation is that uncommitted
transaction messages can block the reading of committed transaction
messages).

In this case, it is necessary to set different isolation levels for
different consumers/subscriptions.

pip/pip-298.md Show resolved Hide resolved
pip/pip-298.md Outdated

## Reject Alternatives

None
Copy link
Contributor

Choose a reason for hiding this comment

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

In the original template there is "# Monitoring".
One aspect of monitoring IMO is to know for each subscription, it's isolation level. Can you please describe how users will be able to know that? What is changing in "## Public-facing Changes״ ׳ which allows me to know that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @asafm , Thank you for your suggestion.
According to your suggestion, I have made the following two improvements:

  1. Added a new section ##Public-facing Changes to describe changes to the API.
  2. Added a new #Monitor section. Users can monitor the newly added subscriptionStats.subscriptionIsolationLevel through the admin tool.
    When you are free, please help and take a look again

## API Changes
# High Level Design

Add a configuration 'subscriptionIsolationLevel' in the consumer builder to allow users to choose different transaction
Copy link
Contributor

Choose a reason for hiding this comment

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

@codelipenghui Do we keep a table of client features each SDK needs to implement?

Copy link
Contributor

Choose a reason for hiding this comment

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


- If Subscription.isolationLevel == ReadUnCommitted, then MaxReadPosition = PositionImpl.LATEST
# Monitoring
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good

@hzh0425
Copy link
Member Author

hzh0425 commented Oct 8, 2023

@dave2wave hi dave, Could you pls take a look for this pip when you are free?
The document is: apache/pulsar-site#712

@liangyepianzhou liangyepianzhou merged commit 618aede into apache:master Oct 24, 2023
19 checks passed
@shibd
Copy link
Member

shibd commented May 27, 2024

This PIP is not implemented, right?

Why did the PR of the document be merged? apache/pulsar-site#712

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants