acrites commented on code in PR #37691:
URL: https://github.com/apache/beam/pull/37691#discussion_r2978258770


##########
sdks/java/harness/src/main/java/org/apache/beam/fn/harness/control/ProcessBundleHandler.java:
##########
@@ -488,6 +492,16 @@ public BundleSplitListener getSplitListener() {
                 public BundleFinalizer getBundleFinalizer() {
                   return bundleFinalizer;
                 }
+
+                @Override
+                public Supplier<Boolean> getHasNoState() {
+                  return () -> hasNoState;

Review Comment:
   I still don't think this is quite right. `hasNoState` here is determined 
from the `ProcessBundleRequest` used when constructing the `BundleProcessor`. 
But we end up reusing these for other `ProcessBundleRequests`. You want to do 
something similar to how we handle `cacheTokens` in 
`setupForProcessBundleRequest`.
   
   It would be good to write some kind of test that might catch this error. 
Something where we call `processBundle` for two different `InstructionRequests` 
that have the same `processBundleDescriptorId` (so we reuse the same 
`BundleProcessor` from the cache, but different values of these two new fields.



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

Reply via email to