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

Replace redis with solidcache #4727

Open
4 tasks
cielf opened this issue Oct 20, 2024 · 6 comments
Open
4 tasks

Replace redis with solidcache #4727

cielf opened this issue Oct 20, 2024 · 6 comments
Labels

Comments

@cielf
Copy link
Collaborator

cielf commented Oct 20, 2024

Summary

Replace redis with solidcache

Why?

Slimming down our prod

Details

Check that everything we use redis for will still works. Make it so!

Criteria for completion

  • through check of what we are using redis for
  • if there isn't anything but the trend reports:
    • try cutting over on local
    • testing of trend reports if we use solidcache instead
@cielf cielf added Difficulty—Advanced Help Wanted Groomed + open to all! labels Oct 20, 2024
@jamesh38
Copy link

jamesh38 commented Nov 10, 2024

Hey there @cielf I took a look at this one today and have some discovery for you to mull over and then I can possibly tackle this one.

I was able to install SolidCache and get working fairly simply in the application.

However if the goal is to remove Redis completely from this application to simplify the production architecture then there are a few hurdles in the app so far as I can see. Hopefully this breakdown helps.

Places HE uses Redis (As far a I can tell)

Standard Rails Cache

In the HistoricalTrends::BaseController the cache is used to fetch an organization historical trend

This was functioning splendidly with SolidCache and I saw no real issues.
image
image

CoverBand Gem

As I'm sure you know the live code usage analysis tool CoverBand gem uses Redis as the place it stores coverage. As far as I can tell it does not have support for SolidCache as an "adapter"

This is one we would need to dig deeper on if the goal is to disconnect Redis from the app completely.

# config/coverband.rb
Coverband.configure do |config|
  config.store = Coverband::Adapters::RedisStore.new(Redis.new(url: ENV["REDIS_URL"]))
  config.logger = Rails.logger

  # default false. Experimental support for tracking view layer tracking.
  # Does not track line-level usage, only indicates if an entire file
  # is used or not.
  config.track_views = true
end

Action Cable

The websocket system action cable is currently configured to talk to Redis but interestingly enough only development. I imagine I could set this to async in the PR along with test and prod?

# config/cable.yml
development:
  adapter: redis
  url: redis://localhost:6379/1

test:
  adapter: async

production:
  adapter: async

On a side not should we consider moving this to SolidCable as well as DelayedJob to SolidQueue?

Conclusion

Other than that it seems to be working fairly reasonably. All tests lest for 1 are passing at this point. (Which may not mean much as the caching doesn't appear to be configured in test)

image

Maybe we should start testing caching in rspec now that its database bound and not in redis?

Let me know if any of this helps. If you'd like me to post the draft PR for simply replacing Rails cache with SolidCache. Or anything else. Thanks!

@cielf
Copy link
Collaborator Author

cielf commented Nov 10, 2024

@dorner, @awwaiid Can you take a look at this and comment?

@dorner
Copy link
Collaborator

dorner commented Nov 10, 2024

Out of the box, it looks like we'd have to choose between having Coverband with a Redis requirement, or not having it. TBH I don't know if anyone has ever looked at CoverBand in HE.

On the other hand, it looks like Coverband does support non-Redis stores: https://github.com/danmayer/coverband/blob/main/lib/coverband/configuration.rb#L187

We could build our own ActiveRecord store based on this API. Seems like someone did it and it took them an hour or so (shame they didn't post the code): danmayer/coverband#533 as long as we turn view tracking off.

Maybe let's take this to the leads meeting to discuss.

@awwaiid
Copy link
Collaborator

awwaiid commented Nov 10, 2024

@dorner I think we should abandon coverband for the benefit of the overall simplification. There is also this coverband PR that we can look forward to.

The note on changing the ActionCable config - yes, async in production I think is fine for our app.

@jamesh38
Copy link

jamesh38 commented Nov 16, 2024

Would ya'll like me to make a PR removing Coverband and adding SolidCache?

@dorner
Copy link
Collaborator

dorner commented Nov 16, 2024

Please do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants