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

Never connect to the database in define_attribute_methods initializer #48793

Merged
merged 1 commit into from
Aug 1, 2023

Conversation

casperisfine
Copy link
Contributor

Followup: #48743

After careful consideration, unless users have a schema cache dump loaded and check_schema_cache_dump_version = false, we have no choice but to arbitrate between resiliency and performance.

If we define attribute methods during boot, we allow them to be shared between forked workers, and prevent the first requests to be slower.

However, by doing so we'd trigger a connection to the datase, which if it's unresponsive could lead to workers not being able to restart triggering a cascading failure.

Previously Rails used to be in some sort of middle-ground where it would define attribute methods during boot if for some reason it was already connected to the database at this point.

But this is now deemed undesirable, as an application initializer inadvertantly establishing the database connection woudl lead to a readically different behavior.

FYI: @zarqman @matthewd

Followup: rails#48743

After careful consideration, unless users have a schema cache dump loaded
and `check_schema_cache_dump_version = false`, we have no choice but
to arbitrate between resiliency and performance.

If we define attribute methods during boot, we allow them to be shared
between forked workers, and prevent the first requests to be slower.

However, by doing so we'd trigger a connection to the datase, which
if it's unresponsive could lead to workers not being able to restart
triggering a cascading failure.

Previously Rails used to be in some sort of middle-ground where
it would define attribute methods during boot if for some reason
it was already connected to the database at this point.

But this is now deemed undesirable, as an application initializer
inadvertantly establishing the database connection woudl lead to
a readically different behavior.
@casperisfine casperisfine force-pushed the define-attributes-initializer branch from 777ef86 to de765e3 Compare July 24, 2023 14:37
@rails-bot rails-bot bot added the railties label Jul 24, 2023
@byroot byroot merged commit 35a614c into rails:main Aug 1, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants