Skip to content

feat(action_log): Allow GroupDerivedData rows to be regenerated without deleting them#119433

Closed
kcons wants to merge 9 commits into
masterfrom
kcons/softback
Closed

feat(action_log): Allow GroupDerivedData rows to be regenerated without deleting them#119433
kcons wants to merge 9 commits into
masterfrom
kcons/softback

Conversation

@kcons

@kcons kcons commented Jul 10, 2026

Copy link
Copy Markdown
Member

Rather than just delete and regenerate, this updates our GroupDerivedData update scheme to allow for safe in-place updates after regeneration.

This is done by introducing a 'generated_at' timestamp and using it as an indicator of the log state included.
We can check generated_at when updating a GroupDerivedData row, and if it is different we know that it may be working with a different log than us.
This field allows us to trigger generation runs safely and ensure the latest one wins.
Using this new capacity, we introduce introduce replacement regeneration tasks that efficiently and safely update the GroupDerivedData rows in place.

Fixes ISWF-3055.

@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 10, 2026
kcons added 3 commits July 17, 2026 16:00
Add build-and-promote workflow: create temporary non-live rows in the
background, drain the full action log into them, and atomically promote
to live — replacing the old live row without disrupting reads.

Includes project-wide batch processing with time-bounded tasks that
self-reschedule, retry caps, and stale row cleanup.
Comment thread src/sentry/issues/derived/processing.py Outdated
@kcons kcons changed the title feat(action_log): Allow provisional GroupDerivedData rows to be created and promoted to live feat(action_log): Allow GroupDerivedData rows to be regenerated without deleting them Jul 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/1141_add_is_live_to_group_derived_data.py

for 1141_add_is_live_to_group_derived_data in sentry

--
-- Remove index sentry_grou_progres_f1627f_idx from groupderiveddata
--
DROP INDEX CONCURRENTLY IF EXISTS "sentry_grou_progres_f1627f_idx";
--
-- Remove index sentry_grou_last_pr_8c8185_idx from groupderiveddata
--
DROP INDEX CONCURRENTLY IF EXISTS "sentry_grou_last_pr_8c8185_idx";
--
-- Add field is_live to groupderiveddata
--
ALTER TABLE "sentry_groupderiveddata" ADD COLUMN "is_live" boolean DEFAULT false NOT NULL;
--
-- Add field generation_id to groupderiveddata
--
ALTER TABLE "sentry_groupderiveddata" ADD COLUMN "generation_id" bigint DEFAULT 0 NOT NULL;
--
-- Alter field group on groupderiveddata
--
SET CONSTRAINTS "sentry_groupderivedd_group_id_5f3f73c7_fk_sentry_gr" IMMEDIATE; ALTER TABLE "sentry_groupderiveddata" DROP CONSTRAINT "sentry_groupderivedd_group_id_5f3f73c7_fk_sentry_gr";
ALTER TABLE "sentry_groupderiveddata" DROP CONSTRAINT "sentry_groupderiveddata_group_id_key";
CREATE INDEX CONCURRENTLY "sentry_groupderiveddata_group_id_5f3f73c7" ON "sentry_groupderiveddata" ("group_id");
ALTER TABLE "sentry_groupderiveddata" ADD CONSTRAINT "sentry_groupderivedd_group_id_5f3f73c7_fk_sentry_gr" FOREIGN KEY ("group_id") REFERENCES "sentry_groupedmessage" ("id") DEFERRABLE INITIALLY DEFERRED NOT VALID;
ALTER TABLE "sentry_groupderiveddata" VALIDATE CONSTRAINT "sentry_groupderivedd_group_id_5f3f73c7_fk_sentry_gr";
--
-- Create index sentry_gdd_progress_live on field(s) progress, group of model groupderiveddata
--
CREATE INDEX CONCURRENTLY "sentry_gdd_progress_live" ON "sentry_groupderiveddata" ("progress", "group_id") WHERE "is_live";
--
-- Create index sentry_gdd_lastprog_live on field(s) last_progressed_at, group of model groupderiveddata
--
CREATE INDEX CONCURRENTLY "sentry_gdd_lastprog_live" ON "sentry_groupderiveddata" ("last_progressed_at", "group_id") WHERE "is_live";
--
-- Create index sentry_grou_group_i_2bcdf6_idx on field(s) group, is_live of model groupderiveddata
--
CREATE INDEX CONCURRENTLY "sentry_grou_group_i_2bcdf6_idx" ON "sentry_groupderiveddata" ("group_id", "is_live");
--
-- Create index sentry_gdd_stale_cleanup on field(s) date_added of model groupderiveddata
--
CREATE INDEX CONCURRENTLY "sentry_gdd_stale_cleanup" ON "sentry_groupderiveddata" ("date_added") WHERE NOT "is_live";
--
-- Create constraint uniq_live_gdd_per_group on model groupderiveddata
--
CREATE UNIQUE INDEX CONCURRENTLY "uniq_live_gdd_per_group" ON "sentry_groupderiveddata" ("group_id") WHERE "is_live";

