-
Notifications
You must be signed in to change notification settings - Fork 113
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
use tusd datastorages #4148
use tusd datastorages #4148
Conversation
Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes. |
0289878
to
6b85fe4
Compare
dd5f382
to
171c9af
Compare
eac4bc5
to
4e34553
Compare
@micbar so the tests expect only one version to exist when a file is uploaded with the same mtime. but the code now keeps track of every revision: Scenario: upload the same file twice with the same mtime and a version is available # /drone/src/tmp/testrunner/tests/acceptance/features/coreApiVersions/fileVersions.feature:251
Given user "Alice" has uploaded file "filesForUpload/textfile.txt" to "file.txt" with mtime "Thu, 08 Aug 2019 04:18:13 GMT" using the WebDAV API # FeatureContext::userHasUploadedFileToWithMtimeUsingTheWebdavApi()
When user "Alice" uploads file "filesForUpload/textfile.txt" to "file.txt" with mtime "Thu, 08 Aug 2019 04:18:13 GMT" using the WebDAV API # FeatureContext::userUploadsFileToWithMtimeUsingTheWebdavApi()
Then the HTTP status code should be "204" # FeatureContext::thenTheHTTPStatusCodeShouldBe()
And the version folder of file "/file.txt" for user "Alice" should contain "1" element # FilesVersionsContext::theVersionFolderOfFileShouldContainElements()
could not find 1 version element(s) in
<?xml version="1.0"?>
<d:multistatus xmlns:s="http://sabredav.org/ns" xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns"><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$88f60b76-eb21-103d-957c-43ed0cdaa8aa%21a0cc9d8f-5794-41e5-9e27-282999ba5df3/</d:href><d:propstat><d:prop><d:getetag/></d:prop><d:status>HTTP/1.1 404 Not Found</d:status></d:propstat></d:response><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$88f60b76-eb21-103d-957c-43ed0cdaa8aa%21a0cc9d8f-5794-41e5-9e27-282999ba5df3/v/a0cc9d8f-5794-41e5-9e27-282999ba5df3.REV.2023-09-19T10:17:51.960713985Z</d:href><d:propstat><d:prop><d:getetag>"b1e23aeabf59d10f5daadc021b988fd5"</d:getetag></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$88f60b76-eb21-103d-957c-43ed0cdaa8aa%21a0cc9d8f-5794-41e5-9e27-282999ba5df3/v/a0cc9d8f-5794-41e5-9e27-282999ba5df3.REV.2023-09-19T10:17:51.926984925Z</d:href><d:propstat><d:prop><d:getetag>"a431c21b9be6bd1327e1f06b40c74d0b"</d:getetag></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response></d:multistatus>
Failed asserting that 2 matches expected 1.
Scenario: upload the same file more than twice with the same mtime and only one version is available # /drone/src/tmp/testrunner/tests/acceptance/features/coreApiVersions/fileVersions.feature:258
Given user "Alice" has uploaded file "filesForUpload/textfile.txt" to "file.txt" with mtime "Thu, 08 Aug 2019 04:18:13 GMT" using the WebDAV API # FeatureContext::userHasUploadedFileToWithMtimeUsingTheWebdavApi()
And user "Alice" has uploaded file "filesForUpload/textfile.txt" to "file.txt" with mtime "Thu, 08 Aug 2019 04:18:13 GMT" using the WebDAV API # FeatureContext::userHasUploadedFileToWithMtimeUsingTheWebdavApi()
When user "Alice" uploads file "filesForUpload/textfile.txt" to "file.txt" with mtime "Thu, 08 Aug 2019 04:18:13 GMT" using the WebDAV API # FeatureContext::userUploadsFileToWithMtimeUsingTheWebdavApi()
Then the HTTP status code should be "204" # FeatureContext::thenTheHTTPStatusCodeShouldBe()
And the version folder of file "/file.txt" for user "Alice" should contain "1" element # FilesVersionsContext::theVersionFolderOfFileShouldContainElements()
could not find 1 version element(s) in
<?xml version="1.0"?>
<d:multistatus xmlns:s="http://sabredav.org/ns" xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns"><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$8911b43e-eb21-103d-957f-43ed0cdaa8aa%214cfb2093-1f4c-485a-bcff-69c601d5d93c/</d:href><d:propstat><d:prop><d:getetag/></d:prop><d:status>HTTP/1.1 404 Not Found</d:status></d:propstat></d:response><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$8911b43e-eb21-103d-957f-43ed0cdaa8aa%214cfb2093-1f4c-485a-bcff-69c601d5d93c/v/4cfb2093-1f4c-485a-bcff-69c601d5d93c.REV.2023-09-19T10:17:52.178843967Z</d:href><d:propstat><d:prop><d:getetag>"9b010889f9a3f345b2d872d6c8abe030"</d:getetag></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$8911b43e-eb21-103d-957f-43ed0cdaa8aa%214cfb2093-1f4c-485a-bcff-69c601d5d93c/v/4cfb2093-1f4c-485a-bcff-69c601d5d93c.REV.2023-09-19T10:17:52.144869863Z</d:href><d:propstat><d:prop><d:getetag>"7349ebd02bc0ba490ece6bab4c95ac15"</d:getetag></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$8911b43e-eb21-103d-957f-43ed0cdaa8aa%214cfb2093-1f4c-485a-bcff-69c601d5d93c/v/4cfb2093-1f4c-485a-bcff-69c601d5d93c.REV.2023-09-19T10:17:52.108406347Z</d:href><d:propstat><d:prop><d:getetag>"90d1fa707994f4f6a53232b538bb1bd9"</d:getetag></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response></d:multistatus>
Failed asserting that 3 matches expected 1.
Scenario: upload the same file twice with the same mtime and no version after restoring # /drone/src/tmp/testrunner/tests/acceptance/features/coreApiVersions/fileVersions.feature:266
Given user "Alice" has uploaded file "filesForUpload/textfile.txt" to "file.txt" with mtime "Thu, 08 Aug 2019 04:18:13 GMT" using the WebDAV API # FeatureContext::userHasUploadedFileToWithMtimeUsingTheWebdavApi()
And user "Alice" has uploaded file "filesForUpload/textfile.txt" to "file.txt" with mtime "Thu, 08 Aug 2019 04:18:13 GMT" using the WebDAV API # FeatureContext::userHasUploadedFileToWithMtimeUsingTheWebdavApi()
When user "Alice" restores version index "1" of file "/file.txt" using the WebDAV API # FilesVersionsContext::userRestoresVersionIndexOfFile()
Then the HTTP status code should be "204" # FeatureContext::thenTheHTTPStatusCodeShouldBe()
And the version folder of file "/file.txt" for user "Alice" should contain "0" element # FilesVersionsContext::theVersionFolderOfFileShouldContainElements()
could not find 0 version element(s) in
<?xml version="1.0"?>
<d:multistatus xmlns:s="http://sabredav.org/ns" xmlns:d="DAV:" xmlns:oc="http://owncloud.org/ns"><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$89344148-eb21-103d-9582-43ed0cdaa8aa%21546c80fa-3ed2-4cea-ae9c-0e33a552c7b2/</d:href><d:propstat><d:prop><d:getetag/></d:prop><d:status>HTTP/1.1 404 Not Found</d:status></d:propstat></d:response><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$89344148-eb21-103d-9582-43ed0cdaa8aa%21546c80fa-3ed2-4cea-ae9c-0e33a552c7b2/v/546c80fa-3ed2-4cea-ae9c-0e33a552c7b2.REV.2023-09-19T10:17:52.333974461Z</d:href><d:propstat><d:prop><d:getetag>"e32940ea5bae7aa1271e92cf6d38fde2"</d:getetag></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response><d:response><d:href>/remote.php/dav/meta/1284d238-aa92-42ce-bdc4-0b0000009157$89344148-eb21-103d-9582-43ed0cdaa8aa%21546c80fa-3ed2-4cea-ae9c-0e33a552c7b2/v/546c80fa-3ed2-4cea-ae9c-0e33a552c7b2.REV.2019-08-08T04:18:13Z</d:href><d:propstat><d:prop><d:getetag>"b2516215c4b4930ea43a608875ac06f4"</d:getetag></d:prop><d:status>HTTP/1.1 200 OK</d:status></d:propstat></d:response></d:multistatus>
Failed asserting that 2 matches expected 0. I'm not sure if we want this keep this arguably broken revision tracking? |
Thesize diff calculation might be racy:
|
No, the result is 15 10 + 5 + 15 -15 = 15 |
so the error seems to be that the revision has no blobid attribute set ... {"level":"error","error":"error reading blobid xattr: xattr.get /drone/src/tmp/reva/data/spaces/28/175508-ec25-103d-820e-9b9f4a005ed7/nodes/bd/1b/06/73/-b87d-49f8-bcab-535b42cc37dc.REV.2023-09-20T17:16:18.608228057Z.mlock user.ocis.blobid: no data available","revision":"/drone/src/tmp/reva/data/spaces/28/175508-ec25-103d-820e-9b9f4a005ed7/nodes/bd/1b/06/73/-b87d-49f8-bcab-535b42cc37dc.REV.2023-09-20T17:16:18.608228057Z.mlock","time":"2023-09-20T17:16:26.941046467Z","message":"error reading blobid attribute"}
�[90m2023-09-20 17:16:26.941�[0m �[1m�[31mERR�[0m�[0m �[1m../../../internal/grpc/services/storageprovider/storageprovider.go:981�[0m�[36m >�[0m failed to empty recycle �[36merror=�[0m�[31m"error reading blobid xattr: xattr.get /drone/src/tmp/reva/data/spaces/28/175508-ec25-103d-820e-9b9f4a005ed7/nodes/bd/1b/06/73/-b87d-49f8-bcab-535b42cc37dc.REV.2023-09-20T17:16:18.608228057Z.mlock user.ocis.blobid: no data available"�[0m �[36mkey=�[0m/ �[36mpid=�[0m12 �[36mpkg=�[0mrgrpc �[36mreference=�[0m{"path":".","resource_id":{"opaque_id":"28175508-ec25-103d-820e-9b9f4a005ed7","space_id":"28175508-ec25-103d-820e-9b9f4a005ed7","storage_id":"1284d238-aa92-42ce-bdc4-0b0000009157"}} �[36mstatus=�[0m{"code":15,"message":"error emptying recycle:error reading blobid xattr: xattr.get /drone/src/tmp/reva/data/spaces/28/175508-ec25-103d-820e-9b9f4a005ed7/nodes/bd/1b/06/73/-b87d-49f8-bcab-535b42cc37dc.REV.2023-09-20T17:16:18.608228057Z.mlock user.ocis.blobid: no data available","trace":"038deb2cd375cbe1eccb9e2e361a3c9b"} �[36mtraceid=�[0ma675d2a0acf71398263689c8bda6c54c
�[90m2023-09-20 17:16:26.941�[0m �[33mDBG�[0m �[1m../../../internal/grpc/interceptors/log/log.go:69�[0m�[36m >�[0m unary �[36mcode=�[0mOK �[36mend=�[0m"20/Sep/2023:17:16:26 +0000" �[36mfrom=�[0mtcp://127.0.0.1:53086 �[36mpid=�[0m12 �[36mpkg=�[0mrgrpc �[36mstart=�[0m"20/Sep/2023:17:16:26 +0000" �[36mtime_ns=�[0m10453361 �[36mtraceid=�[0ma675d2a0acf71398263689c8bda6c54c �[36muri=�[0m/cs3.storage.provider.v1beta1.ProviderAPI/PurgeRecycle �[36muser-agent=�[0mGuzzleHttp/7
�[90m2023-09-20 17:16:26.941�[0m �[33mDBG�[0m �[1m../../../internal/grpc/interceptors/log/log.go:69�[0m�[36m >�[0m unary �[36mcode=�[0mOK �[36mend=�[0m"20/Sep/2023:17:16:26 +0000" �[36mfrom=�[0mtcp://127.0.0.1:44532 �[36mpid=�[0m10 �[36mpkg=�[0mrgrpc �[36mstart=�[0m"20/Sep/2023:17:16:26 +0000" �[36mtime_ns=�[0m11953954 �[36mtraceid=�[0m3e1880e0396e5774c6c5ad2c2f55a4fd �[36muri=�[0m/cs3.gateway.v1beta1.GatewayAPI/PurgeRecycle �[36muser-agent=�[0mGuzzleHttp/7
�[90m2023-09-20 17:16:26.942�[0m �[1m�[31mERR�[0m�[0m �[1m../../../internal/http/services/owncloud/ocdav/errors/error.go:174�[0m�[36m >�[0m Internal Server Error �[36mcode=�[0m500 �[36mitem_path=�[0m/ �[36mkey=�[0m �[36mpid=�[0m9 �[36mpkg=�[0mrhttp �[36mreference=�[0m{"path":".","resource_id":{"opaque_id":"28175508-ec25-103d-820e-9b9f4a005ed7","space_id":"28175508-ec25-103d-820e-9b9f4a005ed7","storage_id":"1284d238-aa92-42ce-bdc4-0b0000009157"}} �[36mstatus=�[0m{"code":15,"message":"error emptying recycle:error reading blobid xattr: xattr.get /drone/src/tmp/reva/data/spaces/28/175508-ec25-103d-820e-9b9f4a005ed7/nodes/bd/1b/06/73/-b87d-49f8-bcab-535b42cc37dc.REV.2023-09-20T17:16:18.608228057Z.mlock user.ocis.blobid: no data available","trace":"038deb2cd375cbe1eccb9e2e361a3c9b"} �[36mtraceid=�[0m038deb2cd375cbe1eccb9e2e361a3c9b
�[90m2023-09-20 17:16:26.942�[0m �[1m�[31mERR�[0m�[0m �[1m../../../internal/http/interceptors/log/log.go:112�[0m�[36m >�[0m http �[36mend=�[0m"20/Sep/2023:17:16:26 +0000" �[36mhost=�[0m192.168.29.5 �[36mmethod=�[0mDELETE �[36mpid=�[0m9 �[36mpkg=�[0mrhttp �[36mproto=�[0mHTTP/1.1 �[36msize=�[0m0 �[36mstart=�[0m"20/Sep/2023:17:16:26 +0000" �[36mstatus=�[0m500 �[36mtime_ns=�[0m29996616 �[36mtraceid=�[0m038deb2cd375cbe1eccb9e2e361a3c9b �[36muri=�[0m/remote.php/dav/trash-bin/Alice �[36murl=�[0m/ |
|
Signed-off-by: Jörn Friedrich Dreyer <[email protected]> filter metadata Signed-off-by: Jörn Friedrich Dreyer <[email protected]> calculate sizediff on demand Signed-off-by: Jörn Friedrich Dreyer <[email protected]> fix empty bucketid Signed-off-by: Jörn Friedrich Dreyer <[email protected]> add changelog Signed-off-by: Jörn Friedrich Dreyer <[email protected]> drop unnecessary flag from cleanup Signed-off-by: Jörn Friedrich Dreyer <[email protected]> fix tests Signed-off-by: Jörn Friedrich Dreyer <[email protected]> drop unused variable Signed-off-by: Jörn Friedrich Dreyer <[email protected]> fix unit tests Signed-off-by: Jörn Friedrich Dreyer <[email protected]> move Finalize to upload package Signed-off-by: Jörn Friedrich Dreyer <[email protected]> move more functions to upload package Signed-off-by: Jörn Friedrich Dreyer <[email protected]> propagate sizediff on BytesReady and Revert on failure Signed-off-by: Jörn Friedrich Dreyer <[email protected]> reuse tus datastore from fs Signed-off-by: Jörn Friedrich Dreyer <[email protected]> always set file length when initiating uploads Signed-off-by: Jörn Friedrich Dreyer <[email protected]> log error if we cannot terminate upload Signed-off-by: Jörn Friedrich Dreyer <[email protected]> make linter happy Signed-off-by: Jörn Friedrich Dreyer <[email protected]> always send upload length Signed-off-by: Jörn Friedrich Dreyer <[email protected]> use s3ng config for datastore as well Signed-off-by: Jörn Friedrich Dreyer <[email protected]> allow configuring upload object prefixes and temp dir Signed-off-by: Jörn Friedrich Dreyer <[email protected]> fix s3ng test config Signed-off-by: Jörn Friedrich Dreyer <[email protected]> fix chunking v1 Signed-off-by: Jörn Friedrich Dreyer <[email protected]> fix chunking v1 --sign open existing file Signed-off-by: Jörn Friedrich Dreyer <[email protected]> restore size diff calculation Signed-off-by: Jörn Friedrich Dreyer <[email protected]> treat 0 byte uploads as size deferred Signed-off-by: Jörn Friedrich Dreyer <[email protected]> return empty reader for 0 byte files Signed-off-by: Jörn Friedrich Dreyer <[email protected]> comment change that always sends filesize in upload opaque Signed-off-by: Jörn Friedrich Dreyer <[email protected]> remove commented code Signed-off-by: Jörn Friedrich Dreyer <[email protected]> fix legacy chunking on s3 Signed-off-by: Jörn Friedrich Dreyer <[email protected]> comment size to check if that causes nc tests te fail Signed-off-by: Jörn Friedrich Dreyer <[email protected]> run single failing test Signed-off-by: Jörn Friedrich Dreyer <[email protected]> drop unused option Signed-off-by: Jörn Friedrich Dreyer <[email protected]> ignore mlock files when iterating revisions Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
0d70283
to
1293660
Compare
I rebased on top of latest reva. no big difference, looking at the current errors. I don't know what is going on with the S3NG driver. IT does not seem to change the checksum ... in the log all cheksums are the same:
must have something to do with the s3 backend ... the tests use the xattr metadata_backend ... will check that 👀 |
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
da39a3ee5e6b4b0d3255bfef95601890afd80709 is the hash for the empty string ... maybe reading back the upload from ceph does not work? |
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Indeed. The added log line appears: 👀
🤔 why are the uploads done via 👀 |
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
@@ -338,6 +338,15 @@ func (s *service) InitiateFileUpload(ctx context.Context, req *provider.Initiate | |||
}, nil | |||
} | |||
|
|||
// FIXME: This is a hack to transport more metadata to the storage.FS InitiateUpload implementation |
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.
I know, we already talked about it, but just for the record 😛 I would rather track the FIXME issues and thought-comments on github or drop them entirely. Either they are important and should be tracked and prioritized properly or they aren't, in which case the FIXMEs in the code are just ballast we'll carry on indefinitely.
@@ -63,6 +63,22 @@ func New(endpoint, region, bucket, accessKey, secretKey string) (*Blobstore, err | |||
}, nil | |||
} | |||
|
|||
func (bs *Blobstore) MoveBlob(node *node.Node, source, bucket, key string) error { | |||
|
|||
_, err := bs.client.CopyObject(context.Background(), minio.CopyDestOptions{ |
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.
I think we also need to delete the source after copying it.
Co-authored-by: Andre Duffeck <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
e1c897e
to
13fdc36
Compare
Everything will be stored in the upload session which we already have for our own metadata anyway.
13fdc36
to
be393b3
Compare
Signed-off-by: Jörn Friedrich Dreyer <[email protected]>
637142d
to
f8264b5
Compare
We evolved our own DataStore into a OcisSession that also covers postprocessing. |
We are currently ... misusing tusd. Misusing because we implemented our own tusd DataStore. This prevents us from using the existing tusd datastore implementations, eg. the filestore or s3store.
We already require calling InitializeUpload via cs3 before being able to send PATCH requests to the dataprovider. To integrate with tusd we should instead use the PreUploadCreateCallback and PreFinishResponseCallback. Actually, only the latter. The dataprovider never receive a POST directly, so the PreUploadCreateCallback is never called. Instead we need to make InitializeUpload call the same steps as the tusd unrouted handler. Then whe can hand off the complete tus process to tusd and wait for it to make the PreFinishResponseCallback call, where we then calculate checksums and create the actual decomposedfs node.
For this to work we cannot use tusds FileInfo.Storage because it is overwritten when calling NewUpload. So we have to put any metadata into the FileInfo.Metadata map. The latter is also persisted as
X-Amz-Meta-
in s3.This PoC works and allows uploading directly to a minio s3 bucket ... with hardcoded credentials ... well ... it is a PoC
TODO
X-Amz-Meta-
is a problem?not if metadata is filtered when it leaves ocis, eg. in the response of HEAD requests
Noteworthy changes
We need to return a fileid after initiating an upload. Currently, we roll the new uuid for new files and put it in the uplod metadata. ...
The problem: If there are two concurrent requests trying to create the same child they would receive different fileids.
We can no longer store the new nodeid, postprocessing status or any other metadata in the tus upload metadata. After creating the upload we have to use the node metadata itself (which actually makes sense because all bytes have been transferred).
This requires us to already roll the node id and create a file node in InitializeUpload, and mark it as in processing.
This in turn causes new files to show up in directory listings even if not all bytes have been transferred, yet.
hmmm ... how does the curren tcode deal with the problem?
info.Storage["NodeExists"] = "false"
initNewNode
to be calledwe could do the same in InitiateUpload ... using the symlink creation for atomicity instead of having to use a filelock
simple upload needs to initialize the file upload with the filesize
We currently have a way to decide how to initialize the SizeDeferred flag of a tus upload. We cannot use 0 because empty files do exist. This causes the sample upload to fail ...
InitiateFileUploadRequest does not even have a size proerty ...