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.
##########
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]),
self.state_handler_factory.create_state_handler(
self.fns[bundle_descriptor_id].state_api_service_descriptor),
Review Comment:
sure, that shouldn't hurt and would be cleaner.
--
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]
