[ 
https://issues.apache.org/jira/browse/SLING-2707?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13757431#comment-13757431
 ] 

Shashank Gupta commented on SLING-2707:
---------------------------------------

Thanks Alex for your detailed feedback. much appreciated. I agree on most of 
them.

>are the node types required in the jcr/resource bundle, in a cnd which 
>otherwise only defines the sling:Resource node type?
No. Yes I think it can be moved to slingpostbundle.

>IMO the :applyToChunks flag is not necessary, if you want to abort, you simple 
>DELETE (or :operation=delete) the target file
It is required in update case where you don't want to delete original file. As 
on now, delete chunks on new file results in empty nt:file. It should delete 
the file itself. wdyt?

>Also, we cannot rely that this is always sent correctly by the client, the 
>usual "abort" case will be that the upload has been aborted and nothing else 
>is happening anymore. We need to handle partially uploaded nt:file nodes in 
>some way then... garbage collection?
Yes I agree. Scheduled job is required which deletes all partial uploads which 
are 12hrs older. frequency of job say 6hrs. It would be configurable anyway. 

>only sequential chunk upload allowed (chunkOffset != currentLength)
Yes. parallel not supported as of now. there would be other considerations too 
for parallel chunks in current implementation. 
As per few first comments on this issue, community doesn't support idea of 
parallel chunk upload, although my experiment with cq deployed on AWS cloud 
clearly demonstrated the advantage [1]. As per your comment too "Agree with 
Julian & Felix: #2 (resume broken upload) makes sense, but I don't get why 
parallel uploads should help. What was used as server/endpoint for the above 
comparison?"

I would be happy to support parallel chunk upload, if community agrees to it. 

[1]https://issues.apache.org/jira/browse/SLING-2707?focusedCommentId=13552658&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13552658

>the totalLength != expectedLength check in line 286 seems wrong to me
File length sent in first chunk upload request is persisted in 
"sling:fileLength". now on subsequent requests the @Length parameter is 
validated with the persisted value. if there is mismatch, proper exception is 
thrown. Total file length cannot change between first and subsequent chunk 
upload request. 

>What else is the need for the sling:length property then? IMO it's not needed, 
>too easy to get out of sync with what's in the sling:chunk child nodes anyway
yes it can be derived. you need to read all chunks and add their respecitive 
sizes. I kept it simple to keep it in a property. Also user can see current 
size from UI anytime. 

>Is this thread-safe? Assuming multiple chunk requests coming in in parallel.
Yes unless they do on same node. Test cases 
SlingPostChunkUploadTest#testConcurrentChunkUpload and 
SlingPostChunkUploadTest#testChunkUploadOnExistingNode covered the scenario. 

>Merging is done through a temporary file stored on the file system.
yes this can be improved. 

> RequestProperty extensions (getOffset, getLength, etc.) might be useful to 
> separate out into a Chunk object..
Yes it can be re-factored. 

                
> Support of chunked file upload into Sling
> -----------------------------------------
>
>                 Key: SLING-2707
>                 URL: https://issues.apache.org/jira/browse/SLING-2707
>             Project: Sling
>          Issue Type: New Feature
>          Components: Servlets
>            Reporter: Shashank Gupta
>            Assignee: Carsten Ziegeler
>             Fix For: Servlets Post 2.3.4
>
>         Attachments: SLING-2707.patch, SLING-2707-svn.patch, uploadclient.jar
>
>
> Use cases: 
> 1. Large file upload - With high speed internet connections, advent of cloud 
> and HD going mainstream, Sling should support large files (> 2GB) upload.
> 2. Fault tolerant uploads - Sling should provide capability to resume upload 
> from failure point. It should not require client to restart the complete 
> upload process. 
> 3. Faster upload: Sling should support its clients to initiate multiple 
> connection and upload file chunks in parallel. 

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to