You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/09/22 22:48:14 UTC

[GitHub] [beam] lostluck opened a new issue, #23335: [Task][Go SDK]: Respond to Control FnAPI messages per-Bundle ordering, not globally.

lostluck opened a new issue, #23335:
URL: https://github.com/apache/beam/issues/23335

   ### What needs to happen?
   
   At present the Go SDK harness responds to FnAPI Control in order of receipt, other than ProcessBundleRequests which are processed in their own thread.
   
   This may be causing contention on the control GRPC thread as all other messages are happening sequentially as it's also the "grpc" receipt thread. This includes handling "keepalive" responses on the GRPC connection and other maintenance messages.
   
   https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/harness/harness.go#L168
   
   Per Bundle Processing spec (Link TBD), the response ordering only needs to be strictly ordered based on the specific bundle instruction. Eg. requests to split, get progress, etc.
   
   A simple solution A would be to remove the response processing from the network thread, allowing that to drain, and having a separate processing thread. This avoids contention from the network thread to some buffered channel limit.
   
   A more elaborate solution B would be to spawn an additional "side car" goroutine for each in flight bundle to handle these ordered messages, similarly unblocking the network thread and allowing the different side messages to be processed in order. Each would get it's own buffered channel to some reasonable value, and allow the unblocking to occur.
   
   The concern with B is in "high concurrent bundle" cases which may be unnecessary memory overhead per bundle, but this is speculation and would need to be validated through profiling. 
   
   In either case, the same single "response" channel will be used to serialize the responses to the runner half since that should remain a low return effort reply. (subject to profiling).
   
   cc: @lukecwik @lostluck 
   
   ### Issue Priority
   
   Priority: 2
   
   ### Issue Component
   
   Component: sdk-go


-- 
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: github-unsubscribe@beam.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck commented on issue #23335: [Task][Go SDK]: Respond to Control FnAPI messages per-Bundle ordering, not globally.

Posted by GitBox <gi...@apache.org>.
lostluck commented on issue #23335:
URL: https://github.com/apache/beam/issues/23335#issuecomment-1275234414

   #23583 removes a long operations from out of the lock, but another action would be to separate the lock into either map specific locks, or into control message usage specific locks. That could also avoid losing process on the control channel handler goroutine.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck commented on issue #23335: [Task][Go SDK]: Respond to Control FnAPI messages per-Bundle ordering, not globally.

Posted by GitBox <gi...@apache.org>.
lostluck commented on issue #23335:
URL: https://github.com/apache/beam/issues/23335#issuecomment-1275433992

   The SDK should do a keyed de-dupe to make a single in-flight for the same requests
   That would have benefits for when the descriptor is large, as we'd save the deserialization time, if not the plan construction time.
   This would have more benefits for streaming jobs than batch, but it's a good move nevertheless, as it would reduce wire congestion.


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [beam] lostluck commented on issue #23335: [Task][Go SDK]: Respond to Control FnAPI messages per-Bundle ordering, not globally.

Posted by GitBox <gi...@apache.org>.
lostluck commented on issue #23335:
URL: https://github.com/apache/beam/issues/23335#issuecomment-1276516910

   Not likely the issue the user is facing, but Bundle Finalization seems to be doing a fair amount of work in the control lock critical section that could be moved out.
   
   https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/harness/harness.go#L413
   
   and accessing the plans map doesn't appear to be locked here too:
   https://github.com/apache/beam/blob/master/sdks/go/pkg/beam/core/runtime/harness/harness.go#L505


-- 
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: github-unsubscribe@beam.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org