Result object for groups containing chords which have a chain as their body time out #6734
Closed
4 of 18 tasks
Comments
|
Discovered while experimenting with tests for #6721 in #6733. #6733 should land an xfailing test for this issue. I think this might be due to the promise in the My gut feel is that we might want to add a final check prior to allowing a timeout to bubble up, to see if the results we were waiting for have actually landed or if we truly did timeout. |
|
I think this is likely something which can wait to land in an upcoming release rather than trying to squeeze into 5.1 |
maybe-sybr
added a commit
that referenced
this issue
Apr 19, 2021
This change ensures that we only have one piece of code which calculates chord sizes (ie. `_chord._descend()`, recently made protected so other canvas classes can use it as required). By doing so, we fix some edge cases in the chord counting logic which was being used for children of groups, and also add some unit tests to capture those cases and their expected behaviours. This change also introduces an integration test which checks the current behaviour of chains used as chord bodies when nested in groups. Due to some misbehaviour, likely with promise fulfillment, the `GroupResult` object will time out unless all of its children are resolved prior to `GroupResult` being joined (specifically, native joins block forever or until timeout). This misbehaviour is tracked by #6734 and the test in not marked as `xfail`ing to ensure that the current janky behaviour continues to work as expected rather than regressing.
maybe-sybr
added a commit
that referenced
this issue
Apr 19, 2021
This change ensures that we only have one piece of code which calculates chord sizes (ie. `_chord._descend()`, recently made protected so other canvas classes can use it as required). By doing so, we fix some edge cases in the chord counting logic which was being used for children of groups, and also add some unit tests to capture those cases and their expected behaviours. This change also introduces an integration test which checks the current behaviour of chains used as chord bodies when nested in groups. Due to some misbehaviour, likely with promise fulfillment, the `GroupResult` object will time out unless all of its children are resolved prior to `GroupResult` being joined (specifically, native joins block forever or until timeout). This misbehaviour is tracked by #6734 and the test in not marked as `xfail`ing to ensure that the current janky behaviour continues to work as expected rather than regressing.
maybe-sybr
added a commit
that referenced
this issue
Apr 27, 2021
This change ensures that we only have one piece of code which calculates chord sizes (ie. `_chord._descend()`, recently made protected so other canvas classes can use it as required). By doing so, we fix some edge cases in the chord counting logic which was being used for children of groups, and also add some unit tests to capture those cases and their expected behaviours. This change also introduces an integration test which checks the current behaviour of chains used as chord bodies when nested in groups. Due to some misbehaviour, likely with promise fulfillment, the `GroupResult` object will time out unless all of its children are resolved prior to `GroupResult` being joined (specifically, native joins block forever or until timeout). This misbehaviour is tracked by #6734 and the test in not marked as `xfail`ing to ensure that the current janky behaviour continues to work as expected rather than regressing.
|
I wonder if #6746 will fix this... |
thedrow
pushed a commit
that referenced
this issue
Apr 28, 2021
* improv: Deconflict `chord` class and kwarg names * improv: Make `chord.descend` protected not private This will allow us to call it from other code in this module which needs to accurately count chord sizes. * fix: Counting of chord-chain tails of zero tasks * fix: Chord counting of group children This change ensures that we only have one piece of code which calculates chord sizes (ie. `_chord._descend()`, recently made protected so other canvas classes can use it as required). By doing so, we fix some edge cases in the chord counting logic which was being used for children of groups, and also add some unit tests to capture those cases and their expected behaviours. This change also introduces an integration test which checks the current behaviour of chains used as chord bodies when nested in groups. Due to some misbehaviour, likely with promise fulfillment, the `GroupResult` object will time out unless all of its children are resolved prior to `GroupResult` being joined (specifically, native joins block forever or until timeout). This misbehaviour is tracked by #6734 and the test in not marked as `xfail`ing to ensure that the current janky behaviour continues to work as expected rather than regressing.
jeyrce
pushed a commit
to jeyrce/celery
that referenced
this issue
Aug 25, 2021
* improv: Deconflict `chord` class and kwarg names * improv: Make `chord.descend` protected not private This will allow us to call it from other code in this module which needs to accurately count chord sizes. * fix: Counting of chord-chain tails of zero tasks * fix: Chord counting of group children This change ensures that we only have one piece of code which calculates chord sizes (ie. `_chord._descend()`, recently made protected so other canvas classes can use it as required). By doing so, we fix some edge cases in the chord counting logic which was being used for children of groups, and also add some unit tests to capture those cases and their expected behaviours. This change also introduces an integration test which checks the current behaviour of chains used as chord bodies when nested in groups. Due to some misbehaviour, likely with promise fulfillment, the `GroupResult` object will time out unless all of its children are resolved prior to `GroupResult` being joined (specifically, native joins block forever or until timeout). This misbehaviour is tracked by celery#6734 and the test in not marked as `xfail`ing to ensure that the current janky behaviour continues to work as expected rather than regressing.
|
I guess we can close this now or push to 5.2.x milestone |
|
will re assign to 5.2.x in case it isnt fixed properly. |
|
What should be expected behavior? The test is marked still by xfailed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Checklist
masterbranch of Celery.contribution guide
on reporting bugs.
for similar or identical bug reports.
for existing proposed fixes.
to find out if the bug was already fixed in the master branch.
in this issue (If there are none, check this box anyway).
Mandatory Debugging Information
celery -A proj reportin the issue.(if you are not able to do this, then at least specify the Celery
version affected).
masterbranch of Celery.pip freezein the issue.to reproduce this bug.
Optional Debugging Information
and/or implementation.
result backend.
broker and/or result backend.
ETA/Countdown & rate limits disabled.
and/or upgrading Celery and its dependencies.
Related Issues and Possible Duplicates
Related Issues
Possible Duplicates
Environment & Settings
Celery version:
celery reportOutput:Steps to Reproduce
Required Dependencies
Python Packages
pip freezeOutput:Other Dependencies
N/A
Minimally Reproducible Test Case
This is the guts of the integration test I'm going to put up in #6733 :
Expected Behavior
Actual Behavior
res.get()raisesTimeoutErrorno matter how long you set your timeout value (AFAICT)res.get()gets the result properly since the task(s) do run and by that time the results have probably landedThe text was updated successfully, but these errors were encountered: