kirktrue commented on code in PR #16083:
URL: https://github.com/apache/kafka/pull/16083#discussion_r1735416616


##########
clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRebalanceListener.java:
##########
@@ -144,9 +144,9 @@ public interface ConsumerRebalanceListener {
     void onPartitionsRevoked(Collection<TopicPartition> partitions);
 
     /**
-     * A callback method the user can implement to provide handling of 
customized offsets on completion of a successful
-     * partition re-assignment. This method will be called after the partition 
re-assignment completes and before the
-     * consumer starts fetching data, and only as the result of a {@link 
Consumer#poll(java.time.Duration) poll(long)} call.
+     * A callback method the user can implement to be notified when partitions 
are assigned to this consumer.

Review Comment:
   I think that "provide handling" is fine, especially since it was used for 
`onPartitionsRevoked()`.
   
   ```suggestion
        * A callback method the user can implement to provide handling when 
partitions are assigned to this consumer.
   ```



##########
clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRebalanceListener.java:
##########
@@ -178,7 +178,7 @@ public interface ConsumerRebalanceListener {
      * For example, this function is called if a consumer's session timeout 
has expired, or if a fatal error has been
      * received indicating the consumer is no longer part of the group.
      * <p>
-     * By default it will just trigger {@link 
ConsumerRebalanceListener#onPartitionsRevoked}; for users who want to 
distinguish
+     * By default, it just triggers {@link 
ConsumerRebalanceListener#onPartitionsRevoked}; for users who want to 
distinguish

Review Comment:
   I'm not a grammar expert, but what about:
   
   ```suggestion
        * By default this function triggers {@link 
ConsumerRebalanceListener#onPartitionsRevoked}; for users who want to 
distinguish
   ```



##########
clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRebalanceListener.java:
##########
@@ -119,15 +119,15 @@
 public interface ConsumerRebalanceListener {
 
     /**
-     * A callback method the user can implement to provide handling of offset 
commits to a customized store.
+     * A callback method the user can implement to provide handling of offset 
commits **sent to** a customized store.

Review Comment:
   I'm not sure that "sent to" adds that much.
   
   Maybe:
   
   ```suggestion
        * A callback method the user can implement to provide handling of 
offset commits **stored in** a customized store.
   ```
   
   Or:
   
   ```suggestion
        * A callback method the user can implement to provide handling of 
offset commits **managed in** a customized store.
   ```



##########
clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerRebalanceListener.java:
##########
@@ -119,15 +119,15 @@
 public interface ConsumerRebalanceListener {
 
     /**
-     * A callback method the user can implement to provide handling of offset 
commits to a customized store.
+     * A callback method the user can implement to provide handling of offset 
commits **sent to** a customized store.
      * This method will be called during a rebalance operation when the 
consumer has to give up some partitions.
      * It can also be called when consumer is being closed ({@link 
KafkaConsumer#close(Duration)})
      * or is unsubscribing ({@link KafkaConsumer#unsubscribe()}).
      * It is recommended that offsets should be committed in this callback to 
either Kafka or a
      * custom offset store to prevent duplicate data.
      * <p>
      * In eager rebalancing, it will always be called at the start of a 
rebalance and after the consumer stops fetching data.
-     * In cooperative rebalancing, it will be called at the end of a rebalance 
on the set of partitions being revoked iff the set is non-empty.
+     * In cooperative rebalancing, it will be called at the end of a rebalance 
on the set of partitions being revoked if and only if the set is non-empty.

Review Comment:
   "iff" is an understood term, isn't it?



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to