search

rails / solid_cable

A database backed ActionCable adapter
MIT License
230 stars 16 forks source link

solid_cable somehow gets tangled in connects_to writing/reading #47

Open wdiechmann opened 4 days ago

wdiechmann commented 4 days ago

I'm "on all the latest jazz" ie

Rails main (Rails 8.1.0.alpha) Ruby 3.3.3 activerecord (8.1.0.alpha c11312a) solid_cable 3.0.2 sqlite3 (2.2.0) activerecord-enhancedsqlite3-adapter (0.8.0)

Pushing the envelope I've separated the writing from the reading - that does not come cheap 😞 - and currently I'm trying to work out why I get this error in my logs

2024-11-19T15:35:38.182583927Z [2243a178-dae9-43f7-8bf5-718e4f1a4dc5] ===============================
2024-11-19T15:35:38.183087168Z [2243a178-dae9-43f7-8bf5-718e4f1a4dc5] ERROR on destroy: Write query attempted while in readonly mode: INSERT INTO "solid_cable_messages" ("created_at","channel","payload","channel_hash") VALUES ('2024-11-19 15:35:38.162670', x'315f74696d655f6d6174657269616c73', x'225c7530303363747572626f2d73747265616d20616374696f6e3d5c2272656d6f76655c22207461726765743d5c2274696d655f6d6174657269616c5f345c225c75303033655c75303033632f747572626f2d73747265616d5c753030336522', -5145717094829308382) ON CONFLICT  DO NOTHING RETURNING "id"
2024-11-19T15:35:38.183598289Z [2243a178-dae9-43f7-8bf5-718e4f1a4dc5] ===============================
2

Firstly I gueessed config/cable.yml but adding reading: cable did not change her mind :/

# Async adapter only works within the same process, so for manually triggering cable updates from a console,
# and seeing results in the browser, you must do so from the web console (running inside the dev process),
# not a terminal started via bin/rails console! Add "console" to any action or any ERB template view
# to make the web console appear.
development:
  adapter: async

test:
  adapter: test

production:
  adapter: solid_cable
  connects_to:
    database:
      writing: cable
      reading: cable
  polling_interval: 0.1.seconds
  message_retention: 1.day

Currently my only comfort is that "production" is limited to myself and other family only 😜

wdiechmann commented 4 days ago

hmm - changing it to

production:
  adapter: solid_cable
  connects_to:
    database: cable
      # writing: cable
      # reading: cable
  polling_interval: 0.1.seconds
  message_retention: 1.day

only made everything worse:

2024-11-19T15:56:38.294460800Z ⚠️ [DEPRECATION] Defining the `template` method on a Phlex component will not be supported in Phlex 2.0. Please rename `Superform::Rails::Components::SelectField#template` to `Superform::Rails::Components::SelectField#view_template` instead.
2024-11-19T15:56:38.537169627Z Exiting
2024-11-19T15:56:38.539241148Z /usr/local/bundle/ruby/3.3.0/bundler/gems/rails-c11312a91855/activerecord/lib/active_record/connection_handling.rb:99:in `block in connects_to': undefined method `each' for an instance of Symbol (NoMethodError)
2024-11-19T15:56:38.539284188Z
2024-11-19T15:56:38.539291188Z         database_keys.each do |role, database_key|
2024-11-19T15:56:38.539296468Z                      ^^^^^
2024-11-19T15:56:38.539300988Z  from /usr/local/bundle/ruby/3.3.0/bundler/gems/rails-c11312a91855/activerecord/lib/active_record/connection_handling.rb:98:in `each'
2024-11-19T15:56:38.539323028Z  from /usr/local/bundle/ruby/3.3.0/bundler/gems/rails-c11312a91855/activerecord/lib/active_record/connection_handling.rb:98:in `connects_to'
npezza93 commented 4 days ago

What happens if you wrap https://github.com/rails/solid_cable/blob/c696aaa3f177b510cf7dffe525c4bb9916457320/app/models/solid_cable/message.rb#L14 in something like: ActiveRecord::Base.connected_to(role: :writing)

npezza93 commented 4 days ago

hmm - changing it to This made it worse because solid_cable still sees the connects_to but its empty so it errors

wdiechmann commented 4 days ago

What happens if you wrap

https://github.com/rails/solid_cable/blob/c696aaa3f177b510cf7dffe525c4bb9916457320/app/models/solid_cable/message.rb#L14

in something like: ActiveRecord::Base.connected_to(role: :writing)

you mean - bundle open solid_cable and "patch" it? - how do I get that to stay when I kamal deploy ?

npezza93 commented 4 days ago

you mean - bundle open solid_cable and "patch" it?

yea

how do I get that to stay when I kamal deploy

