Run integration suite with memcached results backend.#5739
Conversation
|
The failure in EDIT:I have no idea why sleeping resolves the issue but reverting the fix in #5681 resolves the issue. EDIT 2:I still have no idea why sleeping resolved the issue but I found the problem and the correct fix. |
|
The t1 = identity.si(1)
t2 = identity.si(2)
gt = group([identity.si(3), identity.si(4)])
ct = chain(identity.si(5), gt)
task = group(t1, t2, ct)The result should be EDIT:It seems like the problem exists for all synchronous results backends: class SyncBackendMixin(object):
def iter_native(self, result, timeout=None, interval=0.5, no_ack=True,
on_message=None, on_interval=None):
self._ensure_not_eager()
results = result.results
if not results:
return iter([])
return self.get_many(
{r.id for r in results},
timeout=timeout, interval=interval, no_ack=no_ack,
on_message=on_message, on_interval=on_interval,
)
# ...The last result in class BaseKeyValueStoreBackend(Backend):
#...
def get_many(self, task_ids, timeout=None, interval=0.5, no_ack=True,
on_message=None, on_interval=None, max_iterations=None,
READY_STATES=states.READY_STATES):
interval = 0.5 if interval is None else interval
ids = task_ids if isinstance(task_ids, set) else set(task_ids)
cached_ids = set()
cache = self._cache
for task_id in ids:
try:
cached = cache[task_id]
except KeyError:
pass
else:
if cached['status'] in READY_STATES:
yield bytes_to_str(task_id), cached
cached_ids.add(task_id)
ids.difference_update(cached_ids)
iterations = 0
while ids:
keys = list(ids)
r = self._mget_to_results(self.mget([self.get_key_for_task(k) # There's no task id with this group id
for k in keys]), keys)
cache.update(r)
ids.difference_update({bytes_to_str(v) for v in r})
for key, value in items(r):
if on_message is not None:
on_message(value)
yield bytes_to_str(key), value
if timeout and iterations * interval >= timeout:
raise TimeoutError('Operation timed out ({0})'.format(timeout))
if on_interval:
on_interval()
time.sleep(interval) # don't busy loop.
iterations += 1
if max_iterations and iterations >= max_iterations:
breakThat's why it hangs forever or times out. EDIT 2:The strange thing about this bug is that the same test passes with the DynamoDB results backend which inherits from the EDIT 3:The difference is that DynamoDB does not use class ResultSet(ResultBase):
# ...
def join(self, timeout=None, propagate=True, interval=0.5,
callback=None, no_ack=True, on_message=None,
disable_sync_subtasks=True, on_interval=None):
"""Gather the results of all tasks as a list in order.
Note:
This can be an expensive operation for result store
backends that must resort to polling (e.g., database).
You should consider using :meth:`join_native` if your backend
supports it.
Warning:
Waiting for tasks within a task may lead to deadlocks.
Please see :ref:`task-synchronous-subtasks`.
Arguments:
timeout (float): The number of seconds to wait for results
before the operation times out.
propagate (bool): If any of the tasks raises an exception,
the exception will be re-raised when this flag is set.
interval (float): Time to wait (in seconds) before retrying to
retrieve a result from the set. Note that this does not have
any effect when using the amqp result store backend,
as it does not use polling.
callback (Callable): Optional callback to be called for every
result received. Must have signature ``(task_id, value)``
No results will be returned by this function if a callback
is specified. The order of results is also arbitrary when a
callback is used. To get access to the result object for
a particular id you'll have to generate an index first:
``index = {r.id: r for r in gres.results.values()}``
Or you can create new result objects on the fly:
``result = app.AsyncResult(task_id)`` (both will
take advantage of the backend cache anyway).
no_ack (bool): Automatic message acknowledgment (Note that if this
is set to :const:`False` then the messages
*will not be acknowledged*).
disable_sync_subtasks (bool): Disable tasks to wait for sub tasks
this is the default configuration. CAUTION do not enable this
unless you must.
Raises:
celery.exceptions.TimeoutError: if ``timeout`` isn't
:const:`None` and the operation takes longer than ``timeout``
seconds.
"""
if disable_sync_subtasks:
assert_will_not_block()
time_start = monotonic()
remaining = None
if on_message is not None:
raise ImproperlyConfigured(
'Backend does not support on_message callback')
results = []
for result in self.results:
remaining = None
if timeout:
remaining = timeout - (monotonic() - time_start)
if remaining <= 0.0:
raise TimeoutError('join operation timed out')
value = result.get(
timeout=remaining, propagate=propagate,
interval=interval, no_ack=no_ack, on_interval=on_interval,
disable_sync_subtasks=disable_sync_subtasks,
) # Collects the GroupResult's here
if callback:
callback(result.id, value)
else:
results.append(value)
return resultsWe need to improve EDIT 4:The following patch makes the test pass: diff --git a/celery/backends/base.py b/celery/backends/base.py
index 468ccc124..e48e0d580 100644
--- a/celery/backends/base.py
+++ b/celery/backends/base.py
@@ -483,7 +483,7 @@ class SyncBackendMixin(object):
if not results:
return iter([])
return self.get_many(
- {r.id for r in results},
+ {r if isinstance(r, GroupResult) else r.id for r in results},
timeout=timeout, interval=interval, no_ack=no_ack,
on_message=on_message, on_interval=on_interval,
)
@@ -648,6 +648,9 @@ class BaseKeyValueStoreBackend(Backend):
cached_ids = set()
cache = self._cache
for task_id in ids:
+ if isinstance(task_id, GroupResult):
+ continue
+
try:
cached = cache[task_id]
except KeyError:
@@ -660,15 +663,35 @@ class BaseKeyValueStoreBackend(Backend):
ids.difference_update(cached_ids)
iterations = 0
while ids:
- keys = list(ids)
- r = self._mget_to_results(self.mget([self.get_key_for_task(k)
- for k in keys]), keys)
- cache.update(r)
- ids.difference_update({bytes_to_str(v) for v in r})
- for key, value in items(r):
- if on_message is not None:
- on_message(value)
- yield bytes_to_str(key), value
+ try:
+ keys = list(ids)
+ consume_until = next(i for i, k in enumerate(keys) if isinstance(k, GroupResult))
+ if consume_until == 0:
+ g = keys[0]
+ ids.remove(g)
+ yield g.id, g.results
+ else:
+ keys = keys[:consume_until]
+ r = self._mget_to_results(
+ self.mget([self.get_key_for_task(k)
+ for k in keys]), keys)
+ cache.update(r)
+ ids.difference_update({bytes_to_str(v) for v in r})
+ for key, value in items(r):
+ if on_message is not None:
+ on_message(value)
+ yield bytes_to_str(key), value
+ except StopIteration:
+ keys = list(ids)
+ r = self._mget_to_results(self.mget([self.get_key_for_task(k)
+ for k in keys]), keys)
+ cache.update(r)
+ ids.difference_update({bytes_to_str(v) for v in r})
+ for key, value in items(r):
+ if on_message is not None:
+ on_message(value)
+ yield bytes_to_str(key), value
+
if timeout and iterations * interval >= timeout:
raise TimeoutError('Operation timed out ({0})'.format(timeout))
if on_interval:The code in this solution is inelegant at best. |
|
The I still don't know why. |
|
Sorry for my latency of response. |
test_nested_group_chain is still failing and I have no idea why. |
OK, I'll debug it. |
Any luck? |
Too busy these days, I wish I can fix it this week. |
fd133ff to
6cd07f6
Compare
Codecov Report
@@ Coverage Diff @@
## master #5739 +/- ##
=========================================
- Coverage 83.39% 83.29% -0.1%
=========================================
Files 144 144
Lines 16702 16775 +73
Branches 2080 2101 +21
=========================================
+ Hits 13928 13973 +45
- Misses 2560 2586 +26
- Partials 214 216 +2
Continue to review full report at Codecov.
|
|
I'd like to merge this as the other fixes are relevant for other backends as well. |
|
wait let me check before the merge. |
Note: Before submitting this pull request, please review our contributing
guidelines.
Description
The memcache result backend has multiple failures/hangings in the canvas tests.
This is the result of not regularly running the tests with the cache results backend in CI.
This PR introduces the cache backend into the integration test suite matrix.