Skip to content

Delete callback should use namespace_id

Terri Chu requested to merge tchu-fix-indexer-removal-on-enabled-namespace-deleted into master

What does this MR do and why?

Search::Zoekt::Index delete_from_index callback should use namespace_id (not null) since zoekt_enabled_namespace_id foreign key has ON DELETE SET NULL

CREATE TABLE zoekt_indices (
    id bigint NOT NULL,
    zoekt_enabled_namespace_id bigint,
    zoekt_node_id bigint NOT NULL,
    namespace_id bigint NOT NULL,
    created_at timestamp with time zone NOT NULL,
    updated_at timestamp with time zone NOT NULL,
    state smallint DEFAULT 0 NOT NULL
);

...

ALTER TABLE ONLY zoekt_indices
    ADD CONSTRAINT fk_bf205d4773 FOREIGN KEY (zoekt_enabled_namespace_id) REFERENCES zoekt_enabled_namespaces(id) ON DELETE SET NULL;

debugging that led here

slack thread (internal)

Attempt to delete the orphaned index record in staging resulted in an error in the callback method delete_from_index

Click to expand staging logs 
[ gstg ] production> i.attributes
=>
{"id"=>4,
 "zoekt_enabled_namespace_id"=>nil,
 "zoekt_node_id"=>3,
 "namespace_id"=>9970,
 "created_at"=>Thu, 08 Feb 2024 11:40:01.939874000 UTC +00:00,
 "updated_at"=>Thu, 08 Feb 2024 11:40:01.939874000 UTC +00:00,
 "state"=>"ready"}
[ gstg ] production> i.destroy
/opt/gitlab/embedded/service/gitlab-rails/ee/app/models/search/zoekt/index.rb:53:in `delete_from_index': undefined method `root_namespace_id' for nil:NilClass (NoMethodError)

        ::Search::Zoekt::NamespaceIndexerWorker.perform_async(zoekt_enabled_namespace.root_namespace_id,
                                                                                     ^^^^^^^^^^^^^^^^^^
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8.1/lib/active_support/callbacks.rb:400:in `block in make_lambda'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8.1/lib/active_support/callbacks.rb:261:in `block in conditional'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8.1/lib/active_support/callbacks.rb:599:in `block in invoke_after'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8.1/lib/active_support/callbacks.rb:599:in `each'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8.1/lib/active_support/callbacks.rb:599:in `invoke_after'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8.1/lib/active_support/callbacks.rb:108:in `run_callbacks'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8.1/lib/active_support/callbacks.rb:929:in `_run_commit_callbacks'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8.1/lib/active_record/transactions.rb:321:in `committed!'
	from /opt/gitlab/embedded/service/gitlab-rails/config/initializers/skip_transaction_checks.rb:8:in `block (2 levels) in committed!'
	from /opt/gitlab/embedded/service/gitlab-rails/config/initializers/forbid_sidekiq_in_transactions.rb:10:in `skipping_transaction_check'
	from /opt/gitlab/embedded/service/gitlab-rails/config/initializers/skip_transaction_checks.rb:8:in `block in committed!'
	from /opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/exclusive_lease.rb:104:in `skipping_transaction_check'
	from /opt/gitlab/embedded/service/gitlab-rails/config/initializers/skip_transaction_checks.rb:7:in `committed!'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8.1/lib/active_record/connection_adapters/abstract/transaction.rb:155:in `commit_records'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activerecord-7.0.8.1/lib/active_record/connection_adapters/abstract/transaction.rb:304:in `block in commit_transaction'
	from /opt/gitlab/embedded/lib/ruby/gems/3.1.0/gems/activesupport-7.0.8.1/lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
	... 33 levels...

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

backend change only

How to set up and validate locally

  1. enable and configure zoekt for a namespace
  2. for the namespace you setup for zoekt, destroy the enabled namespace record
[7] pry(main)> enabled_namespace
=> #<Search::Zoekt::EnabledNamespace:0x000000013421ab58
 id: 10,
 root_namespace_id: 107,
 created_at: Mon, 18 Mar 2024 18:56:44.751857000 UTC +00:00,
 updated_at: Mon, 18 Mar 2024 18:56:44.751857000 UTC +00:00,
 search: "[FILTERED]">
[8] pry(main)> index
=> #<Search::Zoekt::Index:0x0000000171379048
 id: 12,
 zoekt_enabled_namespace_id: 10,
 zoekt_node_id: 5,
 namespace_id: 107,
 created_at: Mon, 18 Mar 2024 18:56:52.015180000 UTC +00:00,
 updated_at: Mon, 18 Mar 2024 18:56:52.015180000 UTC +00:00,
 state: "ready">
=> enabled_namespace.destroy!
  1. ensure that the Search::Zoekt::Index record is also removed
Edited by Terri Chu

Merge request reports