[GitHub] [beam] lostluck commented on a change in pull request #12934: [BEAM-10959] Store a fixed amount of known process bundle instructions to prevent failures due to concurrency within a runner.

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

lostluck commented on a change in pull request #12934:
URL: https://github.com/apache/beam/pull/12934#discussion_r495205565



##########
File path: sdks/python/apache_beam/runners/worker/sdk_worker.py
##########
@@ -376,35 +385,76 @@ def get(self, instruction_id, bundle_descriptor_id):
 
     Moves the ``BundleProcessor`` from the inactive to the active cache.
     """
-    try:
-      # pop() is threadsafe
-      processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
-    except IndexError:
-      processor = bundle_processor.BundleProcessor(
-          self.fns[bundle_descriptor_id],
-          self.state_handler_factory.create_state_handler(
-              self.fns[bundle_descriptor_id].state_api_service_descriptor),
-          self.data_channel_factory)
-    self.active_bundle_processors[
+    with self._lock:
+      try:
+        # pop() is threadsafe
+        processor = self.cached_bundle_processors[bundle_descriptor_id].pop()
+        self.active_bundle_processors[
+          instruction_id] = bundle_descriptor_id, processor
+        try:
+          del self.known_not_running_instruction_ids[instruction_id]

Review comment:
       Aren't all instruction ids unique for a given SDK harness/runner combo anyway? If this is the first time we've seen this instruction, why would the instruction be repeated?  I thought only the bundle descriptor ids were re-used?




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