Skip to content

Commit

Permalink
Retry once when saving submissions
Browse files Browse the repository at this point in the history
If two submissions are saved in rapid successon,
the simply_versioned gem can throw a unique constraint
error when it tries to create new records.

simply_versioned creates a new version of a record, which
is numbered as simply (previous_version_number + 1). The
version numbers must be unique. So, if two get started at
the same time -- or almost the same time -- they can
have the same number.

The unique_constraint_retry method will retry whatever
is in its block one time if there is a unique constraint
error. Retrying one time should be enough for us to avoid
this simply_versioned error that we're getting; in our
logs, we rarely get more than one in a row. The stack
traces show that it's happening in the broadcast_group_submission
method.

fixes INTEROP-8494

flag = none

test plan:
- specs pass

Change-Id: I0fa67c9c766e98f27a29c83a45576b5900164ed9
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/345096
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>
Reviewed-by: Ryan Hawkins <ryan.hawkins@instructure.com>
QA-Review: Tucker Mcknight <tmcknight@instructure.com>
Product-Review: Tucker Mcknight <tmcknight@instructure.com>
  • Loading branch information
tucker-m committed Apr 19, 2024
1 parent 0a8628e commit 84a1371
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
8 changes: 5 additions & 3 deletions app/models/submission.rb
Expand Up @@ -2460,9 +2460,11 @@ def students
end

def broadcast_group_submission
@group_broadcast_submission = true
save!
@group_broadcast_submission = false
Submission.unique_constraint_retry do
@group_broadcast_submission = true
save!
@group_broadcast_submission = false
end
end

# in a module so they can be included in other Submission-like objects. the
Expand Down
28 changes: 28 additions & 0 deletions spec/models/submission_spec.rb
Expand Up @@ -1792,6 +1792,34 @@
ActiveRecord::Associations.preload([version].map(&:model), :originality_reports)
end.not_to raise_error
end

it "retries if there is a conflict with an existing version number" do
submission_spec_model
version = Version.find_by(versionable: @submission)

allow(@submission.versions).to receive(:create).and_call_original
called_times = 0
expect(@submission.versions).to receive(:create) { |attributes|
called_times += 1
# This is a hacky way of mimicing a bug where two responses, received very quickly,
# would create a RecordNotUnique error when the tried to increment the version
# number at the same time.
v = Version.create(
**attributes,
versionable_id: version.versionable_id,
versionable_type: version.versionable_type
)
v.update_attribute(:number, version.number) if called_times == 1
v
}.twice

submission_versions = @submission.versions.count
@submission.with_versioning(explicit: true) do
@submission.broadcast_group_submission
end

expect(@submission.versions.count).to eq(submission_versions + 1)
end
end

it "ensures the media object exists" do
Expand Down

0 comments on commit 84a1371

Please sign in to comment.