tvalentyn commented on code in PR #29742:
URL: https://github.com/apache/beam/pull/29742#discussion_r1425368545


##########
sdks/python/apache_beam/runners/worker/sdk_worker.py:
##########
@@ -491,7 +492,10 @@ def get(self, instruction_id, bundle_descriptor_id):
     # Make sure we instantiate the processor while not holding the lock.
     processor = bundle_processor.BundleProcessor(
         self.runner_capabilities,
-        self.fns[bundle_descriptor_id],
+        # Reduce risks of concurrent modifications of the same protos
+        # captured in bundle descriptor when the same bundle desciptor is used
+        # in different instructions.
+        copy.deepcopy(self.fns[bundle_descriptor_id]),

Review Comment:
   deepcopy (impl: 
https://github.com/python/cpython/blob/9263173280d7ce949911965efa5b745287c581b2/Lib/copy.py#L118)
 should defer to `__deepcopy__` 
https://github.com/search?q=repo%3Aprotocolbuffers%2Fprotobuf%20%22__deepcopy__%22&type=code
 , which calls MergeFrom; CopyFrom also uses MergeFrom: 
https://github.com/protocolbuffers/protobuf/blob/5076e6206e327942630e82bcec145826cf380559/python/google/protobuf/message.py#L91,
 so yes we can copy ourselves and save some cycles that performs all the 
branching.



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