You would need to fork the repo and then update your gemfile to point to your fork.

wdiechmann commented 4 days ago

you mean - bundle open solid_cable and "patch" it?

yea

how do I get that to stay when I kamal deploy

You would need to fork the repo and then update your gemfile to point to your fork.

Rgr

npezza93 commented 4 days ago

If that doesnt work, can you create basic rails app with repro instructions and send it over and ill investigate further.

wdiechmann commented 4 days ago

it's a wrap ❤️

but honestly - to me it "smells" duck-tape, second-hand pizzabox, and "real Italian virgin olive oil out'a Wuhan" or am I just picky?

npezza93 commented 4 days ago

Did that work?

wdiechmann commented 4 days ago

Did that work?

yep - thx a lot - I knew it would 'cause I've done 2-3 like additions in my own repo, but that I had to go "upstream" did not sit well with me - anyways - I don't know how this is fixed but I sure don't like that "we" will have to litter the solid gems like this -

But, hey, perhaps that's totally ok - in any case: thank you again for reaching out and offering a quick fix ❤️

npezza93 commented 4 days ago

Ill reopen this and work on fix.

wdiechmann commented 4 days ago

Ill reopen this and work on fix.

if you need someone to debug or validate - just yell 😆

npezza93 commented 4 days ago

@wdiechmann Ok put up https://github.com/rails/solid_cable/pull/48. Mind giving it a go?

wdiechmann commented 4 days ago

update 20/11/2024 10:41 CET: what a rabbit hole this turns out to be 😞 My Docker Desktop somehow had died under me (and as I only rarely consult the GUI I hadn't noticed!) Why - I don't know but kamal registry login apparently cannot work if DD is defunc'ing - it's nothing to do with actually being logged in or not... Go figure

update 20/11/2024 8:40 CET: seems to be an issue with "docker login" ATM - so I'll have to postpone the test, but from the looks of it there's no big surprise

this whole 'role' thing, though, smells, to me, like it belongs somewhere else entirely - 'cause all inserts/updates/deletes would potentially taste the salty bitter tears right?

a bit under the weather - but no problem - give me 5-10min

wdiechmann commented 3 days ago

@npezza93 it's good!

wdiechmann commented 3 days ago

just remembered that this was not the first encounter! https://github.com/fractaledmind/activerecord-enhancedsqlite3-adapter/issues/19

We most def should attack this issue not by patching all calls to INSERT/UPDATE/DELETE - but that's just my 2cents 😎

npezza93 commented 3 days ago

What does your database.yml look like by chance? Im having trouble reproducing in a test case

npezza93 commented 3 days ago

I added a test case here https://github.com/rails/solid_cable/pull/48/files for reference @wdiechmann

wdiechmann commented 3 days ago

Sorry - AFK - but will BRB 😎

wdiechmann commented 3 days ago

@fractaledmind led me on my way with this -

my config/database.yml looks like this:

...
production:
  writer:
    <<: *default
    database: storage/production.sqlite3
  reader:
    <<: *default
    database: storage/production.sqlite3
  cache:
    <<: *default
    database: storage/production_cache.sqlite3
    migrations_paths: db/cache_migrate
  queue:
    <<: *default
    database: storage/production_queue.sqlite3
    migrations_paths: db/queue_migrate
  cable:
    <<: *default
    database: storage/production_cable.sqlite3
    migrations_paths: db/cable_migrate

and my cable.yml 

...
production:
  adapter: solid_cable
  connects_to:
    database:
      writing: cable
      reading: cable
  polling_interval: 0.1.seconds
  message_retention: 1.day
npezza93 commented 3 days ago

weird i cant reproduce with that same setup. are you able to spin up a new minimal rails app that reproduces?

wdiechmann commented 3 days ago

? - no - like I wrote - the fix (both the first and the somewhat more elegant second one) did the trick - my only concern is that there must be like 10-12 (at least) places all over the place where this would have to be duplicated which to me does not seem very DRY - and that's not counting other gems cutting their own 'deal' with the writing/reading/whatever they come up with - right?

wdiechmann commented 3 days ago

sorry if that did no translate well enough - english is not my native tongue (danish born and bread) 😅

wdiechmann commented 2 days ago

I added a test case here https://github.com/rails/solid_cable/pull/48/files for reference @wdiechmann

not sure activerecord-enhancedsqlite3-adapter buys that one - if I'm not misreading it will rewrite and use 'writer' and 'reader' (and if configured otherwise I'm not sure how it operates - but in any case:

I would upvote a conclave between maintainers of Rails Core, solid_*, sqlite3, more addressing the read/write challenge and (possibly an agenda with room to consider) introducing the locker room accepting that the root cause in fighting concurrency is one of resource allocation more and SQL writing/reading less