You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/01/17 06:41:45 UTC

[GitHub] [druid] kfaraz commented on a change in pull request #12161: Mitigate Kinesis stream LimitExceededException by using listShards API

kfaraz commented on a change in pull request #12161:
URL: https://github.com/apache/druid/pull/12161#discussion_r785669686



##########
File path: integration-tests/src/main/java/org/apache/druid/testing/utils/KinesisAdminClient.java
##########
@@ -119,13 +126,14 @@ public void updatePartitionCount(String streamName, int newShardCount, boolean b
       // Wait until the resharding started (or finished)
       ITRetryUtil.retryUntil(
           () -> {
-            StreamDescription streamDescription = getStreamDescription(streamName);
-            int updatedShardCount = getStreamShardCount(streamDescription);
-            return verifyStreamStatus(streamDescription, StreamStatus.UPDATING) ||
-                (verifyStreamStatus(streamDescription, StreamStatus.ACTIVE) && updatedShardCount > originalShardCount);
+            String streamStatus = getStreamStatus(streamName);
+            int updatedShardCount = getStreamPartitionCount(streamName);
+            return (streamStatus.equals(StreamStatus.UPDATING.toString()) ||
+                    streamStatus.equals(StreamStatus.ACTIVE.toString()))

Review comment:
       It seems that the position of the braces has changed.
   Please confirm if this is intentional.
   
   Original is: `stream is updating OR (stream is active and updated count > orig count)`
   New is: `(stream is updating OR stream is active) and updated count > orig count`

##########
File path: integration-tests/src/main/java/org/apache/druid/testing/utils/KinesisAdminClient.java
##########
@@ -156,15 +162,16 @@ public boolean verfiyPartitionCountUpdated(String streamName, int oldShardCount,
     return actualShardCount == oldShardCount + newShardCount;
   }
 
-
-  private boolean verifyStreamStatus(StreamDescription streamDescription, StreamStatus streamStatusToCheck)

Review comment:
       You could retain a modified `verifyStreamStatus` method instead of the doing the String.equals() everywhere.




-- 
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@druid.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org