jsancio commented on code in PR #13555:
URL: https://github.com/apache/kafka/pull/13555#discussion_r1172839235
##########
server-common/src/main/java/org/apache/kafka/deferred/DeferredEvent.java:
##########
@@ -15,12 +15,12 @@
* limitations under the License.
*/
-package org.apache.kafka.controller;
+package org.apache.kafka.deferred;
/**
* Represents a deferred event in the controller purgatory.
Review Comment:
Let's fix this comment. Maybe:
> Represents a deferred event in the {@code DeferredEventQueue}.
##########
server-common/src/main/java/org/apache/kafka/deferred/DeferredEventQueue.java:
##########
@@ -77,13 +76,13 @@ void failAll(Exception exception) {
* @param offset The offset to add the new event at.
Review Comment:
Above we have:
> Add a new purgatory event.
how about:
> Add a new deferred event to be completed by the provided offset.
##########
server-common/src/main/java/org/apache/kafka/deferred/DeferredEventQueue.java:
##########
@@ -77,13 +76,13 @@ void failAll(Exception exception) {
* @param offset The offset to add the new event at.
* @param event The new event.
*/
- void add(long offset, DeferredEvent event) {
+ public void add(long offset, DeferredEvent event) {
if (!pending.isEmpty()) {
long lastKey = pending.lastKey();
if (offset < lastKey) {
throw new RuntimeException("There is already a purgatory event
with " +
Review Comment:
How about throwing an `IllegalArgumentException` instead of
`RuntimeException` with the following message:
> There is already a deferred event with offset ...
##########
server-common/src/main/java/org/apache/kafka/deferred/DeferredEventQueue.java:
##########
@@ -60,7 +59,7 @@ void completeUpTo(long offset) {
*
Review Comment:
The line above has
> Fail all the pending purgatory entries.
how about:
> Fail all deferred events with the provided exception.
--
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
