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

Allow Probe Server's /connect to handle a certain number of reconnects before statusing #1075

Merged
merged 1 commit into from
Sep 16, 2023

Conversation

bensheldon
Copy link
Owner

@bensheldon bensheldon commented Sep 15, 2023

Fixes #1068.

Once GoodJob has successfully connected, Probe Server's /connected endpoint will continue returning 200-success if the database connection is lost and reconnections are attempted. This is now aligned with the existing thread-error reporting behavior:

if connection_error
@connection_errors_count.increment
if @connection_errors_reported.false? && @connection_errors_count.value >= CONNECTION_ERRORS_REPORTING_THRESHOLD
GoodJob._on_thread_error(thread_error)
@connection_errors_reported.make_true
end
else
GoodJob._on_thread_error(thread_error)
end

I also increased the number of reconnect attempts, so it will now be 6 attempts, once every 5 seconds = 30 seconds of reconnecting before the Notifier#connected? will become false and the Probe Server will start returning a 503 and the GoodJob.on_thread_error callback will be triggered.

Also some slight refactoring so that the utility methods listen? and wait? are now condition variables and have a timeout.

Copy link
Contributor

@jgrau jgrau left a comment

Choose a reason for hiding this comment

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

FWIW I feel the design and code is good :)

@bensheldon bensheldon merged commit 284d671 into main Sep 16, 2023
20 checks passed
@bensheldon bensheldon deleted the probe-connected branch September 16, 2023 17:52
@bensheldon bensheldon added the bug Something isn't working label Sep 16, 2023
@bensheldon
Copy link
Owner Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

Probe failures on heavy usage of dashboard(?)
2 participants