Commits on Oct 14, 2020

1. fix: Retain group_id when tasks get re-frozen …

   When a group task which is part of a chain was to be delayed by
   `trace_task()`, it would be reconstructed from the serialized request.
   Normally, this sets the `group_id` of encapsulated tasks to the ID of
   the group being instantiated. However, in the specific situation of a
   group that is the last task in a chain which contributes to the
   completion of a chord, it is essential that the group ID of the top-most
   group is used instead. This top-most group ID is used by the redis
   backend to track the completions of "final elements" of a chord in the
   `on_chord_part_return()` implementation. By overwriting the group ID
   which was already set in the `options` dictionaries of the child tasks
   being deserialized, the chord accounting done by the redis backend would
   be made inaccurate and chords would never complete.
   
   This change alters how options are overridden for signatures to ensure
   that if a `group_id` has already been set, it cannot be overridden.
   Since group ID should be generally opaque to users, this should not be
   disruptive.

   

   maybe-sybr committed Oct 14, 2020
   Copy the full SHA

   b41e958 View commit details

   Browse the repository at this point in the history

2. fix: Count chord "final elements" correctly …

   This change amends the implementation of `chord.__length_hint__()` to
   ensure that all child task types are correctly counted. Specifically:
   
    * all sub-tasks of a group are counted recursively
    * the final task of a chain is counted recursively
    * the body of a chord is counted recursively
    * all other simple signatures count as a single "final element"
   
   There is also a deserialisation step if a `dict` is seen while counting
   the final elements in a chord, however this should become less important
   with the merge of celery#6342 which ensures that tasks are recursively
   deserialized by `.from_dict()`.

   

   maybe-sybr committed Oct 14, 2020
   Copy the full SHA

   2afea54 View commit details

   Browse the repository at this point in the history

3. test: Add more integration tests for groups …

   These tests are intended to show that group unrolling should be
   respected in various ways by all backends. They should make it more
   clear what behaviour we should be expecting from nested canvas
   components and ensure that all the implementations (mostly relevant to
   chords and `on_chord_part_return()` code) behave sensibly.

   

   maybe-sybr committed Oct 14, 2020
   Copy the full SHA

   5b2e68e View commit details

   Browse the repository at this point in the history

4. test: Fix old markings for chord tests

   

   maybe-sybr committed Oct 14, 2020
   Copy the full SHA

   d8783ef View commit details

   Browse the repository at this point in the history

5. fix: Make KV-store backends respect chord size …

   This avoids an issue where the `on_chord_part_return()` implementation
   would check the the length of the result of a chain ending in a nested
   group. This would manifest in behaviour where a worker would be blocked
   waiting for for the result object it holds to complete since it would
   attempt to `.join()` the result object. In situations with plenty of
   workers, this wouldn't really cause any noticable issue apart from some
   latency or unpredictable failures - but in concurrency constrained
   situations like the integrations tests, it causes deadlocks.
   
   We know from previous commits in this series that chord completion is
   more complex than just waiting for a direct child, so we correct the
   `size` value in `BaseKeyValueStoreBackend.on_chord_part_return()` to
   respect the `chord_size` value from the request, falling back to the
   length of the `deps` if that value is missing for some reason (this is
   necessary to keep a number of the tests happy but it's not clear to me
   if that will ever be the case in real life situations).

   

   maybe-sybr committed Oct 14, 2020
   Copy the full SHA

   f8ab428 View commit details

   Browse the repository at this point in the history

6. fix: Retain chord header result structure in Redis …

   This change fixes the chord result flattening issue which manifested
   when using the Redis backend due to its deliberate throwing away of
   information about the header result structure. Rather than assuming that
   all results which contribute to the finalisation of a chord should be
   siblings, this change checks if any are complex (ie. `GroupResult`s) and
   falls back to behaviour similar to that implemented in the
   `KeyValueStoreBackend` which restores the original `GroupResult` object
   and `join()`s it.
   
   We retain the original behaviour which is billed as an optimisation in
   f09b041. We could behave better in the complex header result case by not
   bothering to stash the results of contributing tasks under the `.j` zset
   since we won't be using them, but without checking for the presence of
   the complex group result on every `on_chord_part_return()` call, we
   can't be sure that we won't need those stashed results later on. This
   would be an opportunity for optimisation in future if we were to use an
   `EVAL` to only do the `zadd()` if the group result key doesn't exist.
   However, avoiding the result encoding work in `on_chord_part_return()`
   would be more complicated. For now, it's not worth the brainpower.
   
   This change also slightly refactors the redis backend unit tests to make
   it easier to build fixtures and hit both the complex and simple result
   structure cases.

   

   maybe-sybr committed Oct 14, 2020
   Copy the full SHA

   8422937 View commit details

   Browse the repository at this point in the history