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

Adding limit on how many times a crash command can run #2008

Merged
merged 2 commits into from
Jan 13, 2025

Conversation

danieldoglas
Copy link
Contributor

@danieldoglas danieldoglas commented Dec 9, 2024

Details

We currently store specific identifying values when a crash happens. That means that if a specific user call a command in Bedrock (e.g. userID 1) and that command crashes the node, we will store that userID and won't let the cluster execute that command for that user again.

The problem is that if the issue is in the command, and not on specific data related to that user, UserID 2 or UserID 3 could still call the same command and progressively crash all nodes in the cluster.

This change will block commands from being executed if the same command already crashed the cluster more than one time, independent of the specific crash identifying values.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/451831

Tests

need to work on tests.


Internal Testing Reminder: when changing bedrock, please compile auth against your new changes

@danieldoglas danieldoglas self-assigned this Dec 9, 2024
@danieldoglas danieldoglas changed the title [WIP] Adding limit on how many times a command can run Adding limit on how many times a command can run Jan 12, 2025
@danieldoglas danieldoglas marked this pull request as ready for review January 12, 2025 16:49
@danieldoglas danieldoglas changed the title Adding limit on how many times a command can run Adding limit on how many times a crash command can run Jan 13, 2025
@danieldoglas
Copy link
Contributor Author

@tylerkaraszewski all yours

@tylerkaraszewski tylerkaraszewski merged commit ba895ba into main Jan 13, 2025
1 check passed
@tylerkaraszewski tylerkaraszewski deleted the dsilva_preventSeveralCrashesFromHappening branch January 13, 2025 23:43
@pecanoro
Copy link
Contributor

Manually commenting since the deploy script is broken, this was deployed to webrock

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.

5 participants