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 liveness check for workers #140

Merged
merged 1 commit into from
Mar 26, 2024
Merged

Conversation

shubhwip
Copy link
Contributor

@shubhwip shubhwip commented Jan 24, 2023

closes #72

Dependent on PR getredash/redash#5886 and it is merged already.

Related Issues :

getredash/redash#5885
#124
#72
getredash/redash#5801

More context

liveness check is based on
rq info command returns monitoring status of redash workers and other status checks
Sample Output

redash@redash-adhocworker-778fc5dbb6-jpscq:/app$ rq info --url $REDASH_REDIS_URL -R
queries      | 0
default      | 2
periodic     |██████████████████ 748
emails       | 0
schemas      | 0
5 queues, 750 jobs total

queries:  1a6d953d53984424ab775d3647427e7b (idle), 29dbf1ec1f774bacbd91ca36ccef6e43 (idle)
default:  –
periodic: –
emails:   –
schemas:  –
2 workers, 5 queues

@shubhwip
Copy link
Contributor Author

@grugnog can you please check this PR ? This issue has been reported by many.
We can remove default values(in case PR on redash repo doesn't get merged) and get it merged and people who are using this helm charts can supply their own values with liveness checks.

@grugnog
Copy link
Collaborator

grugnog commented Mar 24, 2023

@shubhwip we will need to add the new values to the README but otherwise this looks good to me. We don't want to break installs though, so I don't think we should merge until upstream is merged however, and we can't release until upstream is released.

@shubhwip shubhwip force-pushed the bugs/72 branch 3 times, most recently from bbc73d9 to 7817bfd Compare March 29, 2023 12:08
Update README file

closes getredash#72
@shubhwip
Copy link
Contributor Author

@grugnog made the changes as requested and now upstream is merged.
getredash/redash#5886

@aneagoe
Copy link

aneagoe commented Apr 17, 2023

Would love to see this merged; currently facing issues with workers silently dying and we can only manually bounce them.

@shubhwip
Copy link
Contributor Author

@grugnog when do you plan to merge the changes as upstream changes are merged months ago :) ?

@ffzzhong
Copy link

@grugnog please kindly take a look

@grugnog
Copy link
Collaborator

grugnog commented Jul 29, 2023

@shubhwip @ffzzhong it's merged upstream, but still no stable release to point at - I can try and get a preview release up that folks can try, but I would be wary in terms of upgrade path.

@justinclift
Copy link
Member

Now that Redash is a Community led project, we've been madly working through the outstanding PRs, Dependabot issues, (etc) getting things cleaned up and ready for a next release.

It's going to be a few weeks away though. We might be able to get something like an "Redash 11.0.0 alpha 1" (or beta) out the door in a fortnight or so, but we'll see. 😄

@justinclift
Copy link
Member

@grugnog If you want to create some kind of preview release for people to do with in the meantime, then please feel encouraged to do so. It could be considered a good initial shake out to see if we've busted anything non-obvious in the mad rush of merging PRs and similar. 😄

@grugnog
Copy link
Collaborator

grugnog commented Jul 31, 2023

That sounds good - I am working on getting the other PRs tested and merged, then will look at this

@justinclift
Copy link
Member

@AndrewChubatiuk Does this PR seem sensible to you?

@AndrewChubatiuk AndrewChubatiuk merged commit ea746ef into getredash:master Mar 26, 2024
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.

readiness + liveness for worker
7 participants