Skip to content

[1.x] Rework pulse:check command #314

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

Merged
merged 1 commit into from
Mar 4, 2024
Merged

[1.x] Rework pulse:check command #314

merged 1 commit into from
Mar 4, 2024

Conversation

timacdonald
Copy link
Member

@timacdonald timacdonald commented Feb 13, 2024

The current implementation of the pulse:check command is based on our earlier version of Pulse. Due to this, there is a lot of things that feel clunky and don't work as reliably as they could.

This PR rethinks the command and also how recorders that interact with the shared beats can work.

It also allows the command to run a single time when run within a Laravel Vapor's scheduler.

Improvements

Recorders previously had to do some modulus math if they only wanted to record every now an then. The timeframes were limited to 5 second increments. For example, if we wanted our recorder to take a snapshot every 15 seconds we would do the following...

public function record(SharedBeat $event)
{
    if ($event->time->second % 15 !== 0) {
        return;
    }

    // Record...
}

You now may use the much more reliable Throttling concern, that ships with Pulse, to throttle recording to a max of every 15 seconds. The throttled duration can now be any arbitrary interval, although the beat will only be triggered roughly once per second.

This makes recording on any given interval much more reliable than it was previously, which could miss intervals if listeners took too long.

use Concerns\Throttling;

public function record(SharedBeat $event)
{
    $this->throttle(15, $event, function () {
        // Record...
    });
}

For shared beats this will throttle to the current server. For isolated beats this will throttle across servers.

Implementation notes

  • The lastSnapshotAt has been removed. This was not always accurate, and sometimes it was simply a lie. For example, the first time you run the command it will tell you it was run 5 seconds ago, which it was not.
  • The 5 second interval is now arbitrary. It was important in our early days, but no longer makes sense.
  • The time rounding was important in previous iterations of Pulse, but is no longer relevant.
  • The "now" value is captured before we dispatch events, rather than before we potentially sleep. This was just bad regardless.
  • Beats are now triggered roughly once a second.

@timacdonald timacdonald force-pushed the check branch 2 times, most recently from 5d79cd5 to 71bbc93 Compare February 14, 2024 05:07
@timacdonald timacdonald marked this pull request as ready for review February 22, 2024 03:32
@timacdonald timacdonald marked this pull request as draft February 22, 2024 03:33
@timacdonald timacdonald marked this pull request as ready for review March 4, 2024 04:42
@taylorotwell taylorotwell merged commit 5048e24 into 1.x Mar 4, 2024
@taylorotwell taylorotwell deleted the check branch March 4, 2024 15:23
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.

3 participants