Size limit for tasks?

[Paul Goracke]

On Apr 21, 2008, at 8:33 PM, Paul Goracke wrote:

> On Apr 21, 2008, at 1:23 PM, Jonathan Steinert wrote:
>
>> I fixed this in trunk around r337 for the perl client. There was
>> an implicit job size limitation that I removed.
>>
>> And then around r339 I made 'large' jobs a lot faster.
>>
>> If you can, please try using trunk/HEAD to see if it removes your
>> problem. I'll see what I can do to usher along a release.
>
> This looks like it works--thanks very much!
>
> For future reference, I checked out trunk, currently r366, and it  
> passed my test script on sizes up to 25MB which should be large  
> enough for me for now :^)
>
> I haven't installed it into our "real product" yet, but you can  
> assume it runs fine unless you hear from me later.

The bad news: It doesn't run fine. :^(

The good news: I have a proposed fix.

The tests ran fine when client, server and worker were all on the same  
box. Once I spread it out to multiple systems, I encountered failures-- 
less frequent than before, and failing in the worker job handling  
routine instead of somewhere in the client-server-worker handoff like  
before. Luckily I had already come close to finding the issue before  
your suggestion to try the updated code, so I wasn't completely at  
square one again.

Gearman::Util::read_res_packet() is responsible for reading replies.  
In the 1.09 CPAN release, it calls sysread, then compares the length  
read with the length specified in the message header; if those don't  
match, it fails the job with an error of 'short_body' before passing  
it to the worker's handler. This is what I was experiencing.

The current code loops over multiple calls to sysread, putting each  
retrieved block into an array which it then joins when finished  
reading. While definitely improved, it's wrapped in a loop which is  
limited to a seemingly arbitrary 20 iterations. Once 20 "chunks" have  
been received, it joins the chunks into a buffer and passes that to  
the worker's handler, bypassing the length check against the header  
spec. If your arg isn't completely passed in 20 reads, the handler  
receives an incomplete arg string, and running something like Perl's  
thaw() method against it will fail by returning an undef value,  
indicative of a premature end of buffer.

My fix is to loop over sysread until everything is read, or error out  
if sysread returns without reading anything and the full length hasn't  
been received by changing lines 112-117 of Util.pm from:

         for (my $i = 0; $readlen > 0 && $i < 20; $i++) {
             my $rv = sysread($sock, $buffers[$i], $readlen);
             return $err->("short_body") unless $rv > 0;
             last unless $rv > 0;
             $readlen -= $rv;
         }

to:

         while ( $readlen > 0 ) {
             my $buffer;
             my $rv = sysread($sock, $buffer, $readlen);
             return $err->("short_body") unless $rv > 0;
             push( @buffers, $buffer );
             $readlen -= $rv;
         }

Or, as a svn diff:

Index: api/perl/Gearman/lib/Gearman/Util.pm
===================================================================
--- api/perl/Gearman/lib/Gearman/Util.pm        (revision 366)
+++ api/perl/Gearman/lib/Gearman/Util.pm        (working copy)
@@ -109,10 +109,11 @@
          # the number of memory allocations we have to do.
          my $readlen = $len;
          my @buffers;
-        for (my $i = 0; $readlen > 0 && $i < 20; $i++) {
-            my $rv = sysread($sock, $buffers[$i], $readlen);
+        while ( $readlen > 0 ) {
+            my $buffer;
+            my $rv = sysread($sock, $buffer, $readlen);
              return $err->("short_body") unless $rv > 0;
-            last unless $rv > 0;
+            push( @buffers, $buffer );
              $readlen -= $rv;
          }
          $buf = join('', @buffers);


===================================================================

pg