Fix unintentional dependence upon health check to trigger runUpdates, and password length issue #76
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove await_settings_db() entirely Linebuffer some outputs so they look nicer Move redis specific config items to minimum_config*json Add start_interval to docker-compose.yml to avoid runUpdates race condition caused by health check which could lead to bad db updates, which seems to have been an issue for quite a while but is very hard to reproduce
|
CC @ftoppi |
|
@UFOSmuggler I would just disable the liveness check to be honest at this point |
|
Adding start-interval (requires docker 25.0+) and increasing interval and timeout might also help. I think I was a bit aggressive with the timings (our instance is tiny). Is there another URL that can be tested and won't trigger runUpdates? |
|
Not at the moment (but next release of MISP will have a heartbeat endpoint. |
dgujarathi
pushed a commit
to dgujarathi/misp-docker
that referenced
this pull request
Oct 6, 2024
Remove await_settings_db() entirely Linebuffer some outputs so they look nicer Move redis specific config items to minimum_config*json Add start_interval to docker-compose.yml to avoid runUpdates race condition caused by health check which could lead to bad db updates, which seems to have been an issue for quite a while but is very hard to reproduce
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove await_system_settings_table() entirely
Move redis specific config items to minimum_config*json
Adjust where runUpdates is called
Fix password length setting issue caused by jq quoting
Silence error caused cake admin init user
Linebuffer some outputs so they look nicer
Add start_interval to docker-compose.yml to avoid runUpdates race condition caused by health check which could lead to bad db updates, which seems to have been an issue for quite a while but is very hard to reproduce. issue stems from healthcheck runUpdate being clobbered by configure_misp runUpdate as runUpdate's locking mechanism is not respected by the next run. by setting the start interval to be the same as the start period, we seem to make it significantly less likely that there will be a clobbering. seems to give about 15 seconds leeway. the ideal solution would be to have a configurable option to toggle whether runupdates occurs from browser visits, but this fix has made the issue dramatically less likely.
example of this issue from an image built from 887d1b3 (prior to my changes):
as you can see, it starts at 71, when it should be 65. also ends at 91 when it should end at 125. looks like a 2 way clobbering:
this CAN lead to failed updates, which are progressed past on the next run. the db version will then show as latest, but actually be missing one or more updates. causing this to happen is very hard, but i have seen it a few times.