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

Debounced updatable #261

Merged
merged 5 commits into from
Mar 1, 2023
Merged

Debounced updatable #261

merged 5 commits into from
Mar 1, 2023

Conversation

julianrubisch
Copy link
Contributor

Enhancement

Description

One of the big downsides of using CableReady::Updatable in the wild is that it can lead to a lot of noise on the ActionCable connection. This is because every change on a model that enables updates leads to a ping being sent over ActionCable due to the usage of ActiveRecord callbacks.

This PR introduces a debounce: option that takes an ActiveSupport::Duration argument that will only send out one ping for this model per debounce period.

It makes use of a simple internal per-process MemoryDebounceAdapter, which is just a thin synchronized layer around a Hash.

Preparations are made, however, to override this adapter using a CableReady::Updatable.debounce_adapter mattr_accessor, which can be overridden in an initializer. This should make it possible to devise other adapters (for example based on DRb or Layered Caching) in the future.

Checklist

  • My code follows the style guidelines of this project
  • Checks (StandardRB & Prettier-Standard) are passing

@netlify
Copy link

netlify bot commented Mar 1, 2023

Deploy Preview for cableready ready!

Name Link
🔨 Latest commit 0288894
🔍 Latest deploy log https://app.netlify.com/sites/cableready/deploys/63ff7afb231a2b0008214e7c
😎 Deploy Preview https://deploy-preview-261--cableready.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@julianrubisch julianrubisch self-assigned this Mar 1, 2023
@julianrubisch julianrubisch added ruby Pull requests that update Ruby code enhancement labels Mar 1, 2023
Copy link
Contributor

@hopsoft hopsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great.

app/models/concerns/cable_ready/updatable.rb Show resolved Hide resolved
@@ -30,11 +50,14 @@ def self.enable_cable_ready_updates(*options)
options = options.extract_options!
options = {
on: [:create, :update, :destroy],
if: -> { true }
if: -> { true },
debounce: 0.seconds
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would make sense to have the default provide some degree of protection. Perhaps 100 milliseconds?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I switch on the debounce time in broadcast_updates to gracefully allow the default behavior. But sure, why not!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually I just noticed that this breaks all the tests, so we'd have to specify debounce: 0 everywhere. I'd recommend merging this PR and then introducing this in a separate one to keep things readable

@julianrubisch
Copy link
Contributor Author

Also note that this doesn't yet factor in the case of enable_updates on associations (that would be next on my list)

@hopsoft
Copy link
Contributor

hopsoft commented Mar 1, 2023

Will association support land in a separate PR?

@julianrubisch
Copy link
Contributor Author

Will association support land in a separate PR?

I thought it would be wiser to split it, yes

@julianrubisch julianrubisch merged commit e0b81ec into main Mar 1, 2023
@julianrubisch julianrubisch deleted the debounced-updatable branch March 1, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants