AndrewJSchofield commented on code in PR #15574:
URL: https://github.com/apache/kafka/pull/15574#discussion_r1537251855
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -143,8 +143,8 @@
* <p>
* The <code>buffer.memory</code> controls the total amount of memory
available to the producer for buffering. If records
* are sent faster than they can be transmitted to the server then this buffer
space will be exhausted. When the buffer space is
- * exhausted additional send calls will block. The threshold for time to block
is determined by <code>max.block.ms</code> after which it throws
- * a TimeoutException.
+ * exhausted additional send calls will block. The threshold for time to block
is determined by <code>max.block.ms</code> after which it returns
+ * failed future with BufferExhaustedException.
Review Comment:
"a failed future...".
##########
clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java:
##########
@@ -201,7 +201,7 @@ public class ProducerConfig extends AbstractConfig {
/** <code>buffer.memory</code> */
public static final String BUFFER_MEMORY_CONFIG = "buffer.memory";
private static final String BUFFER_MEMORY_DOC = "The total bytes of memory
the producer can use to buffer records waiting to be sent to the server. If
records are "
- + "sent faster than they
can be delivered to the server the producer will block for <code>" +
MAX_BLOCK_MS_CONFIG + "</code> after which it will throw an exception."
+ + "sent faster than they
can be delivered to the server the producer will block for <code>" +
MAX_BLOCK_MS_CONFIG + "</code> after which it will end up with failed future."
Review Comment:
I'm not convinced by the description here. I know that some errors are
reported as using futures, but I think that "fail with an exception" is
probably better. If the interface in question uses a future, then that is
reported as a failed future. If it's a callback, it's reported as a parameter
on the callback. If it's a synchronous method, it's thrown. I recommend "fail
with an exception" as a way of being accurate and also general at the same time.
##########
clients/src/main/java/org/apache/kafka/clients/producer/Callback.java:
##########
@@ -52,6 +54,7 @@ public interface Callback {
* <li>{@link
org.apache.kafka.common.errors.OffsetOutOfRangeException
OffsetOutOfRangeException}
* <li>{@link
org.apache.kafka.common.errors.TimeoutException TimeoutException}
* <li>{@link
org.apache.kafka.common.errors.UnknownTopicOrPartitionException
UnknownTopicOrPartitionException}
+ * <li>{@link BufferExhaustedException
BufferExhaustedException}
Review Comment:
For consistency, you should use the full
`org.apache.kafka.clients.producer.BufferExhaustedException` class name.
--
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]
