mschilli / net-google-drive-simple

Net::Google::Drive::Simple CPAN Module
Other
11 stars 22 forks source link

endless loop in http_loop #3

Closed xrmb closed 11 years ago

xrmb commented 11 years ago

Today I ended up with non-stop errors like this: Failed with 400: Bad Request at C:/Perl/32/5.16/site/lib/Net/Google/Drive/Simple.pm line 465. Failed with 400: Bad Request at C:/Perl/32/5.16/site/lib/Net/Google/Drive/Simple.pm line 465. Failed with 400: Bad Request at C:/Perl/32/5.16/site/lib/Net/Google/Drive/Simple.pm line 465. Failed with 400: Bad Request at C:/Perl/32/5.16/site/lib/Net/Google/Drive/Simple.pm line 465. Failed with 400: Bad Request at C:/Perl/32/5.16/site/lib/Net/Google/Drive/Simple.pm line 465. Failed with 400: Bad Request at C:/Perl/32/5.16/site/lib/Net/Google/Drive/Simple.pm line 465.

(the line number is off because I added some debug code)

The problem lies here (I think): [code] if( --$RETRIES >= 0 ) { ERROR "Retrying in $SLEEP_INTERVAL seconds"; sleep $SLEEP_INTERVAL; redo; } else { die "Out of retries."; } [/code]

redo needs a loop, but there is no loop. I'm actually worried that perl doesnt warn about that. At least i'm not used to it, maybe it is old legacy perl. Anyway, your redo will just gob back the beginning of the function, and reset the $RETRIES, so it does it forever.

you might better off with this: [code] my $SLEEP_INTERVAL = 10;

for(;;)
{

[/code]

...on the other hand I'm glad my script didn't die, no I need to start eval all API calls.

Also, the cause for the endless loop is a corrupted yml config file. It happens maybe once a day that it becomes 0 bytes in size. I assume the API is not thread-safe and it overwrites it/forgets its contents. Don't know if that is bug, but it is the only threading issue I had so far. I dont have a good idea how to fix that, maybe the lock function can prevent it. For now I keep a backup file. Maybe a check if reading the config was successful helps, not that it would cure the problem, but solve the symptoms.

mschilli commented 11 years ago

Actually, Perl doesn't need a loop for a redo, just a block, and that's what the module is using.

As for your threading issues: writing a yml file isn't an atomic operation and therefore not thread- (or process-) safe. What's your use case?

xrmb commented 11 years ago

Well, I did not know that little redo feature, and it is not what the documentation says. I'm puzzled how it knows which block you want to redo, why not redo the if-block?

The multi-threading, I'm experimenting with a sync script, and its a lot of small files, so I'm using 5 threads to speed it up. But from my latest benchmarking it is not really faster.

Now back to the yml saving. I change my code to have a different file for each thread, so you're good :) No more killing the yml.

But there is this questionable code in the file_upload:

Since a file upload can take a long time, refresh the token

  # just in case.
$self->token_expire();

That might be right, but back to all my little files, it refreshes the token every 1-2 seconds. I havent tried it, but I'd expect that even if I upload a 10mb file 2seconds before the token expires it will work. The token is in the request header, which should be parsed before processing the data. Have you tried it? I commented it out, lets see what happens ;)

mschilli commented 11 years ago

On Wed, 20 Mar 2013, xrmb wrote:

Well, I did not know that little redo feature, and it is not what the documentation says. I'm puzzled how it knows which block you want to redo, why not redo the if-block?

The docs are a bit fuzzy on this, they say that you can't use redo in a "block that returns a value like sub {} or eval {} or do {} ", but "if" falls into the same category.

But there is this questionable code in the file_upload: # Since a file upload can take a long time, refresh the token # just in case. $self->token_expire();

Yeah, that's a bit drastic :). I ran into issues with expired tokens on long files.

I havent tried it, but I'd expect that even if I upload a 10mb file 2seconds before the token expires it will work. The token is in the request header, which should be parsed before processing the data. Have you tried it?

Yes, that's exactly what happened. When a file upload took longer than the token expiration time, Google aborted the upload and sent back an error. Since it's impossible to know how long the upload will take, refreshing the token beforehand seemed like the best option. It's probably a bug on the Google end, though.

But that's a bad strategy for small files, I agree, I'll take the refresh out of file_upload() and will put a note into the docs to recommend a manual refresh in the application before starting a long upload.

-- Mike

Mike Schilli m@perlmeister.com