trilogy-libraries / activerecord-trilogy-adapter

Active Record adapter for the Trilogy database client for Rails v6.0 - v7.0.
https://github.com/trilogy-libraries/trilogy
MIT License
171 stars 17 forks source link

Using Trilogy's multi statement support in `#insert_fixture_set` #37

Closed paarthmadan closed 1 year ago

paarthmadan commented 1 year ago

Context

Rails' abstract adapter defines a method called #insert_fixture_set which is used for inserting multiple fixtures into the database. This method makes use of two private methods, #execute_batch and #with_multi_statements. At the abstract level, these two methods don't end up making use of any multi statement constructs.

Once we enter specific adapter implementations, like Mysql2Adapter or PostgresqlAdapter, we see those definitions making use of muli-statement, presumably to leverage the performance benefits you get by inserting multiple fixtures at once. Taking Mysql2Adapter for instance, here and here.

We added multi statement support to Trilogy, so we were hoping to follow suit of the other adapters and use it during this codepath. @adrianna-chang-shopify and I started to look into this, but noticed Trilogy deviates from Mysql2's API with respect to enabling and disabling multi statement after a connection has already been established.

Mysql2Adapter uses Mysql2#set_server_option to enable multi statements on an existing connection before, and disable after, fixtures have been inserted (if it wasn't configured by the user at initialize-time)

Approaches

There's a few approaches to get the performance benefit of multi statement in this flow.

  1. Add set_server_option to the Trilogy client

This would involve implementing a new type of protocol command, COM_SET_OPTION, whose purpose in the MySQL protocol is exclusively related to altering multi statement on an existing connection. We'd add a new builder to the protocol, a corresponding client method, and then a Ruby binding method to make use of these underlying changes. Then, the changes to the adapter would more or less look like Mysql2Adapter. ie.

def with_multi_statements
  if multi_statements_enabled?
    return yield
  end

  with_raw_connection do |conn|
    conn.set_server_option(Trilogy::OPTION_MULTI_STATEMENTS_ON)

    yield
  ensure
    conn.set_server_option(Trilogy::OPTION_MULTI_STATEMENTS_OFF)
  end
end
  1. Create a new connection with multi_statement: true when using the insert_fixture_set method

This approach avoids changes to the Trilogy client. Instead, _it assumes we can only configure multi_statement: true at initialize time._ Then, in the event it's not enabled, we spin up a new temporary connection for the fixture operation, use it, and then throw it away before returning to the existing connection config.

Tradeoffs

There's tradeoffs for both approaches:

The first approach expands Trilogy's protocol (which seems to have been kept intentionally small). One might argue it bloats the otherwise sleek library for the simple enabling and disabling of one flag (which should be set at initialize-time in majority of cases anyways). The advantage is that the API get closer to feature-parity with Mysql2, and that the changes at the adapter level take semblance to existing solutions.

The second approach requires no changes to Trilogy, an advantage. It does introduce the overhead of using a new, separate connection with different config, temporarily.

I'm currently moving forward with Solution (1), but figured it'd be good to write an issue and solicit early feedback in case there are stronger opinions.

The change set for (1) isn't terrible, the only push-back I can anticipate is whether the frequency of its usage (which I predict will be very small) is worth the introduction of new code.