Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions src/sentry/tasks/backfill_group_action_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,43 @@ def backfill_group_action_log_for_group(
"total_created": total,
},
)


@instrumented_task(
name="sentry.tasks.backfill_group_action_log.reset_and_backfill_group_action_log",
namespace=issues_tasks,
silo_mode=SiloMode.CELL,
)
def reset_and_backfill_group_action_log(
group_id: int,
**kwargs: object,
) -> None:
from sentry.issues.models.groupactionlogentry import GroupActionLogEntry
from sentry.issues.models.groupderiveddata import GroupDerivedData

try:
group = Group.objects.get(id=group_id)
except Group.DoesNotExist:
logger.warning(
"backfill_group_action_log.group_not_found",
extra={"group_id": group_id},
)
return

GroupDerivedData.objects.filter(group_id=group_id).delete()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The reset_and_backfill_group_action_log task pre-emptively deletes GroupDerivedData, which prevents the subsequent async backfill from triggering the necessary derived data rebuild task.
Severity: HIGH

Suggested Fix

Remove the initial GroupDerivedData.objects.filter(group_id=group_id).delete() call from the reset_and_backfill_group_action_log task. The existing logic within the async backfill process, specifically the call to invalidate_group_derived_data, should be sufficient to handle the deletion and subsequent reprocessing trigger. This ensures the condition for scheduling the rebuild task is met correctly.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/sentry/tasks/backfill_group_action_log.py#L74

Potential issue: The `reset_and_backfill_group_action_log` task at line 74
unconditionally deletes `GroupDerivedData` records before scheduling an asynchronous
backfill. The backfill process later calls `invalidate_group_derived_data`, which is
supposed to trigger a data rebuild by calling `process_group_log_task.delay()`. However,
this trigger is conditional on `invalidate_group_derived_data` deleting at least one
row. Since the data was already deleted, this condition is never met. As a result, the
derived data is not rebuilt after the backfill completes, leaving it in an unprocessed
state indefinitely, which defeats the purpose of the backfill task.


deleted_count, _ = GroupActionLogEntry.objects.filter(
group_id=group_id,
source="backfill:activity",
).delete()
Comment thread
cursor[bot] marked this conversation as resolved.

logger.info(
"backfill_group_action_log.reset_completed",
extra={
"group_id": group_id,
"project_id": group.project_id,
"deleted_count": deleted_count,
},
)

Comment thread
sentry[bot] marked this conversation as resolved.
backfill_group_action_log_for_group.delay(group_id=group_id)
Comment thread
cursor[bot] marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Derived data not rebuilt

High Severity

reset_and_backfill_group_action_log deletes the group’s GroupDerivedData row, then only enqueues backfill_group_action_log_for_group. That backfill path calls invalidate_group_derived_data with a cursor, which schedules process_group_log_task only when it deletes an existing derived row. With no row left after reset, re-inserted backfill entries may never trigger derived recomputation, leaving derived state missing or stale despite the reset’s stated rebuild goal.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b9bc2ef. Configure here.

59 changes: 58 additions & 1 deletion tests/sentry/tasks/test_backfill_group_action_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@

from sentry.issues.action_log.types import GroupActionType, GroupActorType
from sentry.issues.models.groupactionlogentry import GroupActionLogEntry
from sentry.tasks.backfill_group_action_log import backfill_group_action_log_for_group
from sentry.tasks.backfill_group_action_log import (
backfill_group_action_log_for_group,
reset_and_backfill_group_action_log,
)
from sentry.testutils.cases import TestCase
from sentry.types.activity import ActivityType

Expand Down Expand Up @@ -132,3 +135,57 @@ def test_logs_and_reraises_on_failure(self, mock_backfill: Any) -> None:

with pytest.raises(RuntimeError):
backfill_group_action_log_for_group(self.group.id)


class ResetAndBackfillGroupActionLogTest(TestCase):
def setUp(self) -> None:
super().setUp()
self.group = self.create_group()
self.now = timezone.now()

def _backfill_group(self) -> None:
self.create_group_activity(
group=self.group,
type=ActivityType.SET_RESOLVED.value,
data={},
user_id=self.user.id,
datetime=self.now - timedelta(minutes=1),
)
backfill_group_action_log_for_group(self.group.id)

def test_deletes_backfilled_entries_and_retriggers(self) -> None:
self._backfill_group()
assert GroupActionLogEntry.objects.filter(group_id=self.group.id).count() == 1

with patch.object(backfill_group_action_log_for_group, "delay") as mock_delay:
reset_and_backfill_group_action_log(self.group.id)

assert GroupActionLogEntry.objects.filter(group_id=self.group.id).count() == 0
mock_delay.assert_called_once_with(group_id=self.group.id)

def test_preserves_non_backfill_entries(self) -> None:
self._backfill_group()

GroupActionLogEntry.objects.create(
group_id=self.group.id,
project_id=self.group.project_id,
type=GroupActionType.VIEW.value,
actor_type=GroupActorType.USER.value,
actor_id=self.user.id,
source="web",
data={},
)
assert GroupActionLogEntry.objects.filter(group_id=self.group.id).count() == 2

with patch.object(backfill_group_action_log_for_group, "delay"):
reset_and_backfill_group_action_log(self.group.id)

remaining = GroupActionLogEntry.objects.filter(group_id=self.group.id)
assert remaining.count() == 1
assert remaining[0].source == "web"

def test_noop_for_nonexistent_group(self) -> None:
with patch.object(backfill_group_action_log_for_group, "delay") as mock_delay:
reset_and_backfill_group_action_log(999999999)

mock_delay.assert_not_called()
Loading