Detect conflict during named blob put operation before upload or stitch operation. #2704
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2704 +/- ##
=============================================
- Coverage 64.24% 31.88% -32.37%
+ Complexity 10398 5348 -5050
=============================================
Files 840 840
Lines 71755 71822 +67
Branches 8611 8621 +10
=============================================
- Hits 46099 22898 -23201
- Misses 23004 47146 +24142
+ Partials 2652 1778 -874 ☔ View full report in Codecov by Sentry. |
ankagrawal
force-pushed
the
ankuagra_check_named_conflict
branch
2 times, most recently
from
5d4f811
to
5653cfd
Compare
ankagrawal
changed the title
ankagrawal
force-pushed
the
ankuagra_check_named_conflict
branch
from
f48c53c
to
3ec9fa9
Compare
| && ((RestServiceException) e.getCause()).getErrorCode() == RestServiceErrorCode.NotFound) { | ||
| return false; | ||
| } else { | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
qq.. would it be better if we complete the detectionFuture with exception instead of throwing run time exception?
| frontendMetrics.addDatasetVersionRate.mark(); | ||
| addDatasetVersion(blobInfo.getBlobProperties(), restRequest); | ||
| } | ||
| idConverter.detectConflict(restRequest, detectConflictCallback(blobInfo)); |
There was a problem hiding this comment.
Hmm, this will result in checking for conflicts for regular named blob uploads as well, i.e. not just stitch requests? Wondering if it may increase latency of regular uploads due to additional call to mysql db. Would it be better to check for conflicts only for stitch requests?
|
|
||
| detectionFuture = getNamedBlobDb().get(namedBlobPath.getAccountName(), namedBlobPath.getContainerName(), | ||
| namedBlobPath.getBlobName(), GetOption.Include_All).thenApply(namedBlobRecord -> { | ||
| LOGGER.info("A mapping for the namedBlob {} already exists. Original BlobId: {}", |
There was a problem hiding this comment.
What about if the named blob is deleted?
If the named blob is deleted, we should allow upload even without the upsert option.
If it's the case, we need check the db record status.
| private CompletionStage<Boolean> detectConflict(RestRequest restRequest) throws RestServiceException { | ||
| final CompletionStage<Boolean> detectionFuture; | ||
| frontendMetrics.idConverterDetectConflictRate.mark(); | ||
| if (restRequest.getRestMethod() == RestMethod.PUT && RestUtils.getRequestPath(restRequest) |
There was a problem hiding this comment.
Can we verify the Dataset behavior?
Does it allow update with the same version number if upsert is not set?
ankagrawal
force-pushed
the
ankuagra_check_named_conflict
branch
from
3ec9fa9
to
1d98605
Compare
If the upload or stitch operation of a named blob is done without
x-ambry-named-upsertheader set to true, then update to the named blob is not allowed, and Ambry should return http 409 Conflict error code.When handling such requests, its efficient to first check for a conflict, and then do the work of saving the blob to storage node. This PR makes the change to first check for conflict, and do the upload only if there is no conflict.