beam-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [beam] jfarr commented on a change in pull request #11090: [BEAM-9470] :sdks:java:io:kinesis:test is flaky
Date Thu, 12 Mar 2020 03:57:14 GMT
jfarr commented on a change in pull request #11090: [BEAM-9470] :sdks:java:io:kinesis:test
is flaky
URL: https://github.com/apache/beam/pull/11090#discussion_r391389538
 
 

 ##########
 File path: sdks/java/io/kinesis/src/test/java/org/apache/beam/sdk/io/kinesis/ShardReadersPoolTest.java
 ##########
 @@ -324,10 +325,15 @@ public void shouldCallRateLimitPolicy()
       }
     }
 
+    ArgumentCaptor<List<KinesisRecord>> recordsCaptor = ArgumentCaptor.forClass(List.class);
+    verify(customRateLimitPolicy, atLeastOnce()).onSuccess(recordsCaptor.capture());
+    List<List<KinesisRecord>> capturedRecords = recordsCaptor.getAllValues();
+    assertThat(capturedRecords).contains(
+        ImmutableList.of(a, b),
+        singletonList(c),
+        singletonList(d),
+        Collections.emptyList()
+    );
     verify(customRateLimitPolicy).onThrottle(same(e));
-    verify(customRateLimitPolicy).onSuccess(eq(ImmutableList.of(a, b)));
-    verify(customRateLimitPolicy).onSuccess(eq(singletonList(c)));
-    verify(customRateLimitPolicy).onSuccess(eq(singletonList(d)));
 
 Review comment:
   @suztomo You have a good point. The intention of this test is to validate that every time
readNextBatch() returns a list of records onSuccess() gets called and every time it throws
`KinesisClientThrottledException` onThrottle() gets called. I can see a couple of code paths
where readNextBatch() could return records without onSuccess() getting called, namely if any
exceptions are thrown along in here: https://github.com/apache/beam/blob/d62521f69ead4b58924043f041978d49e9beeb62/sdks/java/io/kinesis/src/main/java/org/apache/beam/sdk/io/kinesis/ShardReadersPool.java#L146-L149
   
   I haven't seen evidence that this is what's happening, but let's try my latest changes
in ShardReadersPool and see if that fixes it. With this change it should not be possible for
readNextBatch() to return without onSuccess() being called.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message