vsivsi / meteor-file-collection

Extends Meteor Collections to handle file data using MongoDB gridFS.
http://atmospherejs.com/vsivsi/file-collection
Other
159 stars 37 forks source link

Upload stalled with resumableJs #157

Open jerradpatch opened 7 years ago

jerradpatch commented 7 years ago

while uploading many files with the resumable browser client. My files are stalling (the browser shows no network activity after on the client after the 10th upload). Somethings to note, I upload each file sequentially waiting for each file to finish before uploading the next. So, add resumable file then upload, 'on.fileCompleted' start next upload. Also, all 10 files are working (returned to browser). Where would be some places for me to start looking?

jerradpatch commented 7 years ago

here is some more info, "warning: possible EventEmitter memory leak detected. 101 error listeners added." I increased the limit from 10 to 100, so their seems to be a memory leak of some sort.

jerradpatch commented 7 years ago

here is how I upload the item and wait

collection.resumable.files = [resFile];
        collection.resumable.on('fileSuccess', (file,message)=>{
            let idA = resFile.uniqueIdentifier;
            let idB = resFile.uniqueIdentifier;
            if(idA == idB) {
                console.log('file upload', 'completed', collection.base);
                c();
            }
        });
        collection.resumable.on('fileError', (err) => {
            console.log('file upload','error', collection.base);
            f(err);
        });
        collection.resumable.upload();

is there some type of clean up that I need to do?

jerradpatch commented 7 years ago

I always seems to timeout at the beginning of uploading a new file. here are some requests

curl 'http://localhost:3002/gridfs/Videos/_resumable?resumableChunkNumber=1&resumableChunkSize=2096128&resumableCurrentChunkSize=2096128&resumableTotalSize=193473546&resumableType=video%2Fmp4&resumableIdentifier=6b838228daa47c3a1086fcf6&resumableFilename=Scums_Wish--6--1487286845.mp4&resumableRelativePath=Scums_Wish--6--1487286845.mp4&resumableTotalChunks=92' -X HEAD -H 'Referer: http://localhost:3002/' -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36' --compressed

curl 'http://localhost:3002/gridfs/Videos/_resumable?resumableChunkNumber=2&resumableChunkSize=2096128&resumableCurrentChunkSize=2096128&resumableTotalSize=193473546&resumableType=video%2Fmp4&resumableIdentifier=6b838228daa47c3a1086fcf6&resumableFilename=Scums_Wish--6--1487286845.mp4&resumableRelativePath=Scums_Wish--6--1487286845.mp4&resumableTotalChunks=92' -X HEAD -H 'Referer: http://localhost:3002/' -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36' --compressed

curl 'http://localhost:3002/gridfs/Videos/_resumable?resumableChunkNumber=3&resumableChunkSize=2096128&resumableCurrentChunkSize=2096128&resumableTotalSize=193473546&resumableType=video%2Fmp4&resumableIdentifier=6b838228daa47c3a1086fcf6&resumableFilename=Scums_Wish--6--1487286845.mp4&resumableRelativePath=Scums_Wish--6--1487286845.mp4&resumableTotalChunks=92' -X HEAD -H 'Referer: http://localhost:3002/' -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/49.0.2623.87 Safari/537.36' --compressed

all of these will just pend in the browser indefinitely.

vsivsi commented 7 years ago

Hi, do you see this with the sample app? I need you to help me narrow down whether this is a browser-side (and hence resumable.js) problem, or a server-side problem. The sample app can easily upload more than 10 files without any problems.

If you can commit a minimal reproduction codebase (with instructions) to a repo on Github, I'll be happy to take a look.

vsivsi commented 7 years ago

FYI, here is the sample app. Try uploading however many video files using this app. I've uploaded many hundreds of files (drag and drop) with this code without the types of problems you are describing.

https://github.com/vsivsi/meteor-file-sample-app

jerradpatch commented 7 years ago

ok thanks, I will take a look at it. I have gotten around the hanging.

Original Issue: I was having component that each had their own uploader, and I was creating registering file events for each component. That is not how resumible.js works unfortunately. I have to manually unregister the events when the components were removed. Then it started working. However, I could only get it to chunk the file through the 'fileAdded' event, so I am stuck using it; even if I call .rebuild().

