You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/04/18 08:24:00 UTC
[GitHub] [pulsar] liangyepianzhou opened a new pull request, #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
liangyepianzhou opened a new pull request, #15200:
URL: https://github.com/apache/pulsar/pull/15200
### Motivation & Modification
Add brokerDeduplicationEnabled=true config to enable transaction deduplication
### 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 or label this PR directly.
Need to update docs?
- [ ] `doc-required`
(Your PR needs to update docs and you will update later)
- [ ] `no-need-doc`
(Please explain why)
- [x] `doc`
(Your PR contains doc changes)
- [ ] `doc-added`
(Docs have been already added)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Anonymitaet commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852027740
##########
site2/docs/txn-use.md:
##########
@@ -34,6 +34,12 @@ This section provides an example of how to use the transaction API to send and r
```
acknowledgmentAtBatchIndexLevelEnabled=true
```
+ If you want to enable deduplication for transaction, follow the steps below.
Review Comment:
Thanks @codelipenghui
Currently, @momo-jun is more focused on doc work and I’m more focused on doc-related PIP work
so @momo-jun , could you please take a look at this? Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852619738
##########
site2/docs/txn-use.md:
##########
@@ -20,11 +20,17 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
+ //It is a mandatory configuration, if need to guanrantee exactly-once semantics.
Review Comment:
```suggestion
//mandatory configuration, used to guarantee exactly-once semantics.
```
I think it is a must without conditions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] liangyepianzhou commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852657730
##########
site2/docs/txn-use.md:
##########
@@ -20,11 +20,17 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
+ //It is a mandatory configuration, if need to guanrantee exactly-once semantics.
Review Comment:
Oh my fault! Thanks for pointing out.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852008772
##########
site2/docs/txn-use.md:
##########
@@ -34,6 +34,12 @@ This section provides an example of how to use the transaction API to send and r
```
acknowledgmentAtBatchIndexLevelEnabled=true
```
+ If you want to enable deduplication for transaction, follow the steps below.
Review Comment:
It's a required configuration, without message deduplication, we can't support exactly-once semantic.
@Anonymitaet Please help refine this one. And the `acknowledgmentAtBatchIndexLevelEnabled` also required or optional @congbobo184
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852619738
##########
site2/docs/txn-use.md:
##########
@@ -20,11 +20,17 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
+ //It is a mandatory configuration, if need to guanrantee exactly-once semantics.
Review Comment:
```suggestion
//It is a mandatory configuration if you want to guarantee exactly-once semantics.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] congbobo184 commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
congbobo184 commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852938522
##########
site2/docs/txn-use.md:
##########
@@ -20,21 +20,29 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
```
- If you want to enable batch messages in transactions, follow the steps below.
+ * If you want to enable batch messages in transactions, follow the steps below.
Review Comment:
If you want to ack batch message with transaction should enable acknowledgmentAtBatchIndexLevelEnabled, follow the steps below.
##########
site2/docs/txn-use.md:
##########
@@ -20,21 +20,29 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
```
- If you want to enable batch messages in transactions, follow the steps below.
+ * If you want to enable batch messages in transactions, follow the steps below.
Set `acknowledgmentAtBatchIndexLevelEnabled` to `true` in the `broker.conf` or `standalone.conf` file.
```
acknowledgmentAtBatchIndexLevelEnabled=true
```
+ * [message deduplication](cookbooks-deduplication.md) is a mandatory configuration, if need to guarantee exactly-once semantics.
+ You can enable message deduplication at the broker level, the namespace level, or the topic level according to your needs.
Review Comment:
the namespace level or the topic level according to your needs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852085838
##########
site2/docs/txn-use.md:
##########
@@ -34,6 +34,12 @@ This section provides an example of how to use the transaction API to send and r
```
acknowledgmentAtBatchIndexLevelEnabled=true
```
+ If you want to enable deduplication for transaction, follow the steps below.
Review Comment:
Since it's a mandatory setting to enable transactions, I think it can be added to row28 following `systemTopicEnabled=true`.
The following descriptions can be added:
* `systemTopicEnabled=true` is a mandatory configuration. Otherwise, transactions cannot work because transaction buffer snapshots can’t be created.
* `brokerDeduplicationEnabled=true` is a mandatory configuration to guanrantee exactly-once semantics.
BTW, does it make sense to add `standalone.conf` to row23?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] liangyepianzhou commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852944587
##########
site2/docs/txn-use.md:
##########
@@ -20,21 +20,29 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
```
- If you want to enable batch messages in transactions, follow the steps below.
+ * If you want to enable batch messages in transactions, follow the steps below.
Set `acknowledgmentAtBatchIndexLevelEnabled` to `true` in the `broker.conf` or `standalone.conf` file.
```
acknowledgmentAtBatchIndexLevelEnabled=true
```
+ * [message deduplication](cookbooks-deduplication.md) is a mandatory configuration, if need to guarantee exactly-once semantics.
+ You can enable message deduplication at the broker level, the namespace level, or the topic level according to your needs.
Review Comment:
@momo-jun Please help review this, thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852952661
##########
site2/docs/txn-use.md:
##########
@@ -20,21 +20,29 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
```
- If you want to enable batch messages in transactions, follow the steps below.
+ * If you want to enable batch messages in transactions, follow the steps below.
Set `acknowledgmentAtBatchIndexLevelEnabled` to `true` in the `broker.conf` or `standalone.conf` file.
```
acknowledgmentAtBatchIndexLevelEnabled=true
```
+ * [message deduplication](cookbooks-deduplication.md) is a mandatory configuration, if need to guarantee exactly-once semantics.
Review Comment:
```suggestion
* If you want to guarantee exactly-once semantics, you need to enable [message deduplication](cookbooks-deduplication.md).
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] liangyepianzhou commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852637217
##########
site2/docs/txn-use.md:
##########
@@ -20,11 +20,17 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
+ //It is a mandatory configuration, if need to guanrantee exactly-once semantics.
Review Comment:
If the user does not need to turn on deduplication, then this does not need to be turned on. Otherwise it's just a waste of performance
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Technoboy- merged pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
Technoboy- merged PR #15200:
URL: https://github.com/apache/pulsar/pull/15200
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] liangyepianzhou commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852092168
##########
site2/docs/txn-use.md:
##########
@@ -34,6 +34,12 @@ This section provides an example of how to use the transaction API to send and r
```
acknowledgmentAtBatchIndexLevelEnabled=true
```
+ If you want to enable deduplication for transaction, follow the steps below.
Review Comment:
Great suggestion, I will write these configuration together and add descriptions for them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] liangyepianzhou commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
liangyepianzhou commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852981158
##########
site2/docs/txn-use.md:
##########
@@ -20,21 +20,29 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
```
- If you want to enable batch messages in transactions, follow the steps below.
+ * If you want to enable batch messages in transactions, follow the steps below.
Set `acknowledgmentAtBatchIndexLevelEnabled` to `true` in the `broker.conf` or `standalone.conf` file.
```
acknowledgmentAtBatchIndexLevelEnabled=true
```
+ * [message deduplication](cookbooks-deduplication.md) is a mandatory configuration, if need to guarantee exactly-once semantics.
Review Comment:
Can you help review the suggestions given by congBo.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] momo-jun commented on a diff in pull request #15200: [Improve][doc][txn] add brokerDeduplicationEnabled=true to enable transaction deduplication
Posted by GitBox <gi...@apache.org>.
momo-jun commented on code in PR #15200:
URL: https://github.com/apache/pulsar/pull/15200#discussion_r852654408
##########
site2/docs/txn-use.md:
##########
@@ -20,11 +20,17 @@ This section provides an example of how to use the transaction API to send and r
2. Enable transaction.
- Change the configuration in the `broker.conf` file.
+ Change the configuration in the `broker.conf` or `standalone.conf` file.
```
+ //mandatory configuration, used to enable transaction coordinator
transactionCoordinatorEnabled=true
+
+ //mandtory configuration, used to create systemTopic used for transaction buffer snapshot
systemTopicEnabled=true
+
+ //It is a mandatory configuration, if need to guanrantee exactly-once semantics.
Review Comment:
Thanks for clarifying this. Then there will only be a typo fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org