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

Possible regression when handling unique advisory lock #1112

Closed
michelboaventura opened this issue Jun 27, 2024 · 4 comments
Closed

Possible regression when handling unique advisory lock #1112

michelboaventura opened this issue Jun 27, 2024 · 4 comments
Labels
area:oss Related to Oban OSS kind:bug Something isn't working

Comments

@michelboaventura
Copy link

Precheck

  • Do a quick search and make sure the bug has not yet been reported
  • For support, favor using the Elixir Forum, Slack, IRC, etc.
  • Be friendly and polite!

Environment

  • Oban 2.17.11
  • PostgreSQL: 12.16
  • Elixir: 1.16.3 (compiled with Erlang/OTP 26)
  • OTP: 26

Current Behavior

Hey!

I have this simplified version of a worker on my project. The important part is the create method:

defmodule Worker do
  use Oban.Worker,
    queue: :index,
    unique: [
      # for the next 2 seconds, starting on job's insertion time
      period: 2,
      # check for each incoming job, whether or not worker and args are the same
      fields: [:args, :worker],
      # from the args, use id, action and resource
      keys: [:id, :action, :resource],
      states: [:available, :scheduled]
      # then, replace the current persisted job `scheduled_at` by the incoming job.scheduled_at
      # replace: [scheduled: [:scheduled_at]]
    ]

  require Logger

  @spec create(number) :: {:ok, Oban.Job.t()} | {:error, any}
  def create(id) do
    %{id: id}
    |> new(replace: [scheduled: [:scheduled_at]], schedule_in: 1)
    |> Oban.insert()
    |> case do
      {:ok, job} ->
        {:ok, job}

      {:error, error} ->
        Logger.notice("Failed to create Worker job for '#{id}': #{inspect(error)}")
        {:error, error}
    end
  end
end

Recently we've improved our visibility and started sending all sort of errors to Sentry. After that we started seeing a ton of errors
on the case statement within create similar to (I've unwrapped the error in a more legible format):

# CaseClauseError. No case clause matching:
%Oban.Job{
  id: nil,
  state: "scheduled",
  queue: "index",
  worker: "Worker",
  args: %{id: 12345},
  meta: %{},
  tags: [],
  errors: [],
  attempt: 0,
  attempted_by: nil,
  max_attempts: 20,
  priority: nil,
  attempted_at: nil,
  cancelled_at: nil,
  completed_at: nil,
  discarded_at: nil,
  inserted_at: nil,
  scheduled_at: ~U[2024-06-27 20:15:06.514996Z],
  conf: nil,
  conflict?: true,
  replace: [scheduled: [:scheduled_at]],
  unique: %{
    timestamp: :inserted_at,
    keys: [:id, :action, :resource],
    period: 2,
    fields: [:args, :worker],
    states: [:available, :scheduled]
  },
  unsaved_error: nil
}

According to Oban's doc and typespec Oban.insert/1 should never return anything other than {:ok, job} or {:error, error} which makes this behavior odd. After looking at 400+ errors the only thing in common they have is the conflict?: true flag, so after reading throughout Oban's source code I came across this line which seems to be returning only the job and only when a conflict happens. And since this line was changed a couple of days ago I believe this might be a regression.

Expected Behavior

Oban.insert() should only return {:ok, job} or {:error, error}.

@sorentwo sorentwo added kind:bug Something isn't working area:oss Related to Oban OSS labels Jun 28, 2024
@sorentwo
Copy link
Member

Thanks @michelboaventura, that was indeed a regression. Dialyzer didn't help here and that condition is prohibitively difficult to replicate in tests because of the sandbox.

@michelboaventura
Copy link
Author

Thanks @sorentwo. Indeed. I tried reproduce it locally running a bunch of async tasks but was never able to do so.

@michelboaventura
Copy link
Author

@sorentwo not trying to rush anything but do you think it would be possible to cut a new release fixing this today? Thanks

sorentwo added a commit that referenced this issue Jun 28, 2024
The advisory lock clause only returned the job, not a success tuple.

Closes #1112
@sorentwo
Copy link
Member

@michelboaventura Done! Oban v2.17.12 is out with this specific fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:oss Related to Oban OSS kind:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants