Reduce database queries in compliance evaluator worker

What was the problem

ComplianceManagement::ProjectComplianceEvaluatorWorker was using more than 60 seconds of database time per run. Reported figures: p95 of 372 seconds, max of 406 seconds, over 1022 samples.

The worker runs every 12 hours and handles up to 100 projects per job. Inside, it loops over projects, then requirements, then controls. With 10 requirements holding 5 controls each, that is 5000 iterations per job, so any wasted query in the inner body gets multiplied thousands of times.

Root causes

1. Presence validation on associations instead of foreign keys. Both status models used validates_presence_of on the belongs_to associations. A presence check on an association calls the association reader, which loads the record. Status rows are fetched with .first, which does not preload associations, so every save issued four extra SELECT queries for records the worker already held in memory. Tracing queries back to their caller showed all four coming from one line: control_status.update(status: status_value).
2. An unguarded validation running on every save. ProjectRequirementComplianceStatus#validate_associations had no guard, so it loaded four associations and ran an extra existence query on every save. The requirement refresh always sets updated_at: Time.current, so the record is always dirty and this validation always ran. The sibling model, ProjectControlComplianceStatus, already guards the same checks on whether the relevant foreign keys changed.
3. Reloading records the caller already had. RefreshStatusService reloaded the project in permitted? and the requirement in control_statuses, even though the caller had just passed both of them in.

The fix

Three changes, all in the model files. No worker or service changes were needed.

1. Validate the foreign key columns instead of the associations, in both status models. Every one of those columns is NOT NULL and carries a foreign key, so the database still enforces referential integrity and the presence check no longer loads anything. The two tables differ in their delete behaviour, cascade on the control statuses and restrict on the requirement statuses, and either is sufficient here.
2. Guard validate_associations on project_id_changed?, namespace_id_changed?, compliance_requirement_id_changed?, or compliance_framework_id_changed?. These checks only compare foreign key relationships, so a counter refresh that does not touch those keys has nothing new to check. This matches how ProjectControlComplianceStatus already guards its equivalents.
3. In find_or_create_project_and_requirement, attach the project and requirement objects the caller passed in as the association targets on the row that was found. The row was selected by those two ids, so reading either association later returns the object already in memory instead of issuing a query for it.

Measured results

Measured on the steady-state path, with a fixture of 2 requirements holding 5 controls between them, with the real RefreshStatusService running:

variant	queries per project
master	61
foreign key validations only	25
the complete fix	21

That is a 66 percent reduction. The number is a slope, not a total: it is the difference between a two-project run and a five-project run divided by three, so it isolates the cost that scales and cancels out the fixed setup.

An earlier revision of this description quoted 43 before and 13 after. Those were measured while RefreshStatusService was replaced by a test double for the whole spec file, which left the requirement-refresh path out of the measurement and understated both figures.

How this was tested

The tests are split by the kind of claim they make, because a query count and a behaviour are not the same assertion.

How to verify locally

Two ways, in a Rails console. The first isolates the extra queries so you can name them; the second shows the multiplication that made this matter in production.

1. A single save. Create a framework applied to a project, a requirement, and one status row. Then fetch the row the way the worker does and save it while counting queries:

klass = ComplianceManagement::ComplianceFramework::ProjectRequirementComplianceStatus

tables = %w[project_requirement_compliance_statuses compliance_management_frameworks
            compliance_requirements projects namespaces]

measure = lambda do
  status = klass.for_project_and_requirement(project.id, requirement.id).first
  seen = []
  sub = ActiveSupport::Notifications.subscribe("sql.active_record") do |*, payload|
    sql = payload[:sql].to_s
    next if payload[:name].to_s =~ /SCHEMA/
    next unless sql.start_with?("SELECT", "UPDATE")

    table = tables.find { |name| sql.include?(%("#{name}")) }
    seen << "#{sql[0, 6].strip.ljust(6)} #{table}" if table
  end
  status.update!(pass_count: 1, fail_count: 0, pending_count: 0, updated_at: Time.current)
  ActiveSupport::Notifications.unsubscribe(sub)
  puts "#{seen.size} queries:"
  seen.each { |line| puts "   #{line}" }
end

measure.call

On this branch that prints one query, the UPDATE. On master branch the same save now issues five queries: SELECT against projects, namespaces, compliance_requirements and compliance_management_frameworks, then the UPDATE. Those four are reads of records the caller already holds, and the worker performs one of these saves per control per project.

2. The whole worker. Create a framework applied to two projects, with two requirements holding three controls each, then:

ids = framework.projects.pluck(:id)
worker = ComplianceManagement::ProjectComplianceEvaluatorWorker.new

worker.perform(framework.id, ids)                                        # creates the status rows
ActiveRecord::QueryRecorder.new { worker.perform(framework.id, ids) }.count

The worker has to run twice. The first run creates the rows through create!, which receives the project and requirement as objects, so their associations are already loaded and the extra queries never fire. Only the second run reads the rows back with .first, which is what production does on every scheduled run because the rows were written 12 hours earlier. For the exact per-project figures, see the guardrail in ee/spec/workers/compliance_management/project_compliance_evaluator_worker_spec.rb, which measures the slope rather than a single total.

What is deliberately not included

Two further improvements were left out. Both are named as later phases in the existing spec comment, and both are larger refactors:

• Batching the per-control status lookups, which is currently one SELECT per control.
• Memoizing the ProjectFields lookups.

References

#571915