Yes!! That was exactly it. Thanks for the tip. I had a 1.5 year-old test that looked like this:

    test "it sends a notification to the user channel if they are a member that is not present",
         %{user: user, room: room} do
      # This test uses another process
      Ecto.Adapters.SQL.Sandbox.mode(MyApp.Repo, {:shared, self()})
      
      # rest of the test
    end

I think my thinking was that I wanted just this test to use the shared mode of the sandbox and since every test runs in a separate process that Sandbox.mode would set the mode for that specific process (since you’re passing the pid in).

From the docs it seemed okay:

iex(3)> h Ecto.Adapters.SQL.Sandbox.mode

                              def mode(repo, mode)

Sets the mode for the repo pool.

The mode can be :auto, :manual or {:shared, <pid>}.

Although I guess I should have realized that the “repo pool” is a shared resource across multiple tests.

But for my understanding, this is what I think was happening:

1. Multiple tests are running concurrently (since we’re using async: true)
2. This test called Ecto.Adapters.SQL.Sandbox.mode(MyApp.Repo, {:shared, self()})
3. When the DBConnection.Ownership.Manager GenServer received the mode change request it checked in all of the existing transaction checkouts, including for tests that were still running!
4. The tests that were still running tried to continue to use their connection, but the Manager GenServer had already checked them in resulting in the warning at the top of this thread.
5. The tests that were still running would then check out a new connection, and since this new connection was running in a new db transaction, any database entries inserted in the previous connection no longer exist which caused related test failures such as foreign key exceptions

I plan to create a doc PR to add some sort of warning to the Ecto.Adapters.SQL.Sandbox.mode function, unless this can be considered a true db_connection bug (I’m using db_connection 2.0.5). It would be nice if Ecto.Adapters.SQL.Sandbox.mode would eventually cause a warning if it forces the check-in of unrelated existing check-outs. Also this part of the original error message is not technically correct in this scenario:

(DBConnection.ConnectionError) owner #PID<0.2195.0> checked in the connection

Since #PID<0.2195.0> did not initiate the check-in (at least this is that is my current understanding). It looks like you’re in the process of adding an improvement for this case. That PR changes the error message to #PID<0.3100.0> checked in the connection owned by #PID<0.3096.0> which is a definite improvement since #PID<0.3100.0> was the process that was running the test that called Sandbox.mode.

Also due to the new $callers tracking in db_connection that test runs just fine in async mode without having to change the sandbox pool mode.

Thanks again for the help in tracking this down!

In my case, I had the following line in both test_helper.exs and setup block.

Ecto.Adapters.SQL.Sandbox.mode(Blog.Repo, {:shared, self()})

Removing the line from test_helper.exs solved the problem.

Or the async Task can just send a message to the parent process. That makes it both more stable and predictable in the original code and more testable as well.

The error you’re seeing is an integral part of the db_connection package, which several DB packages step on (Postgres for Ecto included). I am not aware of a way to disable it though I haven’t looked at the source in detail either, I admit.

But, why being conservative about waiting for the tasks to finish? If Process.sleep(50) fixed the problem then I’d imagine that Task.await_many will finish in 50ms or less, making it an imperceptible lag.

To expand on this: this is a bandaid, and is done when you just give up and are like “no clue what needs this little extra timie to finish but I am not going to bother looking for it”.

Don’t do that. Be a good engineer, your future self and colleagues will thank you for it.

And yes I know it’s tests and people somehow drop (part of) their coding standards there, but IMO tests need love too!

I’ve read the comments and learnt a few things as well, especially on Process.sleep/1 and I assume :time.sleep/1 being a bandaid.
I learnt a few solutions, thanks to @almirsarajcic and @dimitarvp

1. You could send a message to the parent (test) process from the async process. In my case I needed to mock the async code. This is what I had initially:

test "blah blah blah" do
    test_pid = self()
    ref = make_ref()

    expect(MyMock, :some_fn, fn -> 
        send(test_pid, {:done, ref})  
    end)

    # Just before `end` of the `test` block, I asserted that I received the message from the async process:
    assert_receive {:done, ^ref}
     
    # Here, you can assert on any side-effects of the async code, like DB changes, etc.
end

This way, we are guaranteed that the async process returns before the test exits.

2. You can (and should always) use supervised tasks, which gives you the flexibility to start your Task Supervisor in the tests that need it. Here was my final setup:
   In MyApp.Application:

def start(type, args) do
    children = [MyAppWeb.Endpoint, # other children] ++ more_children()
      end

    # See https://hexdocs.pm/elixir/Supervisor.html
    # for other strategies and supported options
    opts = [strategy: :one_for_one, name: MyApp.Supervisor]
    Supervisor.start_link(children, opts)
  end

  defp more_children(env \\ Application.get_env(:my_app, :env))
  defp more_children(:test), do: []
  defp more_children(_env), do: [{Task.Supervisor, name: MyApp.TaskSupervisor}]

Then, in the function I am testing, I use a supervised Task:

def some_function do
  Task.Supervisor.async_nolink(MyApp.TaskSupervisor, fn -> 
    # do some work
  end)
  
  :ok
end

Finally, in my test, I start the task supervisor as part of my test setup:

describe "blah" do
  setup do
      start_supervised!({Task.Supervisor, name: MyApp.TaskSupervisor, strategy: :one_for_one})

      :ok
  end
  test "blah blah" do
      expect(MyMock, :some_function, fn -> 
         :ok
      end)
  end
end

I loved the second approach and it’s what I use, but I’m curious though. If I want to assert on DB change made by the async process, how else would I do that without adding a delay? I would still get the DB connection errors with this approach.