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/06/14 11:23:48 UTC
[GitHub] [pulsar] nahguam opened a new pull request, #16057: [improve][docs] Improve examples and language
nahguam opened a new pull request, #16057:
URL: https://github.com/apache/pulsar/pull/16057
<!--
### Contribution Checklist
- PR title format should be *[type][component] summary*. For details, see *[Guideline - Pulsar PR Naming Convention](https://docs.google.com/document/d/1d8Pw6ZbWk-_pCKdOmdvx9rnhPiyuxwq60_TrD68d7BA/edit#heading=h.trs9rsex3xom)*.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- Each pull request should address only one issue, not mix up code from multiple issues.
- Each commit in the pull request has a meaningful commit message
- Once all items of the checklist are addressed, remove the above text and this checklist, leaving only the filled out template below.
**(The sections below can be removed for hotfixes of typos)**
-->
*(If this PR fixes a github issue, please add `Fixes #<xyz>`.)*
Fixes #<xyz>
*(or if this PR is one task of a github issue, please add `Master Issue: #<xyz>` to link to the master issue.)*
Master Issue: #<xyz>
### Motivation
*Explain here the context, and why you're making that change. What is the problem you're trying to solve.*
### Modifications
*Describe the modifications you've done.*
### 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)
- [ ] `doc-not-needed`
(Please explain why)
- [ ] `doc`
(Your PR contains doc changes)
- [ ] `doc-complete`
(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 pull request #16057: [improve][docs] Improve examples and language
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on PR #16057:
URL: https://github.com/apache/pulsar/pull/16057#issuecomment-1155901333
@tjiuming
can you review this PR from the technical perspective? 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] Anonymitaet commented on a diff in pull request #16057: [improve][docs] Improve examples and language
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16057:
URL: https://github.com/apache/pulsar/pull/16057#discussion_r897460787
##########
site2/docs/concepts-messaging.md:
##########
@@ -102,7 +102,7 @@ You can compress messages published by producers during transportation. Pulsar c
When batching is enabled, the producer accumulates and sends a batch of messages in a single request. The batch size is defined by the maximum number of messages and the maximum publish latency. Therefore, the backlog size represents the total number of batches instead of the total number of messages.
-In Pulsar, batches are tracked and stored as single units rather than as individual messages. Consumer unbundles a batch into individual messages. However, scheduled messages (configured through the `deliverAt` or the `deliverAfter` parameter) are always sent as individual messages even batching is enabled.
+In Pulsar, batches are tracked and stored as single units rather than as individual messages. Consumers will unbundle a batch into individual messages. However, scheduled messages (configured through the `deliverAt` or the `deliverAfter` parameter) are always sent as individual messages even when batching is enabled.
Review Comment:
```suggestion
In Pulsar, batches are tracked and stored as single units rather than as individual messages. Consumers unbundle a batch into individual messages. However, scheduled messages (configured through the `deliverAt` or the `deliverAfter` parameter) are always sent as individual messages even when batching is enabled.
```
Write in the simple present tense as much as possible if you are covering facts that were, are, and forever shall be true. https://docs.google.com/document/d/1lc5j4RtuLIzlEYCBo97AC8-U_3Erzs_lxpkDuseU0n4/edit#bookmark=id.e8uqh1awkcnp
##########
site2/docs/concepts-messaging.md:
##########
@@ -582,7 +585,7 @@ In the diagram below, **Consumer-C-1** and **Consumer-C-2** are able to subscrib
#### Key_Shared
-In *Key_Shared* type, multiple consumers can attach to the same subscription. Messages are delivered in a distribution across consumers and message with same key or same ordering key are delivered to only one consumer. No matter how many times the message is re-delivered, it is delivered to the same consumer. When a consumer connected or disconnected will cause served consumer change for some key of message.
+In *Key_Shared* type, multiple consumers can attach to the same subscription. Messages are delivered in a distribution across consumers and messages with the same key or same ordering key are delivered to only one consumer. No matter how many times the message is re-delivered, it is delivered to the same consumer. When a consumer connects or disconnects it will cause the served consumer to change for some message keys.
Review Comment:
```suggestion
In *Key_Shared* type, multiple consumers can attach to the same subscription. Messages are delivered in a distribution across consumers and messages with the same key or same ordering key are delivered to only one consumer. No matter how many times the message is re-delivered, it is delivered to the same consumer. When a consumer connects or disconnects, it causes the served consumer to change some message keys.
```
is this correct?
--
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] nahguam commented on a diff in pull request #16057: [improve][docs] Improve examples and language
Posted by GitBox <gi...@apache.org>.
nahguam commented on code in PR #16057:
URL: https://github.com/apache/pulsar/pull/16057#discussion_r897747200
##########
site2/docs/concepts-messaging.md:
##########
@@ -263,14 +264,15 @@ The message redelivery behavior should be as follows.
Redelivery count | Redelivery delay
:--------------------|:-----------
-1 | 10 + 1 seconds
Review Comment:
The 2nd set of examples are for the ack timeout redelivery delay. It makes sense that the overall delay is the timeout (10s) + backoff. But here the examples are for the negative ack redelivery delay which should just be the backoff time from the negative ack if I understand correctly. Hence why I've removed the `10 +`.
--
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 merged pull request #16057: [improve][docs] Improve examples and language
Posted by GitBox <gi...@apache.org>.
Anonymitaet merged PR #16057:
URL: https://github.com/apache/pulsar/pull/16057
--
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] nahguam commented on a diff in pull request #16057: [improve][docs] Improve examples and language
Posted by GitBox <gi...@apache.org>.
nahguam commented on code in PR #16057:
URL: https://github.com/apache/pulsar/pull/16057#discussion_r897747200
##########
site2/docs/concepts-messaging.md:
##########
@@ -263,14 +264,15 @@ The message redelivery behavior should be as follows.
Redelivery count | Redelivery delay
:--------------------|:-----------
-1 | 10 + 1 seconds
Review Comment:
The 2nd set of examples are for the ack timeout redelviery delivery delay. It makes sense that the overall delay is timeout (10s) + backoff). But here the examples are for the negative ack redelivery delay which should just be the backoff time from the negative ack if I understand correctly. Hence why I've removed the `10 +`.
--
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 #16057: [improve][docs] Improve examples and language
Posted by GitBox <gi...@apache.org>.
Anonymitaet commented on code in PR #16057:
URL: https://github.com/apache/pulsar/pull/16057#discussion_r898589413
##########
site2/docs/concepts-messaging.md:
##########
@@ -263,14 +264,15 @@ The message redelivery behavior should be as follows.
Redelivery count | Redelivery delay
:--------------------|:-----------
-1 | 10 + 1 seconds
Review Comment:
@tjiuming any feedback on this?
--
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] nahguam commented on pull request #16057: [improve][docs] Improve examples and language
Posted by GitBox <gi...@apache.org>.
nahguam commented on PR #16057:
URL: https://github.com/apache/pulsar/pull/16057#issuecomment-1156219231
Thanks for the suggestions @Anonymitaet and for pointing me to the style guide :)
--
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