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

Records auto-cleaner #317

Closed
didac-adria opened this issue Feb 15, 2024 · 8 comments
Closed

Records auto-cleaner #317

didac-adria opened this issue Feb 15, 2024 · 8 comments
Assignees

Comments

@didac-adria
Copy link

Hi!

I know this is in betta still, but I think the basic implementation of Pulse should contain a DB records cleaner. Otherwise they pile-up. Do you guys have something in mind already to incorporate a command to clean records older than 7 days (I think is the maximum amount of days to see data in the dashboards)? Or am I missing something and records should be deleted but somehow they aren't in our backend?

@driesvints
Copy link
Member

This already exists: https://laravel.com/docs/10.x/pulse#trimming

@didac-adria
Copy link
Author

Yeah I know @driesvints , but it's not working on our side. Maybe I don't understand the documentation. We have records almost a month old. I think the maximum age is 7 days.

@didac-adria
Copy link
Author

Just for clarification, we have the default configuration:

'trim' => [
            'lottery' => [1, 1_000],
            'keep' => '7 days',
        ],

Also, I'm trying to check the code to see how it works, because to my eyes, it's not clear how the lottery works. If I discover how it works, I may make a PR to the documentation to help.

@didac-adria
Copy link
Author

@driesvints We've been trying to investigate how it works but we don't reach solid conclusions:
1- We've tried to reduce the lottery to see if it prunes the records automatically. It doesn't
2- We have tried to add some logs to check for errors. We don't see any.

We have also realized that we have to manually restart the deamon from forge in order to make it get the new php code after each deployment.

So:
1- Can you give us a hint about how the flow works to make it clean and why 'lottery' => [1, 1], doesn't work?
2- Maybe we need a system to be able to restart the deamon of pulse:check in each deployment, the same way there is one for horizon.

Thanks in advance!

@sinnbeck
Copy link
Contributor

sinnbeck commented Feb 16, 2024

Hmm it seems there is a bug. I used xdebug and it never hits the trim method.

If I remove ... from trim() here it works. It is already inside a closure :)
https://github.com/laravel/pulse/blob/1.x/src/Pulse.php#L314

//either do
->winner(fn () => $this->rescue($ingest->trim(...))
//or
->winner(fn () => $this->rescue(fn () => $ingest->trim()))

@didac-adria You can restart with php artisan pulse:restart

@didac-adria
Copy link
Author

Oh great! I'm adding pulse:restart to our deployment hooks. Thanks!

@timacdonald
Copy link
Member

Closing this as we have a fix up.

Heads up that this mainly impacts apps writing directly to the database and not using the performance focused Redis ingest.

Sorry for the troubles, folks.

@didac-adria
Copy link
Author

Hey team!
Thank you all for your contributions!

See you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants