Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Don't raise an error when ChordCounter is not found
When upgrading from <2.0.0 while current chords are not finished, the ChordCounter object does not exist, breaking the chord.
The `celery.chord_unlock` Task created by the legacy chord should unlock any existing chord while new chord will create the proper ChordCounter object
  • Loading branch information
AllexVeldman authored and auvipy committed Feb 5, 2021
1 parent 961177e commit 3f74b82
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 1 deletion.
5 changes: 4 additions & 1 deletion django_celery_results/backends/database.py
Expand Up @@ -137,8 +137,11 @@ def on_chord_part_return(self, request, state, result, **kwargs):
# SELECT FOR UPDATE is not supported on all databases
chord_counter = (
ChordCounter.objects.select_for_update()
.get(group_id=gid)
.filter(group_id=gid).first()
)
if chord_counter is None:
logger.warning("Can't find ChordCounter for Group %s", gid)
return
chord_counter.count -= 1
if chord_counter.count != 0:
chord_counter.save()
Expand Down
11 changes: 11 additions & 0 deletions t/unit/backends/test_database.py
Expand Up @@ -725,6 +725,17 @@ def test_on_chord_part_return(self):

request.chord.delay.assert_called_once()

def test_on_chord_part_return_counter_not_found(self):
"""Test if the chord does not raise an error if the ChordCounter is not found
Basically this covers the case where a chord was created with a version
<2.0.0 and the update was done before the chord was finished
"""
request = mock.MagicMock()
request.id = uuid()
request.group = uuid()
self.b.on_chord_part_return(request=request, state=None, result=None)

def test_callback_failure(self):
"""Test if a failure in the chord callback is properly handled"""
gid = uuid()
Expand Down

5 comments on commit 3f74b82

@zenoran
Copy link

@zenoran zenoran commented on 3f74b82 May 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.filter(group_id=gid).first() breaks Oracle database backends with ORA error (related to trying to LIMIT on select_for_update).

Couldn't the original code be wrapped in a try block and warn/return on ChordCounter.DoesNotExist?

@auvipy
Copy link
Member

@auvipy auvipy commented on 3f74b82 May 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that might be a possibility

@auvipy
Copy link
Member

@auvipy auvipy commented on 3f74b82 Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please review #325

@zenoran
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please review #325

Yes, that matches what we've been running in production for a few weeks now.

@auvipy
Copy link
Member

@auvipy auvipy commented on 3f74b82 Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you mind reviewing and approving that MR? additionally, does that break anything as we are removing .first() call

Please sign in to comment.