Remove use_loose_foreign_keys_deleted_record_store feature flag

What does this MR do and why?

Until now a feature flag decided where the loose foreign keys cleanup workers looked for deleted records. With the flag off they read only the old cell-local table, loose_foreign_keys_deleted_records. With it on they read through Gitlab::LooseForeignKeys::DeletedRecordStore, which merges that table with the four newer ones keyed by organization, namespace, project and user.

The flag has been at 100% in production for weeks and the behaviour is stable, so there is nothing left to decide. This MR deletes the flag, deletes the RecordStoreSelector concern that existed only to read it, and lets the three cleanup workers always go through the store. Production behaviour does not change, because that is already the path in use.

Doing this now instead of in Phase 5 is what makes the next step safe. The MR after this one starts rewriting triggers so deleted rows land in the newer tables. If the flag were still around, a self-managed instance would have it off by default, the triggers would write to the newer tables and the worker would keep reading only the old one. Those records would sit there untouched and the child rows would never be cleaned up. With the flag gone, that gap cannot happen.

The record_store: keyword defaults on ProcessDeletedRecordsService and BatchCleanerService are staying for now. They come out in Phase 5 along with the rest of the temporary code.

There is no migration here, so reverting this MR is enough to undo it.

References

How to verify

The three worker specs and the flow spec for the sharded records cover this change. Locally they pass with 36 examples and no failures, and RuboCop is clean.

bundle exec rspec spec/workers/loose_foreign_keys spec/lib/gitlab/database/loose_foreign_keys/sharded_deleted_records_flow_spec.rb
Edited by Leonardo da Rosa

Merge request reports

Loading
Loading