Removes sql retry on upsert #2571
Conversation
brendankowitz
force-pushed
the
personal/bkowitz/sql-rawresource-fix
branch
from
c782383
to
eee46ea
Compare
| using (SqlConnectionWrapper sqlConnectionWrapper = await _sqlConnectionWrapperFactory.ObtainSqlConnectionWrapperAsync(cancellationToken, true)) | ||
| using (SqlCommandWrapper sqlCommandWrapper = sqlConnectionWrapper.CreateRetrySqlCommand()) | ||
| using (SqlCommandWrapper sqlCommandWrapper = sqlConnectionWrapper.CreateNonRetrySqlCommand()) |
There was a problem hiding this comment.
Is this the only command that need retry disabled? We use this in a lot of places.
There was a problem hiding this comment.
Taking a quick look I think we should also disallow retries in the SqlServerTaskManager line 39, SqlServerContextUpdater line 46, SqlServerTaskConsumer lines 52, 172, and 229, SqlServerFhirOperationsDataStore lines 57, 130, 200, 249, SqlServerFhirModel lines 227 and 373, SqlServerFhirDataStore lines 498 and 615, and SqlServerSearchParameterStatusDataStore line 205.
There was a problem hiding this comment.
All the commands above are creating or updating resources, so they could have the same stream issue.
There was a problem hiding this comment.
Its only a problem if we use Stream
There was a problem hiding this comment.
I think this is ok to merge. Of all the other instances I've checked, RawResource is the only instance I can see that uses VarBinary / Stream. VarChar / String is fine
Description
Removes Retry on sql command for Upsert. The error still occurs, but data is saved correctly in success cases.
Related issues
Addresses [issue #].
Testing
Manual testing using faults inserted into SQL Proc.
FHIR Team Checklist
Semver Change (docs)
Patch