Comment thread src/sentry/issues/models/groupderiveddata.py Outdated
@github-actions

Copy link
Copy Markdown
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/1141_add_invalidated_at_to_group_derived_data.py

for 1141_add_invalidated_at_to_group_derived_data in sentry

--
-- Add field invalidated_at to groupderiveddata
--
ALTER TABLE "sentry_groupderiveddata" ADD COLUMN "invalidated_at" timestamp with time zone NULL;
--
-- Create index sentry_gdd_needs_rebuild on field(s) invalidated_at of model groupderiveddata
--
CREATE INDEX CONCURRENTLY "sentry_gdd_needs_rebuild" ON "sentry_groupderiveddata" ("invalidated_at") WHERE "invalidated_at" IS NOT NULL;

Comment thread src/sentry/issues/derived/processing.py Outdated
@github-actions

Copy link
Copy Markdown
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/1141_add_generated_at_to_group_derived_data.py

for 1141_add_generated_at_to_group_derived_data in sentry

--
-- Add field generated_at to groupderiveddata
--
ALTER TABLE "sentry_groupderiveddata" ADD COLUMN "generated_at" timestamp with time zone DEFAULT (STATEMENT_TIMESTAMP()) NOT NULL;

@linear-code

linear-code Bot commented Jul 20, 2026

Copy link
Copy Markdown

ISWF-3055

@kcons

kcons commented Jul 20, 2026

Copy link
Copy Markdown
Member Author

FWIW, once this is considered acceptable I'll pull out the migration and do it first.

# the generation observed. Used as a CAS version — incremental writes
# check equality, generation promotes require their timestamp to be
# newer. Defaults to row creation time.
generated_at = models.DateTimeField(default=timezone.now, db_default=models.functions.Now())

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.

db_default references nonexistent models.functions.Now()

Import Now from django.db.models.functions and change db_default to Now(). django.db.models does not expose a functions attribute, so this line raises AttributeError at class definition time on every startup.

Evidence
  • groupderiveddata.py:62 evaluates db_default=models.functions.Now() while executing the GroupDerivedData class body.
  • models is django.db.models, which does not have a functions attribute.
  • Every other model usage in the codebase (e.g., groupactionlogentry.py:6) imports Now from django.db.models.functions.
  • This causes an AttributeError on every application startup before any request is handled.

Identified by Warden sentry-backend-bugs · YQJ-6WZ

Comment on lines +310 to +327


def promote_to_live(candidate: GroupDerivedData) -> PromotionResult:
"""Upsert the candidate's state into the row for its group.

The UPDATE guard requires that ``candidate.generated_at`` is >= the
row's (newer generation wins) and the cursor is at or ahead. On
success, all state fields (including ``generated_at``) are stamped.

Returns SUPERSEDED if the row has a newer ``generated_at``.
Returns CURSOR_BEHIND if the cursor guard failed.

The candidate object itself is not persisted — it may be an unsaved
in-memory instance used only to carry the computed state.
"""
generated_at = candidate.generated_at
values = {f: getattr(candidate, f) for f in _STATE_FIELDS}

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.

promote_to_live regresses cursor when concurrent incremental processing advances ahead

The update guard in promote_to_live matches rows with an older generated_at regardless of cursor position. When a full regeneration runs concurrently with incremental processing, the generation can overwrite a more advanced cursor, causing duplicate processing and incorrect derived state.

Evidence
  • _process_batch increments the live row's cursor but does not update generated_at, so the DB row retains its older generation timestamp.
  • build_and_promote_derived_data drains the log with a new generated_at = timezone.now() and then calls promote_to_live.
  • promote_to_live filters with Q(generated_at__lt=generated_at), which matches the live row even when its cursor is ahead of the candidate's cursor.
  • .update(**values) then stamps the candidate's older cursor into the row, regressing progress so future incremental passes re-process already-handled entries.
  • The CURSOR_BEHIND reload-and-retry block is bypassed because promote_to_live returns PROMOTED instead of CURSOR_BEHIND.

Identified by Warden sentry-backend-bugs · GPH-MJ7

@kcons kcons closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant