diff options
author | Nick Thomas <nick@gitlab.com> | 2018-01-24 18:06:50 +0300 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2018-01-25 14:39:46 +0300 |
commit | b02f9b6141627127acd027ed9491dd7c0ca5dd2a (patch) | |
tree | b8e8433f51f44d60d9e3c6a687d641b0db9a177e /app/models/merge_request.rb | |
parent | e2a56af930f9f7d17a6a9b638f52007a60e4cc60 (diff) |
Look at notes created just before merge when deciding if an MR can be reverted
On MySQL, at least, `Note#created_at` doesn't seem to store fractional seconds,
while `MergeRequest::Metrics#merged_at` does. This breaks the optimization
assumption that we only need to search for notes created *after* the MR has
been merged.
Unsynchronized system clocks also make this a dangerous assumption to make.
Adding a minute of leeway still optimizes away most notes, but allows both
cases to be handled more gracefully. If the system clocks are more than a
minute out, we'll still be broken, of course.
Diffstat (limited to 'app/models/merge_request.rb')
-rw-r--r-- | app/models/merge_request.rb | 8 |
1 files changed, 7 insertions, 1 deletions
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 8028ff3875b..4accb08eaf9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -989,8 +989,14 @@ class MergeRequest < ActiveRecord::Base merged_at = metrics&.merged_at notes_association = notes_with_associations + # It is not guaranteed that Note#created_at will be strictly later than + # MergeRequestMetric#merged_at. Nanoseconds on MySQL may break this + # comparison, as will a HA environment if clocks are not *precisely* + # synchronized. Add a minute's leeway to compensate for both possibilities + cutoff = merged_at - 1.minute + if merged_at - notes_association = notes_association.where('created_at > ?', merged_at) + notes_association = notes_association.where('created_at >= ?', cutoff) end !merge_commit.has_been_reverted?(current_user, notes_association) |