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

Add healthchecks to ensure we can deserialize canvases #4650

Merged
merged 1 commit into from Dec 19, 2022

Conversation

pbiggar
Copy link
Member

@pbiggar pbiggar commented Dec 18, 2022

Changelog:

Internal
- Check for serialization errors before starting up servers

This adds a serialization check at startup. That is, do not start up if we cannot successfully deserialize 5 important canvases.

I made the list of canvases a config variable to allow us flexibility. Perhaps we break one of these canvases - we don't want to be unable to restart the servers or scale up or whatever.

Not added to cronchecker as it isn't needed and also its deployment pattern (delete old server, start new one) makes this unsafe.

Not added to cronchecker as it isn't needed and also its deployment pattern makes this unsafe
Copy link
Member

@StachuDotNet StachuDotNet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great.

We have several test canvases that we have in static files (currently in backend/test_appdata).
Could/should we add those to the configs of non-prod environments? Many of them will be caught in integration tests - maybe sample-gettingstarted is one to add in all configs? (since it's used in prod as well as the ones currently listed)

@pbiggar
Copy link
Member Author

pbiggar commented Dec 19, 2022

Great.

We have several test canvases that we have in static files (currently in backend/test_appdata). Could/should we add those to the configs of non-prod environments? Many of them will be caught in integration tests - maybe sample-gettingstarted is one to add in all configs? (since it's used in prod as well as the ones currently listed)

I think this doesn't bring us much because I'm worried about old data in the binary serialized format in production. If we created this data for testing, we presumably wouldn't be saving this in the old format.

Also feels like it'll be tricky to set up.

@pbiggar pbiggar merged commit 981f377 into main Dec 19, 2022
@pbiggar pbiggar deleted the paul/check-serialized branch December 19, 2022 17:51
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