Poc multi adapter single model single process by hmcguire-shopify · Pull Request #49 · Shopify/rails

hmcguire-shopify · 2026-01-15T15:24:17Z

Opening here so I can write notes/solicit feedback

TODOS:

cleanup send

andrewn617 · 2026-01-15T18:42:04Z

activerecord/lib/active_record/model_schema/schema.rb

+
+      # Set the table name and clear cached state that depends on it
+      def table_name=(value)
+        @table_name = nil


I think this should be value right

yes it should (or it should be removed because it isn't used)

andrewn617 · 2026-01-15T18:45:38Z

activerecord/lib/active_record/model_schema.rb

+      def model_schema # :nodoc:
+        context_key = current_schema_context_key
+        @model_schemas ||= {}
+        @model_schemas[context_key] ||= Schema.new(self, context_key)


So the model schema is loaded lazily only when the current context needs it. Makes sense. But don't we need to iterate through all the schemas when we load the model so we can make sure we generate the right attribute methods? Or are we expecting DBs to always have the same columns and thus the same methods (is that even enough to generate the needed proper methods 🤔 )

andrewn617 · 2026-01-15T19:41:13Z

activerecord/lib/active_record/database_configurations/hash_config.rb

+      # Multiple pools (read replicas, shards) can share the same key, meaning
+      # they share cached schema information. Defaults to "default".
+      def schema_context_key # :nodoc:
+        configuration_hash.fetch(:schema_context, "default").to_s


Is it worth asking do we really need this config? The vision is we are switching between different DB adapters right? So can we just switch based on that? Is there really a use case to have arbitrary schema contexts?

activerecord/lib/active_record/model_schema/schema.rb

hmcguire-shopify · 2026-02-04T19:02:47Z

activerecord/lib/active_record/model_schema/schema.rb

+      def symbol_column_to_string(name_symbol)
+        @symbol_column_to_string_name_hash ||= column_names.index_by(&:to_sym)
+        @symbol_column_to_string_name_hash[name_symbol]
+      end


This one's also interesting: maybe it should just be a global hash so that models with the same column names could share?

Nope, its a proxy for column_names/column_hash: https://github.com/rails/rails/pull/34197

hmcguire-shopify · 2026-02-04T21:48:07Z

activerecord/lib/active_record/model_schema/schema.rb

+      end
+
+      # Returns the column object for a named attribute
+      def column_for_attribute(name)


Maybe this can be global too... the columns_hash will be per context anyways and nothing here is cached?

hmcguire-shopify · 2026-02-24T19:40:29Z

activerecord/lib/active_record/model_schema/schema.rb

+      def _default_attributes
+        @default_attributes ||= begin
+          attributes_hash = columns_hash.transform_values do |column|
+            ActiveModel::Attribute.from_database(column.name, column.default, model_class.send(:type_for_column, column))


type_for_column -> TimeZoneConversion

time_zone_aware_attributes -> probably per context

hmcguire-shopify · 2026-02-24T19:42:31Z

activerecord/lib/active_record/model_schema/schema.rb

+        @symbol_column_to_string_name_hash = nil
+        @primary_key = nil
+        @composite_primary_key = nil
+      end


investigate viability table_name/primary_key

specifically primary_key may be difficult for associations

class Post model_schema["default"].table_name = "core.posts" end

hmcguire-shopify · 2026-02-24T19:47:13Z

activerecord/lib/active_record/model_schema/schema.rb

+    # Schema context keys group connections sharing a schema shape - the same adapter,
+    # column types, SQL dialect, etc. Multiple pools (read replicas, shards) can share
+    # the same key, meaning they share cached schema information.
+    class Schema


eval whether model_schema should be documented, and currently public-on-models should be public-on-model-schema

hmcguire-shopify · 2026-02-24T20:04:45Z

activerecord/lib/active_record/model_schema/schema.rb

+        @model_class = model_class
+        @context_key = context_key
+        @schema_loaded = false
+        @load_schema_monitor = Monitor.new


potentially move back up, only use here would be concurrently loading schema

hmcguire-shopify · 2026-02-24T20:11:30Z

activerecord/lib/active_record/model_schema.rb

+      rescue ConnectionNotDefined
+        "default"


Investigate the failing test:

Failure: BasicsTest#test_.columns_hash_raises_an_error_if_the_record_has_an_empty_table_name [test/cases/base_test.rb:1820]: [ActiveRecord::TableNotSpecified] exception expected, not Class: <ActiveRecord::ConnectionNotDefined> Message: <"No database connection defined for FirstAbstractClass."> bin/test activerecord/test/cases/base_test.rb:1818

The issue is Active Model defines `attribute_types` and caches it. `ModelSchema` brought the value inside Active Record because it depends on `default_attributes` (which we want to be per-context). However, the implementation was missing the addition of an override for Active Model's `attribute_types` implementation.

since it depends on columns_hash, which is ModelSchema aware

The last few commits were trying to fix issues with methods defined in Active Model that now need to be multi-context aware. However, moving the method from Attributes to ModelSchema actually _undefined_ it because ModelSchema was included before Attributes, so the one in AttributeRegistration was being defined last. This commit fixes the issue by moving ModelSchema after Attributes so that our context aware methods win.

Other methods (type_for_column) depend on the current order, so instead I'll move the model_schema aware methods to better places

table_name= was previously clearing some instance variables, but those variables are now inside ModelSchema so they need to be cleared there. attribute_names ha a similar issue. Its clearing was moved inside ModelSchema, but should not be, since one of our constraints is that attribute_names should be consistent across contexts.

arel_table and predicate_builder are now inside the Schema, so they don't need to be cleared here. Instead, they get cleared by reseting the model_schemas hash, which is done in a child inherited definition.

At the moment it fails because find_by_statement_cache is being shared between contexts, which I know we need to fix.

Previously it was global across contexts which would cause issues if contexts don't generate the same queries.

For some reason I can't get our connected? to return true. It's unclear whether its a (monkey patching) bug in our app, but it shouldn't be necessary here anyways since we're only looking at statically defined configuration. This rescue may not be necessary, there's one test in Rails' suite that fails without it but it seems suspect.

We want to ensure attributes aren't defined concurrently, which per-context monitors wouldn't prevent

github-actions bot added the activerecord label Jan 15, 2026

andrewn617 reviewed Jan 15, 2026

View reviewed changes

hmcguire-shopify force-pushed the poc-multi-adapter-single-model-single-process branch 2 times, most recently from 3ceeeb8 to 690e14a Compare January 20, 2026 20:36

github-actions bot added the actionview label Jan 20, 2026

hmcguire-shopify force-pushed the poc-multi-adapter-single-model-single-process branch from 4950c19 to 02a744d Compare January 29, 2026 18:06

github-actions bot added activejob activesupport railties labels Jan 29, 2026

hmcguire-shopify commented Feb 4, 2026

View reviewed changes

activerecord/lib/active_record/model_schema/schema.rb Outdated Show resolved Hide resolved

hmcguire-shopify commented Feb 4, 2026

View reviewed changes

hmcguire-shopify force-pushed the poc-multi-adapter-single-model-single-process branch from 02a744d to 0936bd9 Compare February 17, 2026 20:25

hmcguire-shopify commented Feb 24, 2026

View reviewed changes

skipkayhil added 10 commits March 4, 2026 11:42

poc - yay claude

a107b7b

_default_attributes needs to be ModelSchema aware

a55f5b3

since it depends on columns_hash, which is ModelSchema aware

Don't make changes from original method impls

6b3f43a

Revert the inclusion order change

9bbb9f7

Other methods (type_for_column) depend on the current order, so instead I'll move the model_schema aware methods to better places

Cleanup unused variable assignment

d97249f

arel_table and predicate_builder are now inside the Schema, so they don't need to be cleared here. Instead, they get cleared by reseting the model_schemas hash, which is done in a child inherited definition.

Add mostly-passing integration test

bc6bcd0

At the moment it fails because find_by_statement_cache is being shared between contexts, which I know we need to fix.

Fix find_by cache to work with multiple contexts

1ba86ee

Previously it was global across contexts which would cause issues if contexts don't generate the same queries.

skipkayhil added 2 commits March 4, 2026 11:42

Fix find_by cache to work with multiple contexts

1ba86ee

Previously it was global across contexts which would cause issues if contexts don't generate the same queries.

hmcguire-shopify force-pushed the poc-multi-adapter-single-model-single-process branch from 0936bd9 to a870d4e Compare March 4, 2026 16:43

Move schema loading monitor back up to model

52f80b3

We want to ensure attributes aren't defined concurrently, which per-context monitors wouldn't prevent

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Poc multi adapter single model single process#49

Poc multi adapter single model single process#49
hmcguire-shopify wants to merge 12 commits intomainfrom
poc-multi-adapter-single-model-single-process

hmcguire-shopify commented Jan 15, 2026 •

edited

Loading

Uh oh!

andrewn617 Jan 15, 2026

Uh oh!

hmcguire-shopify Jan 15, 2026

Uh oh!

andrewn617 Jan 15, 2026

Uh oh!

andrewn617 Jan 15, 2026

Uh oh!

Uh oh!

hmcguire-shopify Feb 4, 2026

Uh oh!

hmcguire-shopify Feb 5, 2026

Uh oh!

hmcguire-shopify Feb 4, 2026

Uh oh!

hmcguire-shopify Feb 24, 2026

Uh oh!

hmcguire-shopify Feb 24, 2026 •

edited

Loading

Uh oh!

hmcguire-shopify Feb 24, 2026 •

edited

Loading

Uh oh!

hmcguire-shopify Feb 24, 2026

Uh oh!

hmcguire-shopify Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

hmcguire-shopify commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hmcguire-shopify Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hmcguire-shopify Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hmcguire-shopify commented Jan 15, 2026 •

edited

Loading

hmcguire-shopify Feb 24, 2026 •

edited

Loading

hmcguire-shopify Feb 24, 2026 •

edited

Loading