Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Set thread_id column to non-null for event_push_{actions,actions_staging,summary} #15350

Merged
merged 6 commits into from
Mar 30, 2023

Conversation

clokep
Copy link
Member

@clokep clokep commented Mar 29, 2023

Updates the schema to require a thread_id (i.e. make the column non-null) for event_push_actions, event_push_actions_staging, and event_push_actions_summary.

This bumps the schema compat version to 74. This was last bumped in #15036, which was released in v1.77.0rc2 so users will be unable to rollback to earlier than that version.

This has two commits which are mostly separate, it performs the following operations:

  • Deletes the background job to fill in any rows with thread_id = null in these tables.
  • Completes the background job in the foreground. (We previously added an index in Update the thread_id right before use (in case the bg update hasn't finished) #14222 to make this fast.)
  • Deletes the background jobs to create the indexes used above.
  • (The following depend on Postgres vs. sqlite, but I'll write out the operations ignoring that sqlite needs to make a copy):
    • Drop the indexes used above.
    • Alter the thread_id column to be non-null.
  • This causes a bunch of code to be unneeded, so we clean that up:
    • The background update to backfill this data.
    • The declarations to create the background indexes.
    • The just-in-time updating of the thread_id column (and the background checker to see if the background update is done).

Fixes #14225, reapplies part of #13776.

Comment on lines +16 to +17
-- Force the background updates from 06thread_notifications.sql to run in the
-- foreground as code will now require those to be "done".
Copy link
Member Author

Choose a reason for hiding this comment

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

@erikjohnston I doubt you remember, but does this sound like what we were looking to do in #14225 / #14222? I think it is, but would be curious if you have concerns!

@clokep clokep marked this pull request as ready for review March 29, 2023 19:00
@clokep clokep requested a review from a team as a code owner March 29, 2023 19:00
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

good otherwise

@clokep clokep requested a review from reivilibre March 30, 2023 16:32
@clokep clokep merged commit 2a234b7 into develop Mar 30, 2023
@clokep clokep deleted the clokep/thread-clean-up branch March 30, 2023 19:11
reivilibre added a commit that referenced this pull request Mar 31, 2023
…ons_staging,summary} (#15350)"

This reverts commit 2a234b7.

See #15359 for context.
clokep added a commit that referenced this pull request Apr 14, 2023
…ing,summary} (#15350)

Clean-up from adding the thread_id column, which was initially
null but backfilled with values. It is desirable to require it to now
be non-null.

In addition to altering this column to be non-null, we clean up
obsolete background jobs, indexes, and just-in-time updating
code.
clokep added a commit that referenced this pull request May 15, 2023
…ing,summary} (#15350)

Clean-up from adding the thread_id column, which was initially
null but backfilled with values. It is desirable to require it to now
be non-null.

In addition to altering this column to be non-null, we clean up
obsolete background jobs, indexes, and just-in-time updating
code.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make thread_id required in event_push tables
2 participants