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

Sidecar sequencer is using panic for control flow #1868

Open
cbiffle opened this issue Sep 12, 2024 · 1 comment
Open

Sidecar sequencer is using panic for control flow #1868

cbiffle opened this issue Sep 12, 2024 · 1 comment

Comments

@cbiffle
Copy link
Collaborator

cbiffle commented Sep 12, 2024

In debugging an unrelated issue, we noticed some Sidecar sequencer panics.

It turns out, the sidecar sequencer contains a lot of deliberate panics as a very expensive way of performing a loop. We have better tools for this that don't take the system down for a dump and distract engineers:

  • A loop (which is what I would strongly recommend)
  • The Jefe::restart_me operation is how you do a deliberate restart without a dump.
@hawkw
Copy link
Member

hawkw commented Sep 12, 2024

It's probably also worth going through and thinking a bit about whether any of these panics are places where we should just reset a particular component and retry the particular operation that failed, or if they all really require starting the whole thing over from the top. The Gimlet sequencer task retries individual I2C transactions in a number of places instead of starting back at the top of the initialization process, and I wonder if it would make sense to borrow that here?

Of course, I'm not familiar enough with the sidecar startup process to know which operations can be retried individually, and which require resetting everything and starting over, but it might be worth thinking about for someone who does have a sense of that...

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

No branches or pull requests

2 participants