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

Don't start event source until Rails is ready. #117

Merged
merged 5 commits into from
Sep 23, 2024
Merged

Conversation

TreyE
Copy link
Contributor

@TreyE TreyE commented Jul 31, 2024

During some workloads, DevOps discovered that Event Source would sometimes undertake work before the Rails application had fully booted and initialized all of its items (such as database connections). This was because Event Source was creating connections and workers when the Event Source initializer file was being loaded, not after Rails had signalled it had completed the boot process.

Event source has been updated in the following way:

  1. Configuration is still loaded in the initializer file, so the changes are backward compatible.
  2. Connections and work are not undertaken until after the Rails framework triggers the finalizer hook indicating all frameworks and plugins have been initialized and started.

Ticket: https://www.pivotaltracker.com/story/show/188045317

@TreyE TreyE changed the title Don't start event source until rails is ready. Don't start event source until Rails is ready. Jul 31, 2024
@TreyE TreyE force-pushed the wait_for_rails_start branch 2 times, most recently from 7271182 to 52e5a67 Compare September 11, 2024 00:09
@saikumar9 saikumar9 added the do not merge PRs that should not be merged. label Sep 18, 2024
@saikumar9 saikumar9 requested review from saikumar9 and removed request for ymhari and vkghub September 19, 2024 20:14
@saikumar9 saikumar9 removed the do not merge PRs that should not be merged. label Sep 21, 2024
@saikumar9 saikumar9 enabled auto-merge (squash) September 21, 2024 01:06
Copy link
Member

@saikumar9 saikumar9 left a comment

Choose a reason for hiding this comment

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

lgtm! Regression testing is needed.

Copy link
Contributor

@raghuramg raghuramg left a comment

Choose a reason for hiding this comment

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

looks good!

@saikumar9 saikumar9 merged commit 6d3acdb into trunk Sep 23, 2024
4 checks passed
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.

3 participants