-
Notifications
You must be signed in to change notification settings - Fork 3
Introduce ReplicationCoordinator to support multiple AZs #35
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
base: main
Are you sure you want to change the base?
Conversation
Note that this pins Rails to a version with ActiveSupport::ReplicationCoordinator see basecamp/rails#35
Note that this pins Rails to a version with ActiveSupport::ReplicationCoordinator see basecamp/rails#35
|
Chatted a bit with my teammates including @kevinmcconnell, @rosa, @djmb, and @jeremy and there were some thoughts around how to make this idea more general and broadly applicable: Additional use cases
These thoughts point in the direction of having a "replication coordinator" associated with a specific component (or set of components) rather than a single global instance.
InterfaceThey also point in the direction of using something more complex than a boolean It would probably be worth doing an inventory of gems like redis-sentinel and rails_failover (and others) to get a feel for what some of the likely integrations look like and what sort of interface would make it easy to do that integration. Tighter integration with Active RecordFinally, there's a potential subclass of Replication Coordinator for relational databases that interacts with |
Currently, `.connects_to(shards: ...)` coerces all of the shard keys into symbols. Because an integer cannot be coerced into a symbol, attempting to identify a shard with an integer will raise an exception: ```ruby 1.to_sym (irb):1:in '<main>': undefined method 'to_sym' for an instance of Integer (NoMethodError) ``` I think it would be useful to support integers as shard keys along with symbols. This would simplify shard switching when shards are identified by integer columns in a database. As an example, if there is an `accounts` table with a `shard` integer column which identifies which shard that account's data lives on, this currently would not work: ```ruby account = Account.first ActiveRecord::Base.connected_to(shard: account.shard) do # Raises NoMethodError end ``` A workaround would be to first coerce or serialize the integer into a string but that's unideal: ```ruby account = Account.first ActiveRecord::Base.connected_to(shard: account.shard.to_s.to_sym) do # ... end ``` This makes a `.connects_to` change, coercing the shard key into a symbol _unless_ the key is an integer. From there, passing a symbol or integer to `connected_to(shard: ...)` should both work. In the test, we call: ```ruby ActiveRecord::Base.default_shard = 0 ``` Normally, it would be `:default`. In that case we'd have to mix symbols and integers in the `.connects_to:(shards:)` hash which is pretty ugly.
…config-keys Support integer shard keys
- <img> tag was missing alt attribute/value that described image - <html> tag was missing lang attribute/value
The problem was fixed in sdoc 2.6.2. See https://github.com/rails/sdoc/releases/tag/v2.6.2
…come-template Address small a11y errors in the welcome template
Restore support to analyze image blobs with `image_magick`
…pace Fix label `for` attribute missing form namespace
DRY up test helpers in DatabaseTasks tests
…has-and-belongs-to-many-assosiations-test Fix nondeterministic behavior in DeprecatedHasAndBelongsToManyAssociationsTest
Signed-off-by: Takuya Noguchi <[email protected]>
Make CodeStatistics pattern overridable
…isolation Make transaction isolation work smoothly with transactional tests
Temp point to sdoc stable to fix doc-preview
While private, it appears to be used a lot by other gems
…nils Fix negative scopes for enums to include records with `nil` values
The Problem
---
Prior to this change, testing the content of multi-part Action
Mailer-generated `Mail` instances involved parsing content derived from
each part into the appropriate type. For example, tests might read
`mail.body.raw_source` from `text/plain` bodies into `String` instances
and parse `text/html` bodies into HTML documents with Nokogiri.
The Proposal
---
This commit defines `assert_part` and `assert_no_part` as assertion
methods on `ActionMailer::TestCase`.
The `assert_part` method iterates over each available part, and asserts
that the multi-part email includes a part that corresponds to the
`content_type` argument.
```ruby
test "assert against the HTML and text parts of the last delivered MyMailer.welcome mailer" do
MyMailer.welcome("Hello, world").deliver_now
assert_part :text do |text|
assert_includes text, "Hello, world"
end
assert_part :html do |html|
assert_dom html.root, "h1", "Hello, world"
end
assert_no_part :xml
end
test "assert against the HTML and text parts of a MyMailer.welcome mail instance" do
mail = MyMailer.welcome("Hello, world")
assert_part :text, mail do |text|
assert_includes text, "Hello, world"
end
assert_part :html, mail do |html|
assert_dom html.root, "h1", "Hello, world"
end
assert_no_part :xml, mail
end
```
Additional details
---
While HTML assertion support is already baked-into the gem's railtie,
calls to `assert_part(:html)` make the fully-formed HTML
document (a `Nokogiri::HTML4::Document` or `Nokogiri::HTML5::Document`
instance) available to the block. The method also accepts a `Mail` instance directly.
By comparison, the [assert_dom_email][] method (and its
`assert_select_email` alias) provided by `Rails::Dom::Testing` yields an
HTML fragment, and cannot assert against any `Mail` instance except for
the last delivery.
[Rails::Dom::Testing]: https://github.com/rails/rails-dom-testing
[assert_dom_email]: https://github.com/rails/rails-dom-testing/blob/v2.3.0/lib/rails/dom/testing/assertions/selector_assertions.rb#L287-L317
Extend `ActionMailer::TestCase` with multi-part assertions
* Add wishlists guide * Update tutorials link * Update guides/source/wishlists.md * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @MatheusRich Co-authored-by: Matheus Richard <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @MatheusRich Co-authored-by: Matheus Richard <[email protected]> * Apply suggestion from @MatheusRich Co-authored-by: Matheus Richard <[email protected]> * Apply suggestion from @MatheusRich Co-authored-by: Matheus Richard <[email protected]> * Apply suggestion from @MatheusRich Co-authored-by: Matheus Richard <[email protected]> * Apply suggestion from @MatheusRich Co-authored-by: Matheus Richard <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @rafaelfranca Co-authored-by: Rafael Mendonça França <[email protected]> * Apply suggestion from @excid3 * Apply suggestion from @excid3 * Update tests * More consistent code tags when referencing models * Apply suggestions from code review Co-authored-by: Amanda Perino <[email protected]> * Apply suggestions from code review Co-authored-by: Amanda Perino <[email protected]> * Apply suggestions from code review Co-authored-by: Amanda Perino <[email protected]> * Two more paragraph tweaks * Clarify validation reference --------- Co-authored-by: Rafael Mendonça França <[email protected]> Co-authored-by: Matheus Richard <[email protected]> Co-authored-by: Amanda Perino <[email protected]> Co-authored-by: Rafael Mendonça França <[email protected]>
…ort.deprecation` is set to `:notify`.
* adding dark mode to http error pages * added a brighter hue of red to the error name in dark mode for better readability * decrease error id brigtness for better readability * better contrast (aa compliant)
Adds an optional :prefix paramer to has_secure_token that prepends a string to generated tokens, making token types immediately identifiable in logs, debugging sessions, and error messages. Before: user.auth_token # => "pX27zsMN2ViQKta1bGfLmVJE" user.reset_token # => "tU9bLuZseefXQ4yQxQo8wjtB" After: has_secure_token :auth_token, prefix: "auth_" has_secure_token :reset_token, prefix: "reset_" user.auth_token # => "auth_pX27zsMN2ViQKta1bGfLmVJE" user.reset_token # => "reset_tU9bLuZseefXQ4yQxQo8wjtB" This enables better developer clarity without additional database columns or complex token parsing logic. Particularly valuable when analyzing application logs, supporting operations teams, and debugging authentication flows with multiple token types. Anecdotally, I have dozens of classes that I override the _feels_ far nicer to copy/paste a token with context to others than an string without any purpose encoded. Notably, the current implementation makes this a breaking change for anyone who has overriden the #generates_unique_secure_token like myself. Add test case for test_token_with_prefix Add missing quotation Update documentation notes Add prefix option to has_secure_token for improved token identification Adds an optional :prefix paramer to has_secure_token that prepends a string to generated tokens, making token types immediately identifiable in logs, debugging sessions, and error messages. Before: user.auth_token # => "pX27zsMN2ViQKta1bGfLmVJE" user.reset_token # => "tU9bLuZseefXQ4yQxQo8wjtB" After: has_secure_token :auth_token, prefix: "auth_" has_secure_token :reset_token, prefix: "reset_" user.auth_token # => "auth_pX27zsMN2ViQKta1bGfLmVJE" user.reset_token # => "reset_tU9bLuZseefXQ4yQxQo8wjtB" This enables better developer clarity without additional database columns or complex token parsing logic. Particularly valuable when analyzing application logs, supporting operations teams, and debugging authentication flows with multiple token types. Anecdotally, I have dozens of classes that I override the _feels_ far nicer to copy/paste a token with context to others than an string without any purpose encoded. Notably, the current implementation makes this a breaking change for anyone who has overriden the #generates_unique_secure_token like myself. Add test case for test_token_with_prefix Add missing quotation Update documentation notes Add TrueClass option for secure_token prefix
…ervice (rails#55856) The s3_service exposes it's respective client and bucket attributes publicly. I think this makes sense in the context of active storage, as it allows users to rely on the same adapter & configs that the framework is using for Activestorage without having to duplicate your own client and ensure it matches the original, which can be modified at runtime, in order to perform operations outside of the scope of ActiveStorage. This commit serves to make the same true of the gcs_service, which currently defined it's respective client & bucket as private method definitions. I did a bit of git snooping and didn't see any mention of this being an intentional decision when first released. Lint
Add structured event for Rails deprecations, when `config.active_support.deprecation` is set to `:notify`.
Right now if one presses CONTROL+C during the parallel test run, one get a very long backtrace that is very annoying. The same problem was solved 11 years ago for the non parallel tests here: * minitest/minitest@b6ec36d * minitest/minitest#503 The reason why the 11 years old fix does not work for the parallel test is, that the rescue happens before the shutdown method gets executed. The shutdown method is where things gets executed. In my opinion, this is wrong, but my guess is, that it was done so, because doing it otherwise would probably require changes in the Minitest gem itself. NOTE: this is my first PR on the rails gem, so I apologize if I made something wrong. Also, there is a big probability that there is a better solution or that I miss some edge case that I am missing because of lack of knowledge.
Add prefix option to has_secure_token for improved token identification
…ace-on-parallel-test-abortion hidde backtrace when parallel tests get interrupted
…tcher-config-in-routes-reloader [Fix rails#55708] Respect the file_watcher config in the routes reloader
- Pass command arguments as an array of string to avoid any escaping issues. - Report errors if the loading failed. Co-Authored-By: Jean Boussier <[email protected]>
…tion-fix avoid direct interpolation for args in structure_load
…ery-assertions-outputs Make `ActiveRecord::Assertions::QueryAssertions` method outputs consistent
Closes [rails#50345][] First, handle the exception mentioned in [rails#50345][]: ``` BugTest#test_params_with_htm_content_type: NoMethodError: undefined method `to_html' for {:name=>"Muad'Dib"}:Hash .../actionpack/lib/action_dispatch/testing/request_encoder.rb:39:in `encode_params' .../actionpack/lib/action_dispatch/testing/integration.rb:251:in `process' ``` Calls with `as: :html` result in a `NoMethodError` because `Hash#to_html` does not exist. Passing `as: :html` implies that the request parameters will come from a `POST` body encoded as `text/html`. That isn't entirely true -- browsers will encode `POST` parameters as with the `Content-Type:` header set to either [application/x-www-form-urlencoded][] or [multipart/form-data][]. This commit skips setting the `CONTENT_TYPE` Rack header when processed with `as: :html`. To account for that, extend the `RequestEncoder` constructor to accept a `content_type `argument to use when provided. When omitted, continue to fall back to the provided MIME type. Extend the default `:html` encoder configuration to default to submitting with `Content-Type: x-www-form-urlencoded`. [rails#50345]: rails#50345 [application/x-www-form-urlencoded]: https://developer.mozilla.org/en-US/docs/Learn/Forms/Sending_and_retrieving_form_data#the_post_method [multipart/form-data]: https://developer.mozilla.org/en-US/docs/Learn/Forms/Sending_and_retrieving_form_data#the_enctype_attribute
…butes Make `ActiveModel::Serializers::JSON#from_json` compatible with `#assign_attributes`
Treat `as: :html` tests request params as `:url_encoded_form`
This broke because the keyword arg change started always passing `shallow:` through to this `scope` block. However, this caused an issue because passing the `shallow` option caused it to _always_ override the scope's current `shallow` value, instead of only doing that when specified. This commit fixes the behavior by only passing `shallow` to the `scope` when it is specified, instead of unconditionally. Co-authored-by: Marek Kasztelnik <[email protected]>
Fix passing both module: and shallow: to resources
…column for Subscriber and explain the resulting migration file <!-- Thanks for contributing to Rails! Please do not make *Draft* pull requests, as they still send notifications to everyone watching the Rails repo. Create a pull request when it is ready for review and feedback from the Rails team :). If your pull request affects documentation or any non-code changes, guidelines for those changes are [available here](https://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html#contributing-to-the-rails-documentation) About this template The following template aims to help contributors write a good description for their pull requests. We'd like you to provide a description of the changes in your pull request (i.e. bugs fixed or features added), the motivation behind the changes, and complete the checklist below before opening a pull request. Feel free to discard it if you need to (e.g. when you just fix a typo). --> When I was doing the tutorial, I noticed that the `email` column was not specified with a type yet it defaulted to a string. This confused me, so I wanted to add more detail about it. <!-- Describe why this Pull Request needs to be merged. What bug have you fixed? What feature have you added? Why is it important? If you are fixing a specific issue, include "Fixes #ISSUE" (replace with the issue number, remove the quotes) and the issue will be linked to this PR. --> I added a note for the default with showing the generated migration file. I tried looking for documentation on this, but didn't find anything. When I tried it in a few different projects, this did work. I did find the relevant code in `railties/lib/rails/generators/generated_attribute.rb` where ```ruby def initialize(name, type = nil, index_type = false, attr_options = {}) @name = name @type = type || :string ``` In the tutorial I don't want to link to the actual codebase, but I think there's a few more pages that I can add a note too. * https://guides.rubyonrails.org/active_record_migrations.html * https://guides.rubyonrails.org/command_line.html * https://guides.rubyonrails.org/active_record_basics.html Not sure if there's any other API specifc pages I can add a note to. If I do add these, should this be in this PR or a separate one? <!-- Provide additional information such as benchmarks, references to other repositories, or alternative solutions. --> Before submitting the PR make sure the following are checked: * [x] This Pull Request is related to one change. Unrelated changes should be opened in separate PRs. * [x] Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: `[Fix #issue-number]` * [x] Tests are added or updated if you fix a bug or add a feature. - N/A * [x] CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included. - N/A
[Getting Started Tutorial] Add note on default string type for `email` column for Subscriber and explain the resulting migration file [ci skip]
It's common for applications that are deployed across multiple availability zones (using a replicated database) to create an ad-hoc method for processes to discover which zone is "active", meaning the zone primarily responsible for writing to the database. For example, a team may choose to use a MySQL system variable to indicate the data center where the primary database sits. In which case, they need to write code to make sure all Rails processes in all zones query this efficiently (it may be slow to access in non-primary zones) and are notified if the primary zone changes, as in the case of a data center failover. `ReplicationCoordinator::Base` is introduced to allow developers to write code that determines whether a process is in an active zone, and then: - monitor and cache that value, with configurable polling interval - fire callbacks when the state changes from active -> passive or vice versa Additionally, a test helper class is provided to simplify testing failover behavior. Finally, Rails is configured by default to use a simple concrete replication coordinator, `SingleZone`, which always indicates the caller is in an active zone.
ed57fc7 to
25ccac9
Compare
Note that this pins Rails to a version with ActiveSupport::ReplicationCoordinator see basecamp/rails#35
This is a draft PR to solicit some initial feedback.
I've got a branch of HEY using this at https://github.com/basecamp/haystack/pull/7837 which is deployed in the beta1 environment.
I've got a branch of Solid Queue using this interface at flavorjones/solid_queue#1
And I've got a MovableWriter gem that implements this interface for replicated SQLite (still very rough, needs to be tested and the processes table does not yet do the right thing) at basecamp/fizzy#580
Motivation / Background
It's common for applications that are deployed across multiple availability zones (using a replicated database) to create an ad-hoc method for processes to discover which zone is "active", meaning the zone primarily responsible for writing to the database.
For example, a team may choose to use a MySQL system variable to indicate the data center where the primary database sits. In which case, they need to write code to make sure all Rails processes in all zones query this efficiently (it may be slow to access in non-primary zones) and are notified if the primary zone changes, as in the case of a data center failover.
Detail
ReplicationCoordinator::Baseis introduced to allow developers to write code that determines whether a process is in an active zone, and then:Additionally, a test helper class is provided to simplify testing failover behavior.
Finally, Rails is configured by default to use a simple concrete replication coordinator,
SingleZone, which always indicates the caller is in an active zone.Additional information
TODO - link to the Solid Queue PR once it's open
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]