[GitHub] [beam] lukecwik commented on a change in pull request #12639: [BEAM-9979] Fix read index being reported to be reset after any metrics request could happen for the prior bundle and before any metrics request for the next bundle.

[GitBox <gi...@apache.org>]

lukecwik commented on a change in pull request #12639:
URL: https://github.com/apache/beam/pull/12639#discussion_r474104405



##########
File path: sdks/java/harness/src/main/java/org/apache/beam/fn/harness/BeamFnDataReadRunner.java
##########
@@ -237,8 +232,9 @@ public void trySplit(
     }
 
     synchronized (splittingLock) {
-      // Don't attempt to split if we haven't started.
-      if (!started) {
+      // Don't attempt to split if we are already done since there isn't a meaningful split we can
+      // provide.
+      if (index == stopIndex) {

Review comment:
       No, the logic below will allow us to choose a stopIndex even if `registerInputLocation` has not been invoked. See `testSplittingWhenNoElementsProcessed`. I added a variant of that test where we split before `registerInputLocation` happens to get coverage for this case.




----------------------------------------------------------------
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