Skip to content

Commit 7bc2fb4

Browse files
authored
Fix a memory leak in the failed instruction id cache. (#39405)
* Sanitize the exception message stored for failed instructions to avoid inadvertently capturing the content of stack frames in the heap and limit RAM growth due to cache size. * Use Py3.10 syntax
1 parent dca1253 commit 7bc2fb4

2 files changed

Lines changed: 48 additions & 2 deletions

File tree

sdks/python/apache_beam/runners/worker/sdk_worker.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@
8080
MAX_KNOWN_NOT_RUNNING_INSTRUCTIONS = 1000
8181
# The number of ProcessBundleRequest instruction ids that BundleProcessorCache
8282
# will remember for failed instructions.
83-
MAX_FAILED_INSTRUCTIONS = 10000
83+
MAX_FAILED_INSTRUCTIONS = 1000
8484

8585
# retry on transient UNAVAILABLE grpc error from state channels.
8686
_GRPC_SERVICE_CONFIG = json.dumps({
@@ -559,7 +559,15 @@ def discard(self, instruction_id, exception):
559559
"""
560560
processor = None
561561
with self._lock:
562-
self.failed_instruction_ids[instruction_id] = exception
562+
tb_str = "".join(traceback.format_exception(exception))
563+
if len(tb_str) > 10240:
564+
tb_str = (
565+
tb_str[:5000] + "\n... [traceback truncated] ...\n" +
566+
tb_str[-5000:])
567+
clean_exception = RuntimeError(
568+
f"Original Exception: {type(exception).__name__}: {str(exception)[:2000]}\n{tb_str}"
569+
)
570+
self.failed_instruction_ids[instruction_id] = clean_exception
563571
while len(self.failed_instruction_ids) > MAX_FAILED_INSTRUCTIONS:
564572
self.failed_instruction_ids.popitem(last=False)
565573
if instruction_id in self.active_bundle_processors:

sdks/python/apache_beam/runners/worker/sdk_worker_test.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,44 @@ def test_failed_bundle_processor_returns_failed_split_response(self):
296296
worker.do_instruction(split_request).error,
297297
hc.contains_string('test message'))
298298

299+
def test_failed_instruction_id_cache_size_is_capped(self):
300+
data_channel_factory = mock.create_autospec(
301+
data_plane.GrpcClientDataChannelFactory)
302+
bundle_processor_cache = BundleProcessorCache(
303+
None, None, data_channel_factory, {})
304+
if bundle_processor_cache.periodic_shutdown:
305+
bundle_processor_cache.periodic_shutdown.cancel()
306+
307+
with mock.patch(
308+
'apache_beam.runners.worker.sdk_worker.MAX_FAILED_INSTRUCTIONS', 10):
309+
for i in range(15):
310+
bundle_processor_cache.discard(f'inst_{i}', RuntimeError(f'error {i}'))
311+
312+
for i in range(5):
313+
self.assertNotIn(
314+
f'inst_{i}', bundle_processor_cache.failed_instruction_ids)
315+
for i in range(5, 15):
316+
self.assertIn(
317+
f'inst_{i}', bundle_processor_cache.failed_instruction_ids)
318+
319+
def test_failed_instruction_tracebacks_are_truncated_when_too_long(self):
320+
data_channel_factory = mock.create_autospec(
321+
data_plane.GrpcClientDataChannelFactory)
322+
bundle_processor_cache = BundleProcessorCache(
323+
None, None, data_channel_factory, {})
324+
if bundle_processor_cache.periodic_shutdown:
325+
bundle_processor_cache.periodic_shutdown.cancel()
326+
327+
long_message = "x" * 15000
328+
bundle_processor_cache.discard('instruction_id', RuntimeError(long_message))
329+
330+
stored_exception = bundle_processor_cache.failed_instruction_ids[
331+
'instruction_id']
332+
tb_str = str(stored_exception)
333+
334+
self.assertLessEqual(len(tb_str), 13000)
335+
self.assertIn('[traceback truncated]', tb_str)
336+
299337
def test_data_sampling_response(self):
300338
# Create a data sampler with some fake sampled data. This data will be seen
301339
# in the sample response.

0 commit comments

Comments
 (0)