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

Make boot great again #83

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

voxdei
Copy link

@voxdei voxdei commented Sep 15, 2023

Description

Using some black magic with sleep(0.1) to give some breathing room for a process check for windows, surprisingly allowing our service to boot in no time instead of timing out for not clear reason

@voxdei voxdei requested review from a team as code owners September 15, 2023 14:36
@voxdei voxdei closed this Sep 15, 2023
@daipom
Copy link

daipom commented Feb 26, 2024

Why was this PR closed?

Service startup is noticeably slower with Ruby 3. (Please see #84)
Inserting sleep could be one way to solve #84.

@voxdei
Copy link
Author

voxdei commented Feb 26, 2024

I don't remember why I closed it honestly, as we are still using that "patch" on our side to prevent the service from timing out

@voxdei voxdei reopened this Feb 26, 2024
Copy link

sonarcloud bot commented Feb 26, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@daipom daipom mentioned this pull request Feb 26, 2024
3 tasks
@daipom
Copy link

daipom commented Feb 26, 2024

Thanks for your quick response!
I see.

I thought it could be caused by using none-ruby threads, and I have made a PR #85.
I noticed this PR when I was making #85.

I have confirmed that inserting sleep also solves the slow start problem.
So, there are two solutions:

@jaymzjulian
Copy link

This seems to track with #84 and the race around service management, but I will say that sleep as a race condition breaker makes me generally nervous, because without actually addressing the race, there's likely to be a set of systems where the race triggers anyhow, particularly under load.

That having been said: I suspect at least part of this is needing the equivlent of a thread.yield (think pthreads yield, not ruby yield) rather than busy waiting to allow for whatever set of locks are being held in that race, because I was totally able to reproduce the behaviour, and this did seem to fix it every single time. So there is that, my inclination, is that in the absense of a better solution, this makes sense to do.

The PR text says just the sleep() stuff, but I do see a bunch of other changes in there - those changes do seem to make sense to me too, to be honest, but it would be good to enumerate what they're doing too and why they're doing it

@voxdei
Copy link
Author

voxdei commented Feb 29, 2024

The PR text says just the sleep() stuff, but I do see a bunch of other changes in there - those changes do seem to make sense to me too, to be honest, but it would be good to enumerate what they're doing too and why they're doing it

Oh that's because I started my sleep change on top of my collegue's changes which are actually coming from #82 and they just got brought foward in my PR

I'll rebase just my commit, as there is already another MR just focusing on those other changes already

@swistak35
Copy link

I (the colleague mentioned above 😄 ) can also just double-confirm that these issues which this MR solves indeed started happening pretty much with our upgrade to ruby 3.

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.

5 participants