-
Notifications
You must be signed in to change notification settings - Fork 276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split the pr to support named blob ttl update only #2542
Conversation
6f03da5
to
7737676
Compare
7737676
to
90d2f5c
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2542 +/- ##
=============================================
+ Coverage 33.05% 53.61% +20.55%
- Complexity 4428 7642 +3214
=============================================
Files 794 801 +7
Lines 63782 63947 +165
Branches 7793 7801 +8
=============================================
+ Hits 21086 34284 +13198
+ Misses 40479 26669 -13810
- Partials 2217 2994 +777
☔ View full report in Codecov by Sentry. |
89b2550
to
90af4af
Compare
67fdd8d
to
a6b696c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
@@ -764,6 +764,30 @@ private void applySoftDelete(short accountId, short containerId, String blobName | |||
} | |||
} | |||
|
|||
@Override | |||
public CompletableFuture<PutResult> ttlUpdate(NamedBlobRecord record, NamedBlobState state) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we have a dedicated sql statement for updating ttl. And there should be some limitation to when a named blob's ttl can be updated, like if there is an expiration date, it needs to be more than one day. We should share the same configuration with what is in the server nodes. Maybe we can leave this logic out side of mysql database and have it in the http request handler?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed offline, I think I can re-use this updateBlobStateToReady method, just change the name to updateBlobTtlAndStateToReady to be more clear.
And in covertId logic, when operation is TTL_UPDATE, we just get the named blob for id and version.
conversionFuture = getNamedBlobDb().get(namedBlobPath.getAccountName(), namedBlobPath.getContainerName(), namedBlobPath.getBlobName(), getOption).thenApply(result -> { restRequest.setArg(RestUtils.InternalKeys.NAMED_BLOB_VERSION, result.getInsertedRecord().getVersion()); restRequest.setArg(RestUtils.InternalKeys.NAMED_BLOB_MAPPED_ID, result.getInsertedRecord().getBlobId()); return result.getInsertedRecord().getBlobId(); });
And in ttlUpdateHanlder -> routerCallback we can directly call this method. updateBlobTtlAndStateToReady with the blobId and Version updated in the newNamedBlobRecord.
NamedBlobRecord record = new NamedBlobRecord(namedBlobPath.getAccountName(), namedBlobPath.getContainerName(), namedBlobPath.getBlobName(), (String) restRequest.getArgs().get(NAMED_BLOB_MAPPED_ID), Utils.Infinite_Time, namedBlobVersion); namedBlobDb.updateBlobStateToReady(record).get();
be3d55b
to
8b70ca8
Compare
No description provided.