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

Is there any value in seeing a backtrace for ConcurrencyExceededError? #347

Closed
reczy opened this issue Aug 24, 2021 · 2 comments · Fixed by #348
Closed

Is there any value in seeing a backtrace for ConcurrencyExceededError? #347

reczy opened this issue Aug 24, 2021 · 2 comments · Fixed by #348

Comments

@reczy
Copy link
Contributor

reczy commented Aug 24, 2021

Hi @bensheldon!

I'm curious if you (or other users) find any value in seeing the full backtrace for concurrency errors? In my case, concurrency errors are expected and common so I don't really view them as "errors" to be further investigated. I am using the following to silence the backtraces:

# config/initializers/good_job.rb

module GoodJob
  module ActiveJobExtensions
    module Concurrency
      class ConcurrencyExceededError < StandardError
        def backtrace
          [] # suppress backtrace
        end
      end
    end
  end
end

Does it make sense to do any of the following?

  • hardcode this in GoodJob, or
  • include an option to accomplish this in GoodJob via config, or
  • include some documentation mentioning that the above code (or something similar) can suppress the backtraces?
@bensheldon
Copy link
Owner

@reczy Is the backtrace cluttering up the logs?

I think suppressing the backtrace in code is appropriate. I don't think there is a use for the exception other than to trigger the behavior. I'd accept a PR if you wanted to rewrite this to include it:

ConcurrencyExceededError = Class.new(StandardError)

I also believe it would be better to directly retry the job, rather than raising an exception and catching it. But I haven't done that yet because:

  • ActiveJob doesn't expose retry functionality fully; incremental backoff and execution count have to be re-added (it's not too messy, but it's not simple either, and I haven't looked at if the interface changes across Rails versions).
  • I know of developers who are using discard_on(GoodJob::ActiveJobExtensions::Concurrency::ConcurrencyExceededError) because of GoodJob's Cron on multiple workers is enqueuing the same job (which is a downside of GoodJob's Cron but I don't want to break that workaround).

But that's beside the clear action step here: suppress the backtrace 👍

@reczy
Copy link
Contributor Author

reczy commented Aug 24, 2021

Is the backtrace cluttering up the logs?

I think it's a combination of that mixed with the fact that I don't need to know or act on the information provided in the backtrace.

Thanks - PR incoming...

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

Successfully merging a pull request may close this issue.

2 participants