amphp / beanstalk

Asynchronous Beanstalk Client for PHP.
https://amphp.org/beanstalk/
MIT License
66 stars 24 forks source link

With big payload reserved job is not the same as the put one #26

Closed mmenozzi closed 5 years ago

mmenozzi commented 5 years ago

It seems that for jobs with a big payload (for example a 64k payload) the reserve function doesn't return the entire payload which was put. Given the following script:

<?php
declare(strict_types=1);

require_once __DIR__ . '/vendor/autoload.php';

\Amp\Loop::run(function () {
    $client = new Amp\Beanstalk\BeanstalkClient("tcp://127.0.0.1:11300");
    yield $client->put(str_repeat('*', 65535));
    yield $client->watch('default');
    list($jobId, $job) = yield $client->reserve();
    echo "Job $jobId, length " . strlen($job) . PHP_EOL;
//    yield $client->bury($jobId);
    $client->quit();
    \Amp\Loop::stop();
});

The output is:

Job 1, length 65518

But it should be:

Job 1, length 65535

It seems a weird buffering issue because if you try to uncomment the bury function call the output become:

Job 1, length 65518
PHP Fatal error:  Uncaught Amp\Beanstalk\BeanstalkException: Unknown response: ***************** in /tmp/vendor/amphp/beanstalk/src/BeanstalkClient.php:203
Stack trace:
#0 /tmp/vendor/amphp/beanstalk/src/BeanstalkClient.php(74): Amp\Beanstalk\BeanstalkClient->Amp\Beanstalk\{closure}(Array)
#1 [internal function]: Amp\Beanstalk\BeanstalkClient->Amp\Beanstalk\{closure}()
#2 /tmp/vendor/amphp/amp/lib/Coroutine.php(76): Generator->send(Array)
#3 /tmp/vendor/amphp/amp/lib/Internal/Placeholder.php(130): Amp\Coroutine->Amp\{closure}(NULL, Array)
#4 /tmp/vendor/amphp/amp/lib/Deferred.php(45): class@anonymous->resolve(Array)
#5 /tmp/vendor/amphp/beanstalk/src/BeanstalkClient.php(38): Amp\Deferred->resolve(Array)
#6 /tmp/vendor/amphp/beanstalk/src/Connection.php(44): Amp\Beanstalk\BeanstalkClient->Amp\Beanstal in /tmp/vendor/amphp/beanstalk/src/BeanstalkClient.php on line 203

I think this happens because the remaining part of the job payload is then treated as the response of the bury command.

I pushed the reserved-not-as-put-fix branch with a failing test about this issue.

mmenozzi commented 5 years ago

@kelunik what do you think? Maybe the issue is in some underlying library...

mmenozzi commented 5 years ago

Here is the Travis failed build for the reserved-not-as-put-fix: https://travis-ci.org/amphp/beanstalk/builds/516073149

kelunik commented 5 years ago

@mmenozzi This is probably caused by the maximum job size, see e.g. https://stackoverflow.com/q/35782549/2373138, but given that, BeanstalkD should return an error in that case, which it doesn't.

mmenozzi commented 5 years ago

I don’t think so because the default maximum job size is 65535 bytes and I didn’t changed it. If you change the string length to 65536 you'll receive the proper job too big exception.

kelunik commented 5 years ago

@mmenozzi Right, this is some parser bug... fixed it. Should I tag a release or do you want to?

mmenozzi commented 5 years ago

Thank you @kelunik! I’ll tag the new release tomorrow afternoon.

mmenozzi commented 5 years ago

Here we go @kelunik: https://github.com/amphp/beanstalk/releases/tag/v0.2.5 🎉