Allow to pass metadata in ActiveRecord::Result

[kirs]

At Shopify, we're about to leverage mysql's client session tracking (brianmario/mysql2#1092) and pass around some metadata from MySQL server to the client. That metadata is per connection and is fetched with something like @connection.session_track(Mysql2::Client::SESSION_TRACK_SYSTEM_VARIABLES).

I'd like ActiveRecord::Result (and eventually maybe records themselves) to retain that metadata so that we can link it to records and instrument with ActiveSupport::Notifications.

I've took a stab at trying to abstract it in a way that it's not tied to MySQL. I'm open to suggestions for what would be a better design.

cc @bibstha @sirupsen @rafaelfranca @Edouard-chin @byroot

[kirs]

Those who wanted to leverage metadata support would patch or subclass from another adapter, for instance:

class MyAdapter < ActiveRecord::ConnectionAdapters::Mysql2Adapter
  def metadata
    system_variables = @connection.session_track(Mysql2::Client::SESSION_TRACK_SYSTEM_VARIABLES)
    return unless system_variables
    JSON.parse(system_variables)
  end
end

[byroot]

Should the abstract implementation default to returning an empty hash?

[kirs]

Sounds like a good idea. Would you prefer that we freeze an empty hash and use that as a default to reduce allocations?

[byroot]

That's what I'd do, but I'm not familiar enough with the design of this part of Rails to know wether it would be expected or not.

Also if the default is frozen, it would be preferable that the actual implementation is frozen as well.

[kirs]

Updated 👍

[rafaelfranca]

Does any other database provides this kind of information?

[kirs]

Does any other database provides this kind of information?

Not to my [limited] knowledge of Postgres or sqlite. I roughly scanned Postgres protocol spec for anything that would resemble MySQL's session tracking but I wasn't able to find.

Although this metadata field is fairly abstract - it could even be used to store facts like "which shard is origin of this record" for #38531, if anyone ever needs that.

[kirs]

As for the next things that could be based on having metadata on Result, I was thinking of something like this: kirs/rails@activerecord-result-metadata...kirs:record_metadata

Let me know if this looks like a wrong direction.

[jeremy]

I'd like ActiveRecord::Result (and eventually maybe records themselves) to retain that metadata so that we can link it to records and instrument with ActiveSupport::Notifications.

My instinct is to demonstrate this system first, and separately, before introducing a generic way to provide Result metadata.

Connection session tracking may be seen as metadata about the connection and server state, or even as server notifications, since they may be registered as observation of state changes. Carrying this information along with the resultset may be natural, and it's straightforward to decorate the Result to do it; on its own, this would not merit introduction of general-purpose metadata.

Furthermore, result metadata suggests it's descriptive of the resultset itself. Things like affected row count, column types, generated warnings, and such. These metadata are already returned from the drivers and represented in AR.

Related, mysql2 exposes status flags as well:

result.server_flags[:no_good_index_used]

[kirs]

@jeremy thanks for your feedback! Here's an example how metadata on Result could be leveraged in next steps: kirs@10ff02d

I don't know if having _metadata accessor on AR objects is a good design, but at least in my case it would allow to do things like:

shop = Shop.find(1)
shop._metadata
=> { replication_delay: 0.01, ... }

# if replication_delay is within a healthy range, we're good to cache this shop record. Otherwise we shouldn't cache stale data

(btw I'm good with calling it whatever and not metadata if we think there's a better name)

Or maybe we don't bother to injecting all of metadata into the AR object, and we should instead hook into instantiate and make it set _replication_delay on the object itself from Result#metadata.

[simi]

In PostgreSQL AFAIK you can introduce connection variables through plperl or similar scripts as a global variable.

Anyway I do not see any benefit in this change. In majority of applications this will result in empty hash in each Result instance. I do not see any general usage (even I probably understand original motivation of this patch for your usage @kirs). Seems more a "hack" of codebase than new feature.

Any chance to elaborate more on this and explain how this can be used (any usecase)?

If I understand this well you need to inherit adapter (which is IMHO non-standard approach) to make any benefit of this change. Initial value is useless and you need to override it in custom adapter. What's the reason to make it general than?
Also I do not understand why you can't call @connection.session_track in instrumenting on its own and why it needs to be part of Result, but that's probably related to your app.

[simi]

@kirs can be this closed in favour of #38713?