ankane / ahoy

Simple, powerful, first-party analytics for Rails
MIT License
4.24k stars 377 forks source link

[Idea] Describe the configuration of sidekiq in the README. #468

Closed aiji42 closed 3 years ago

aiji42 commented 3 years ago

Hello. I am very grateful for your activities.

I have been using this gem and have encountered some problems. It's a problem with geocode queues not resolving, resulting in redis memory being over the limit. Upon investigation, we realized that we forgot to add ahoy in the queue setting in sidekiq.yml: while ActiveJob kept stacking the queues one after the other, sidekiq was not monitoring them and the queues were not being resolved. The discovery was delayed due to a slow increase in memory occupancy over several months.

:concurrency: 5
:queues:
  - default
  - ahoy

I would like to see an announcement in the README about the sidekiq setting in the geocoding section and the danger of the queue getting stuck forever if that setting is left unchanged. It might be safer to set the default value of Ahoy.geocode to false. (However, I'm aware that this proposal would be difficult to adopt because of the disruptive changes it would entail.)

That's all. Thank you for your consideration.

fatihtas commented 3 years ago

Hi, I am having the same problem, and I was trying to figure out where I have made the worker for Ahoy. Is there a way to set an expiration on those queues at least ? so redis memory can be set free afterwards.

aiji42 commented 3 years ago

@fatihtas Thank you for the question. I don't see a way to set an expiration for ActiveJob, and even if we could hack it, I don't think it should be set. The role of ActiveJob is to get the queued job to work properly without losing it, and it seems out of the concept to let it go without doing it. I think that you should control the number of parallel workers of sidekiq and the maximum number of memory of redis so that all queues can be handled properly.

fatihtas commented 3 years ago

Hi @aiji42

Thanks for the quick eply. You are right about the out of concept point.

my sidekiq.yml is set as follows: :concurrency: 4 production: :concurrency: 4 :queues: - ["critical", 3] - ["default", 2] - ["low",1] :verbose: true

and in my application_controller: skip_before_action :track_ahoy_visit, only: [abcd]

However I can't see a way to solve the problem yet. This is my sidekiq queues:ahoy list element sample:
"{\"class\":\"ActiveJob::QueueAdapters::SidekiqAdapter::JobWrapper\",\"queue\":\"ahoy\",\"args\":[{\"job_class\":\"Ahoy::GeocodeV2Job\",\"job_id\":\"c8f426de-9f83-4b8d-a24b-3195d033d042\",\"queue_name\":\"ahoy\",\"arguments\":[\"297a49a2-dc60-41f1-9171-4ca52d2a9b01\",\"162.158.74.141\"]}],\"retry\":true,\"jid\":\"b1191515ace150be619bf0cf\",\"created_at\":1607767724.092067,\"enqueued_at\":1607767724.092164}" In 6 hours, there are about 500 of them added up. and in about 3 weeks it fills up entire 30mb redis memory. Our redis memory was never filled up in the past year until I set up ahoy last month. + Increasing memory is not a solution as there is no money enough for this kinda growth. I suspect I set up ahoy wrong in some way, either queues in redis are not being processed, or processed but this list data stays there unless being removed manually?

I did not set any custom workers for ahoy so it is taking jobs to sidekiq automatically i guess.

What was your case? Did you able solve it and if so, can you please explain how?

aiji42 commented 3 years ago

@fatihtas My solution is this code (described at the beginning of this issue).

:concurrency: 5
:queues:
  - default
  - ahoy # I added this line

In my environment, the reason the queue keeps accumulating is that sidekiq subscribes only in the default namespace, even though ActiveJob enqueues in the ahoy namespace. When I added the ahoy namespace, the queue was successfully handled. Also, geocoding wasn't needed for analysis in my project, so I turned it off in the settings. Ahoy.geocode = false

aiji42 commented 3 years ago

Or I think it can be solved by changing the namespace to default or low. Ahoy.job_queue = :default

fatihtas commented 3 years ago

Ohh I see :) Thanks a lot, appreciate your lead 👍

ankane commented 3 years ago

Hey @aiji42, thanks for the suggestion! I've decide to disable geocoding by default for new installations for privacy (https://github.com/ankane/ahoy/commit/a7d718b66bbc6a924584e2d82909d113a4769720). The readme (both current and past) includes information about the job queue, but feel free to submit a PR if it can be improved. Ahoy may not need it's own job queue by default, so will considering changing this for Ahoy 4 (#450).