Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[10.x] Ensure array driver expires values at the expiry time #48497

Merged
merged 1 commit into from
Sep 22, 2023

Conversation

timacdonald
Copy link
Member

@timacdonald timacdonald commented Sep 22, 2023

The array cache driver's logic for determining when a key has expired is wrong.

It currently requires the time to be PAST the expiry - but the value indicates when the key expires at, thus if it is currently the time that it expires, it should have expired.

This means the array driver does not currently function like Redis. You can demo this yourself using the following script to test.

Make sure to test with array and with redis to see the difference.

You will notice that the array driver cannot make another hit until 1 second past the NEXT minute while Redis is able to make a hit at the very start of the second (give or take a few hundred milliseconds for communicating with Redis itself).

Artisan::command('limit', function () {
    while (now()->second !== 0) {
        echo '.';

        usleep(100_000);
    }

    echo PHP_EOL;

    $continue = true;

    while ($continue) {
        $continue = RateLimiter::attempt('echo-message', 10, function() {
            echo 'Executing rate limited code at '.now()->toDateTimeString().PHP_EOL;
        });
    }

    sleep(1);

    while (now()->second !== 0) {
        echo '.';

        usleep(100_000);
    }

    echo PHP_EOL;

    $continue = true;

    while (true) {
        $continue = RateLimiter::attempt('echo-message', 10, function() {
            echo 'Executing rate limited code at '.now()->toDateTimeString('m').PHP_EOL;
        });
    }
});

Array

Waiting until the start of the minute...
.............................................................................................................................................
.............................................................................................................................................
...............................................................
Executing rate limited code at 2023-09-22 05:56:00
Waiting until the start of the NEXT minute...
.............................................................................................................................................
.............................................................................................................................................
.............................................................................................................................................
.............................................................................................................................................
.........
Executing rate limited code at 2023-09-22 05:57:01.000

Redis

Waiting until the start of the minute...
.............................................................................................................................................
.............................................................................................................................................
..................................................................................................................................
Executing rate limited code at 2023-09-22 05:50:00
Waiting until the start of the NEXT minute...
.............................................................................................................................................
.............................................................................................................................................
.............................................................................................................................................
.............................................................................................................................................
.......
Executing rate limited code at 2023-09-22 05:51:00.054

The included test is one I was already writing that lead me down the path of testing all of this - so I left it in for us.

@timacdonald timacdonald marked this pull request as ready for review September 22, 2023 06:18
@timacdonald timacdonald force-pushed the array-cache-expiry branch 2 times, most recently from 0c00c9f to 4c1dc79 Compare September 22, 2023 06:39
@timacdonald timacdonald marked this pull request as draft September 22, 2023 07:00
@timacdonald timacdonald marked this pull request as ready for review September 22, 2023 07:01
@taylorotwell taylorotwell merged commit ebb780c into laravel:10.x Sep 22, 2023
20 checks passed
@timacdonald timacdonald deleted the array-cache-expiry branch September 25, 2023 05:27
driesvints added a commit that referenced this pull request Sep 28, 2023
taylorotwell pushed a commit that referenced this pull request Sep 28, 2023
* Revert "Fix array cache driver expiry (#48497)"

This reverts commit ebb780c.

* Revert PR #48543
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants