Abacn commented on code in PR #24403:
URL: https://github.com/apache/beam/pull/24403#discussion_r1036312432
##########
sdks/java/io/google-cloud-platform/src/test/java/org/apache/beam/sdk/io/gcp/datastore/DatastoreV1Test.java:
##########
@@ -651,6 +652,45 @@ public void testDatatoreWriterFnWithLargeEntities() throws
Exception {
}
}
+ /** Tests {@link DatastoreWriterFn} correctly flushes batch upon receive
same entity keys. */
+ @Test
+ public void testDatatoreWriterFnWithDuplicateEntities() throws Exception {
Review Comment:
Thanks @damondouglas! This unit test is exactly `replicates the conditions
that lead to the Exception reported in #18521 that we can validate resolves by
the changes in DatastoreV1.java.` Though it is running on mock datastore
client. Prior to the change the mutation sent to datastore would have keys `{0,
1, 0, 2}` and cause the exception because there are duplicated keys in a single
commit. Now it send two commits with keys `{0, 1}` and `{0, 2}` and would not
cause exception.
The RampupThrottlingFnFnTest change is because originally the mutations are
having all the same (Void), but due to that the duplication did not work
properly, a single commit will be called. This is a bad test case because for
real datastore (not mock), this would throw exception. Now I just use a
mutation with random elements to let the test works properly.
--
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]
