Avoid mutating cached epic in EE Note#touch_noteable
What does this MR do?
The EE override of Note#touch_noteable (for Epics) does noteable_object.touch followed by noteable_object.sync_work_item_updated_at. When noteable_object is a let_it_be-cached frozen Epic, the .touch raises FrozenError.
This MR mirrors the CE fix (!236545 (closed)): when the noteable_object is frozen, use touch_all to issue the SQL UPDATE without mutating the cached instance, then reload a fresh instance so sync_work_item_updated_at sees the new updated_at timestamp:
return unless noteable_object
if noteable_object.frozen?
# `noteable_object` may be a `let_it_be`-cached frozen instance in tests;
# touch_all writes the UPDATE without mutating the cached AR object.
# Reload a fresh instance for downstream sync (we need its new updated_at).
noteable_object.class.where(id: noteable_object.id).touch_all
noteable_object = noteable_object.class.find(noteable_object.id)
else
noteable_object.touch
end
# Ensure epic and work items are kept in sync after creating notes on the epic
noteable_object.sync_work_item_updated_atThe extra find only happens in the frozen-record path; production code never takes that branch. Same SQL, same effect on the database.
Why?
This is the second-highest-volume mutation site surfaced by the freeze-flip probe (#600433 (closed)): 1,584 occurrences of FrozenError originated from ee/app/models/ee/note.rb:145. Same pattern as the CE Note's touch_noteable, applied to Epic noteables.
Empirically validated: the validation MR (!236528 (closed)) layered this fix together with two others on top of the probe (!236505 (closed)), and the probe pipeline's failure count dropped from 665 → 183 (a 72.5% reduction), with zero remaining occurrences of ee/note.rb:145 in the validation traces.
Verification
- Local:
bundle exec rspec ee/spec/models/note_spec.rb - This MR's CI run.
- Validation evidence: !236528 (closed) (compare https://gitlab.com/gitlab-org/gitlab/-/pipelines/2533568410 baseline vs https://gitlab.com/gitlab-org/gitlab/-/pipelines/2534042883 with this fix layered).
References
- #600433 (closed) (production-code mutation tracking issue)
- #600267 (closed) (let_it_be freeze rollout)
- !236505 (closed) (probe MR)
- !236528 (closed) (validation MR)
- !236545 (closed) (CE sibling for
app/models/note.rb) - !236544 (merged) (sibling for
app/models/concerns/base_label.rb)
The three sibling MRs together fix the top-3 mutation sites and account for ~80% of probe failures.
/cc @pedropombeiro