Geo selective sync cleanup worker uses inefficient `NOT IN` SQL query
The following discussion from !3595 (merged) should be addressed:
-
@nick.thomas started a discussion: OK, so this worker is only used when a
RepositoriesChangedEvent
is issued from the primary to the secondary. It signals that the geo node's selective sync list has changed.The query itself is problematic - it ends up looking slightly like this:
SELECT "projects".* FROM "projects" WHERE ("projects"."id" NOT IN (SELECT "projects"."id" FROM "projects" WHERE "projects"."id" IN (...)))
More details about
NOT IN
and alternatives: https://explainextended.com/2009/09/16/not-in-vs-not-exists-vs-left-join-is-null-postgresql/Fortunately, we don't send that event when we move from selective sync to full sync, so this is probably more or less OK as the list of IDs will tend to be fairly short.
I don't think this MR is making this any worse, so perhaps we can address this in a follow-up issue, since it's not directly related to this MR.
We need to think of how best to replace this query, as it's something of a liability.
Opening gambit: How about deprecating/renaming this event, in favour of passing a list of projects that have been removed from selective sync? This is what we're really interested in, and we can't calculate it on the secondary from the data we have.
create_table :geo_selective_sync_project_no_longer_tracked_events do |t|
t.integer :geo_node_id
t.integer :project_id
end
(terrible name, I know)
We'd handle each event individually on the secondary, so we could eliminate the NOT IN
logic entirely.