Skip to content

Commit daf162b

Browse files
Keep job meta that a worker cannot deserialize when the job is saved (#2538)
* Keep job meta that a worker cannot deserialize when the job is saved Job.restore() replaces meta it cannot deserialize with {'unserialized': <raw>} so the worker does not crash. A later save() or save_meta() then serialized that placeholder over the stored meta, so a retry, repeat or dependent enqueue by such a worker replaced the meta the enqueuing app had written. Write the original bytes back while meta is still the placeholder. * Combine the unreadable meta tests into one
1 parent 65dd7eb commit daf162b

3 files changed

Lines changed: 55 additions & 2 deletions

File tree

‎rq/job.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ def __init__(self, id: str | None = None, connection: Redis | None = None, seria
216216
self._status: JobStatus = JobStatus.CREATED
217217
self._dependency_ids: list[str] = []
218218
self.meta: dict[str, Any] = {}
219+
# Stored meta that could not be deserialized here, kept so a save writes it back unchanged.
220+
self._unreadable_meta: bytes | None = None
219221
self.serializer = resolve_serializer(serializer)
220222
# Tracks remaining retries for exception-based retry. Set to retry.max when a job
221223
# is enqueued with retry=Retry(...) via queue.enqueue(). Decremented each time
@@ -1039,10 +1041,12 @@ def restore(self, raw_data) -> Any:
10391041
self.allow_dependency_failures = bool(int(allow_failures)) if allow_failures else None
10401042
self.enqueue_at_front = bool(int(obj['enqueue_at_front'])) if 'enqueue_at_front' in obj else None
10411043
self.ttl = int(obj['ttl']) if obj.get('ttl') else None
1044+
self._unreadable_meta = None
10421045
try:
10431046
self.meta = self.serializer.loads(obj['meta']) if obj.get('meta') else {}
10441047
except Exception: # depends on the serializer
10451048
self.meta = {'unserialized': obj.get('meta', {})}
1049+
self._unreadable_meta = obj['meta']
10461050

10471051
self.number_of_retries = int(obj['number_of_retries']) if obj.get('number_of_retries') else None
10481052
self.retries_left = int(obj['retries_left']) if obj.get('retries_left') else None
@@ -1158,7 +1162,7 @@ def to_dict(self, include_meta: bool = True, include_result: bool = True) -> dic
11581162
obj['dependency_id'] = self._dependency_ids[0] # for backwards compatibility
11591163
obj['dependency_ids'] = json.dumps(self._dependency_ids)
11601164
if self.meta and include_meta:
1161-
obj['meta'] = self.serializer.dumps(self.meta)
1165+
obj['meta'] = self._serialize_meta()
11621166
if self.ttl:
11631167
obj['ttl'] = self.ttl
11641168

@@ -1195,9 +1199,17 @@ def save(self, pipeline: Pipeline | None = None, include_meta: bool = True, incl
11951199

11961200
def save_meta(self):
11971201
"""Stores job meta from the job instance to the corresponding Redis key."""
1198-
meta = self.serializer.dumps(self.meta)
1202+
meta = self._serialize_meta()
11991203
self.connection.hset(self.key, 'meta', meta)
12001204

1205+
def _serialize_meta(self) -> bytes:
1206+
# If this process could not deserialize the stored meta and it is still the
1207+
# placeholder holding those bytes, write the bytes back instead of the placeholder.
1208+
is_placeholder = len(self.meta) == 1 and self.meta.get('unserialized') is self._unreadable_meta
1209+
if is_placeholder and self._unreadable_meta is not None:
1210+
return self._unreadable_meta
1211+
return self.serializer.dumps(self.meta)
1212+
12011213
def cancel(
12021214
self,
12031215
pipeline: Pipeline | None = None,

‎tests/test_job.py‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,31 @@ def test_custom_meta_is_rewriten_by_save_meta(self):
471471
serialized2.pop('meta')
472472
self.assertDictEqual(serialized, serialized2)
473473

474+
def test_unreadable_meta(self):
475+
"""Meta this process could not deserialize is written back by save() and save_meta() until it is changed."""
476+
job = Job.create(func=fixtures.say_hello, args=('Lionel',), connection=self.connection)
477+
job.save()
478+
# Pickled meta naming a module this process cannot import (e.g. a type only the enqueuing app has)
479+
unreadable_meta = b'cnot_an_importable_module\nOwner\n.'
480+
self.connection.hset(job.key, 'meta', unreadable_meta)
481+
job.refresh()
482+
self.assertEqual(job.meta, {'unserialized': unreadable_meta})
483+
484+
job.save()
485+
self.assertEqual(self.connection.hget(job.key, 'meta'), unreadable_meta)
486+
487+
job.save_meta()
488+
self.assertEqual(self.connection.hget(job.key, 'meta'), unreadable_meta)
489+
490+
# Changed meta is saved as usual
491+
job.meta['foo'] = 'bar'
492+
job.save_meta()
493+
self.assertEqual(loads(self.connection.hget(job.key, 'meta')), {'unserialized': unreadable_meta, 'foo': 'bar'})
494+
495+
job.meta = {'foo': 'baz'}
496+
job.save()
497+
self.assertEqual(loads(self.connection.hget(job.key, 'meta')), {'foo': 'baz'})
498+
474499
def test_unpickleable_result(self):
475500
"""Unpickleable job result doesn't crash job.save() and job.refresh()"""
476501
job = Job.create(func=fixtures.say_hello, args=('Lionel',), connection=self.connection)

‎tests/test_worker.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,22 @@ def test_meta_is_unserializable(self):
332332
self.assertIsInstance(job.meta, dict)
333333
self.assertIn('unserialized', job.meta.keys())
334334

335+
def test_retry_keeps_unreadable_meta(self):
336+
"""A worker that cannot deserialize meta leaves it unchanged when it retries the job."""
337+
q = Queue(connection=self.connection)
338+
job = q.enqueue(div_by_zero, 1, retry=Retry(max=1), meta={'key': 'value'})
339+
# Pickled meta naming a module the worker cannot import
340+
unreadable_meta = b'cnot_an_importable_module\nOwner\n.'
341+
self.connection.hset(job.key, 'meta', unreadable_meta)
342+
343+
w = Worker([q], connection=self.connection)
344+
w.work(burst=True)
345+
346+
job.refresh()
347+
self.assertEqual(job.retries_left, 0)
348+
self.assertIn(job, FailedJobRegistry(queue=q))
349+
self.assertEqual(self.connection.hget(job.key, 'meta'), unreadable_meta)
350+
335351
@mock.patch('rq.worker.logger.error')
336352
def test_deserializing_failure_is_handled(self, mock_logger_error):
337353
"""

0 commit comments

Comments
 (0)