Anonymitaet commented on code in PR #15368:
URL: https://github.com/apache/pulsar/pull/15368#discussion_r861427010


##########
site2/docs/cookbooks-deduplication.md:
##########
@@ -4,13 +4,15 @@ title: Message deduplication
 sidebar_label: Message deduplication 
 ---
 
-When **Message deduplication** is enabled, it ensures that each message 
produced on Pulsar topics is persisted to disk *only once*, even if the message 
is produced more than once. Message deduplication is handled automatically on 
the server side. 
+When **Message deduplication** is enabled, it ensures that each message 
produced on Pulsar topics is persisted to disk *only once*, even if the message 
is produced more than once. Message deduplication is handled automatically on 
the broker side. 
 
-To use message deduplication in Pulsar, you need to configure your Pulsar 
brokers and clients.
+Message deduplication could affect performance in the brokers during 
informational snapshot.

Review Comment:
   ```suggestion
   Message deduplication could affect the performance of the brokers during 
informational snapshots.
   ```



##########
site2/docs/cookbooks-deduplication.md:
##########
@@ -4,13 +4,15 @@ title: Message deduplication
 sidebar_label: Message deduplication 
 ---
 
-When **Message deduplication** is enabled, it ensures that each message 
produced on Pulsar topics is persisted to disk *only once*, even if the message 
is produced more than once. Message deduplication is handled automatically on 
the server side. 
+When **Message deduplication** is enabled, it ensures that each message 
produced on Pulsar topics is persisted to disk *only once*, even if the message 
is produced more than once. Message deduplication is handled automatically on 
the broker side. 
 
-To use message deduplication in Pulsar, you need to configure your Pulsar 
brokers and clients.
+Message deduplication could affect performance in the brokers during 
informational snapshot.
+
+To use message deduplication in Pulsar, you need to configure your Pulsar 
brokers, namespaces or topics and it is recommended to modify configuration in 
the clients, setting send timeout to infinity.

Review Comment:
   ```suggestion
   To use message deduplication in Pulsar, you need to configure your Pulsar 
brokers, namespaces, or topics. It is recommended to modify the configuration 
in the clients, for example, setting send timeout to infinity.
   ```
   



##########
site2/docs/cookbooks-deduplication.md:
##########
@@ -70,7 +74,7 @@ The instructions for Java, Python, and C++ clients are 
different.
 <!--DOCUSAURUS_CODE_TABS-->
 <!--Java clients-->
 
-To enable message deduplication on a [Java 
producer](client-libraries-java.md#producers), set the producer name using the 
`producerName` setter, and set the timeout to `0` using the `sendTimeout` 
setter. 
+Not to break the guarantee order on a [Java 
producer](client-libraries-java.md#producers) sending to a topic with message 
deduplication active, set the producer name using the `producerName` setter, 
and set the timeout to `0` using the `sendTimeout` setter. 

Review Comment:
   what is the meaning of "Not to break the guarantee order on a Java producer 
sending to a topic with message deduplication active"? 
   
   what are the logical relationships between the 3 parts of this whole 
sentence?
   
   would u mind re-writing this sentence? Thanks



##########
site2/docs/cookbooks-deduplication.md:
##########
@@ -4,13 +4,15 @@ title: Message deduplication
 sidebar_label: Message deduplication 
 ---
 
-When **Message deduplication** is enabled, it ensures that each message 
produced on Pulsar topics is persisted to disk *only once*, even if the message 
is produced more than once. Message deduplication is handled automatically on 
the server side. 
+When **Message deduplication** is enabled, it ensures that each message 
produced on Pulsar topics is persisted to disk *only once*, even if the message 
is produced more than once. Message deduplication is handled automatically on 
the broker side. 
 
-To use message deduplication in Pulsar, you need to configure your Pulsar 
brokers and clients.
+Message deduplication could affect performance in the brokers during 
informational snapshot.
+
+To use message deduplication in Pulsar, you need to configure your Pulsar 
brokers, namespaces or topics and it is recommended to modify configuration in 
the clients, setting send timeout to infinity.
 
 ## How it works
 
-You can enable or disable message deduplication at the namespace level or the 
topic level. By default, it is disabled on all namespaces or topics. You can 
enable it in the following ways:
+You can enable or disable message deduplication at broker, namespace or topic 
level. By default, it is disabled on all brokers, namespaces or topics. You can 
enable it in the following ways:

Review Comment:
   ```suggestion
   You can enable or disable message deduplication at broker, namespace, or 
topic level. By default, it is disabled on all brokers, namespaces, or topics. 
You can enable it in the following ways:
   ```



##########
site2/docs/cookbooks-deduplication.md:
##########
@@ -60,7 +62,9 @@ $ bin/pulsar-admin namespaces set-deduplication \
 
 ## Pulsar clients
 
-If you enable message deduplication in Pulsar brokers, you need complete the 
following tasks for your client producers:
+If you enable message deduplication in Pulsar brokers, namespaces or topics, 
it is recommended to make the client retry infinitely the messages until it 
succeed, otherwise is possible to break the ordering guarantee as some request 
can time out and the application wont know if the request was successfully 
added to the topic or not. 

Review Comment:
   ```suggestion
   If you enable message deduplication in Pulsar brokers, namespaces, or 
topics, it is recommended to make the client retry infinitely the messages 
until it succeeds, otherwise it is possible to break the ordering guarantee as 
some requests may time out and the application does not know whether the 
request is successfully added to the topic or not. 
   ```
   
   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



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to