Fix: FlatFileDriver does not notice new messages

[ackintosh]

FlatFileDriver does not notice new messages until the polling duration ends.
So we should update the $files. 💡

[acrobat]

I think we should check if the problem still persists after merge of #308 and merge this after! But already thanks for the work on this!

[ackintosh]

@acrobat #308 seems it is not active. 👀 Would you please merge this as before #308 ?

[acrobat]

@ackintosh I will take a look at this PR later today!

[acrobat]

Sorry for the delay! Changes look good to me, but can you add a test for this behavior (messages added after the initial collect)?

[ackintosh]

A test case for this behavior requires multiprocess(pop, push). Snidel (depends bernard!) provides the easiest way to achieve that.

[acrobat]

Can we use the php internal function pcntl_fork for this instead of snidel?

[acrobat]

No real big issue, just checking if we can avoid an extra dependency 😄

[ackintosh]

Yes. If that's what you want, I will rewrite it. 💡

[sagikazarmark]

This library looks really interesting, thanks for sharing. Nevertheless, I second @acrobat 's opinion: let's keep the dependencies at the absolute minimum if possible.

[ackintosh]

@acrobat @sagikazarmark Thank you for your review! I replaced Snidel with pcntl_fork.

[acrobat]

Can you please remove this added line as it is not project specific and should be added to your global .gitignore file

[acrobat]

Only a small comment about the gitignore file, the rest looks good! Thanks for your extra time to rewrite the tests!

[ackintosh]

I fixed it!

[sagikazarmark]

After going trough this PR I realized that it probably affects performance as well since between the loops it reads the whole file list again.

I would love to see some benchmarks comparing this version to the previous one, just to make sure it does not introduce a performance regression.

But I don't think it blocks merging @acrobat !

[acrobat]

I will try to do some benchmarks before merging, just to be sure we don't have (huge) performance regressions or if you are able to do some tests (or blackfire profiles) @ackintosh would be nice to! (Not sure if I will find time before mid next week or so)

[reinfi]

I would not say that this could cause a performance bug because it is only reading the directory again if there are no files.

The other solution would be to not enter the while loop because it makes no sense to run when there are no files. But this would cause the job to quit and it must be restarted manually or automatically.

So for my opinion this is the better solution as the runtime is used to find messages and not to always ask the driver again and again. But I think there is not the right way for it.

[ackintosh]

I've tried to benchmark with blackfire and it seems there's no critical performance problems.

Here is the sample code used on benchmark and the results of profile. Please let me know if there's something wrong on the result or my profiling.

---

• test.php

<?php
require_once(__DIR__ . '/vendor/autoload.php');

$driver = new \Bernard\Driver\FlatFile\Driver(__DIR__ . '/queue');

$driver->createQueue('test');

list($message,) = $driver->popMessage('test', 30);

var_dump($message);

• before(master)

https://blackfire.io/profiles/eb65f6df-c03f-43be-8fa4-be953c471c95/graph

• after(this PR)

https://blackfire.io/profiles/bd1aec80-1ee9-406e-bf62-8492a307c4b1/graph

[ackintosh]

Resolved the conflicts between master branch. 😉
0923aa3

[sagikazarmark]

Thanks @ackintosh !