jaketf commented on a change in pull request #11450:
URL: https://github.com/apache/beam/pull/11450#discussion_r415997894



##########
File path: 
sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/healthcare/HttpHealthcareApiClient.java
##########
@@ -127,6 +114,7 @@ public ListMessagesResponse makeHL7v2ListRequest(
             .messages()
             .list(hl7v2Store)
             .set("view", "full")
+            .setPageSize(1000)

Review comment:
       This is not a sleep. 
   I think this is a comment on the wrong line. 
   
   Currently the 
[approach](https://github.com/apache/beam/pull/11450/files#diff-bd93c0439e90fe2da58109fb639c71b1R97-R123)
 I've taken is to retry listing of HL7v2 messages until the desired number of 
messages is returned with EBO and an over all timeout of 10 minutes.
   This is very different than a 10 minute sleep as it's expected to succeed 
well under 10 mins.
   I'm pretty sure this is an extremely over kill timeout for indexing 3 
messages.
   I've asked the internal team about stats we have on this async indexing 
process to increase confidence here.
   
   I'm not sure how to move this out of the post commit test suite.
   
   So I have some questions:
   1. What would an acceptable timeout be to keep this in the post commit?
   1. If I were to run this test 1000x on a VM in the same region as the 
jenkins VMs with the contents of this PR to prove that it fixes the flakiness, 
is there additional stats (beyond 1000/1000 runs pass) you'd find helpful (e.g. 
distribution of total runtime for this test)?
   1. How to move this to a "sick bay" or other test suite? Does this already 
exist in beam code base?




----------------------------------------------------------------
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:
[email protected]


Reply via email to