Closed scriby closed 11 years ago
scriby
commented
11 years ago I experimented with adding a call to "request.abort" in the callback to the request method here: https://github.com/LearnBoost/knox/blob/57c656a17695a59a29c1a8fd243a176c4819e760/lib/client.js#L310.
It fixed the problem in the sense that I never saw more than X sockets open to s3, where X is the number of files I'm uploading in parallel
domenic
commented
11 years ago That does seem very bad. Some sample code would be helpful.
scriby
commented
11 years ago var s3 = knox.createClient({
key: 'redacted',
secret: "redacted",
bucket: "redacted",
region: 'us-standard'
});
var operations = [];
var walker = walk.walk(webRoot, { followLinks: false });
walker.on('file', function(root, stat, callback){
var fileName = stat.name;
var fullPath = path.join(root, fileName);
var s3Path = '/' + path.relative(webRoot, fullPath);
operations.push(function(callback){
console.log(s3Path);
s3.putFile(fullPath, s3Path, function(err){
console.log('done');
callback(err);
});
});
callback();
});
walker.on('end', function(){
async.forEachLimit(operations, 1, function(operation, callback){
operation(callback);
}, function(err){
if(err){
console.log(err);
}
console.log('Finished uploading web root to s3');
});
});Oh! Well of course. You're getting a response, but completely ignoring it! The second parameter passed to putFiles callback is a response object. You need to resume() it, or read it into a buffer, or some such.
And yes, this is kind of a footgun. Makes #114 seem more urgent...
scriby
commented
11 years ago Haha, I'll give that a try, thanks!
scriby
commented
11 years ago Apparently I can't read:
"// Always either do something with res or at least call res.resume()."
domenic
commented
11 years ago To be fair, we did just add that a couple days ago, and also it's pretty unintuitive. It's kind of a problem with how Node's request/response cycle works, but also kind of a problem with how we are adapting that API into a "simpler" callback API---it's a pretty leaky abstraction, as you can see.
scriby
commented
11 years ago I added a resume and things are quite happy. Want me to close this one out or want to keep it open as a placeholder to reduce the level of astonishment?
domenic
commented
11 years ago Let's close it for now. It was good to help us understand how much of a footgun this is, and will feed into #114.
I'm working on something that uploads a few thousand files to s3, and am continually having issues with network errors. I'm using knox in a pretty vanilla way, just using all the defaults (no agent, SSL, etc.)
I was attempting to do several uploads in parallel, but I dropped it down to one at a time which has been more reliable. But even when doing a single upload at a time, it seems like I have a bunch of connections that aren't closing.
This output is after running my program for about a minute. Eventually these build up, I run out file descriptors, and node starts throwing various errors.
Curious if you guys have ever seen something like this? I'm on node v0.10.15 and using knox 0.8.6.