New Issue: event emitter leak.

at GridReadStream. (packages/vsivsi_file-collection/src/gridFS_server.coffee:300:33) W20170408-20:26:22.359(-4)? (STDERR) at emitNone (events.js:67:13) W20170408-20:26:22.360(-4)? (STDERR) at GridReadStream.emit (events.js:166:7) W20170408-20:26:22.361(-4)? (STDERR) at EventEmitter. (/home/jerrad/.meteor/packages/vsivsi_file-collection/.1.3.6.v9a47a++os+web.browser+web.cordova/npm/node_modules/gridfs-locking-stream/index.js:205:54) W20170408-20:26:22.361(-4)? (STDERR) at emitOne (events.js:77:13) W20170408-20:26:22.362(-4)? (STDERR) at EventEmitter.emit (events.js:169:7) W20170408-20:26:22.371(-4)? (STDERR) at EventEmitter.emitExpiresSoonEvent (/home/jerrad/.meteor/packages/vsivsi_file-collection/.1.3.6.v9a47a++os+web.browser+web.cordova/npm/node_modules/gridfs-locks/index.js:436:8)

I will look at your app and see if the event emitter leak occurs their as well. Thanks for the sample.

vsivsi commented 7 years ago

Okay, let me know how it goes.

jerradpatch commented 7 years ago

meteor-file-collection Version 1.3.6 might have been fixed in 1.3.7? "Fixed (via gridfs-locking-stream npm package) an event memory leak when renewing locks on open files." Let me update and come back. So, I have just gotten back to this and here is where I have gotten to. Line 300 of gridFS_server.coffee, calls .renewLock(...)

readStream.on 'expires-soon', () =>
 readStream.renewLock (e, d) ->
    if e or not d
      console.warn "Automatic Read Lock Renewal Failed: #{file._id}", e

renewLock, sets a timeout that emits a 'expires-soon' event. Then renewLock gets called again, so the resource seems to always be being renewed (even after the page is closed). I proved this by putting a console.out in the loop, and it just gets called and called. This wouldn't give this warning except that renewLock() creates two EventEmitters, in gridfs-locking-stream/index.js at line 186

if (callback) {
 lock.once('renewed', function (l) { console.log("renewed event", this.fileId); callback(null, l); });
 lock.once('error', function (e) { lock.removeAllListeners(); callback(e); });
}

It seems something should be done to break the cycle of renewing the lock (in gridFS_server.coffee)?

In addition to this, I question if the (line 188 of gridfs-locking-stream/index.js)

lock.once('error', function (e) { lock.removeAllListeners(); callback(e); });

is never removed because the 'error' event never occurs. It's likely because the warnings start occurring right when the renewLocks() is called 6 times, which is what I have my max event listeners set to (for debugging). Your thoughts on this?

jerradpatch commented 7 years ago

Yes, upgrading fixed the EventEmitter leak. However, I still notice that the read stream is still calling the renew lock on the file long after the page has been closed (20 minutes after).

vsivsi commented 7 years ago

Hmm. It's been over a year since I've looked deeply at this code...

Is this happening (the lock renewal loop) when a client aborts a download before it is complete? How consistently does this happen?

Here's my theory... If the client aborts the download, then the read stream (from GridFS) is never read to the end. Now express.js should be closing the read stream when the TCP connection drops, and so a close event should propagate back to gridfs-locking-stream resulting in a release of the lock (and stopping all pending expiration events, etc).

See: https://github.com/vsivsi/gridfs-locking-stream/blob/master/index.js#L209-L211

But maybe there is a corner case or race condition here where that doesn't get done.

To dig into this, I would need a simple reproduction that preferably works every time it is run.

In addition to this, I question if the (line 188 of gridfs-locking-stream/index.js) is never removed because the 'error' event never occurs.

(Actually line 185...) That line goes hand-in-hand with the one above it. One or the other of those will be called eventually. https://github.com/vsivsi/gridfs-locking-stream/blob/master/index.js#L184-L185