Presence List/Fetch Override: Too many unnecessary database calls, 4+ calls per join, how to fix?

Greetings,

I am using the documentation to set up a simple working example for phoenix.presence, and I am using the fetch override to call the database and get user details as described here: Phoenix.Presence — Phoenix v1.6.2

Here are my scripts so far:

/assets/js/socket.js (client side)

import {Socket, Presence} from "phoenix"
let socket = new Socket("/socket", {params: {token: window.token, foo: window.bar}})

socket.connect()
let channel = socket.channel("room:lobby", {})
let presence = new Presence(channel)

function renderOnlineUsers(presence) {
  let response = ""

  presence.list((id, stuff) => {
    console.log(id+' '+JSON.stringify(stuff))
    response += '<br>${id}</br>'
  })

  document.querySelector("#UserList").innerHTML = response
}

presence.onSync(() => renderOnlineUsers(presence))
export default socket

/lib/exchat_web/channels/presence.ex

defmodule ExchatWeb.Presence do
  alias ExchatWeb.Accounts

  use Phoenix.Presence, otp_app: :exchat,
                        pubsub_server: Exchat.PubSub

  def fetch(_topic, presences) do
    IO.inspect(presences)
    users = presences |> Map.keys() |> Accounts.get_users_map()

    for {key, %{metas: metas}} <- presences, into: %{} do
      {key, %{metas: metas, user: users[key]}}
    end
  end

end

This is pretty much the basic examples found in the documentation. Here is an example of what happens when someone joins:

Let’s say these user ids are already in the chat room:

[299, 1999, 18100, 18, 1]

When a new user id “2000” joins, I’m seeing this in the Phoenix logs:

[info] CONNECTED TO ExchatWeb.UserSocket in 1ms
"join room"
"handle after join"

"run fetch - call database"
"run fetch - call database"
[debug] QUERY OK source="members" db=0.9ms queue=0.8ms idle=302.2ms
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (m0.`id` IN (?,?,?,?,?,?)) [299, 2000, 1999, 18100, 18, 1]

"run fetch - call database"
[debug] QUERY OK source="members" db=1.1ms queue=3.1ms idle=0.0ms
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (m0.`id` IN (?)) [2000]
"run fetch - call database"

"empty query: ids to find = []"
[debug] QUERY OK source="members" db=1.1ms queue=3.1ms idle=0.0ms
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (false)

[debug] QUERY OK source="members" db=0.6ms queue=45.3ms idle=0.0ms
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (m0.`id` IN (?,?,?,?,?,?)) [299, 2000, 1999, 18100, 18, 1]

In addition, if that same user leaves the chat room, two uneccessary calls are done:


"run fetch - call database"
"empty query"
[debug] QUERY OK source="members" db=1.1ms queue=3.1ms idle=0.0ms
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (false)

"run fetch - call database"
[debug] QUERY OK source="members" db=0.5ms queue=0.7ms idle=1553.2ms
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (m0.`id` IN (?)) [2000]

When someone joins, such as user id “2000”, shouldn’t there just be two calls - one call to grab the details just that one user to display for the existing users, and one call to grab all the user details to display to the new user? For example:

SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (m0.`id` IN (?)) [2000] # display this to existing users
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (m0.`id` IN (?)) [299, 2000, 1999, 18100, 18, 1] # display this to the new user - or simply only just use this query for both the existing and new user, send it to everyone?

and when a user leaves, shouldn’t it just remove that user from the presence instead of trying to run another query to fetch the details for the user that is leaving?

I’m using the simplest example from the documentation, so I’m not really sure how to fix/optimize this for minimal database calling. I’d greatly appreciate some advice.

Thanks

Anyone have insight on this?

IMO if you make a small (but complete) GitHub repo demonstrating your issue, then people might be more willing to look into it.

As it is, your question includes just a few coding snippets, some not even in Elixir.

The two elixir snippets are pretty much all there is and comes from the very basic documentation examples. There were a couple of other unresolved concerns on the internet about this exact problem related to the fetch override and duplicate queries being called:

The rest of the non-elixir stuff below the presence.ex code is just my custom IO.inspect to show when a query is run and what the query looks like (to show which ids are trying to be fetched). I added that there just to show that there are indeed 4 queries being called for just one person joining the room (two queries are exact duplicates), and 2 queries for one person leaving the room.

The problem is, I don’t even know why or where this many queries are being called. I think it’s an unnecessary waste of database resources. It might even be a bug.

It looks like fetch/2 is called twice on each presence diff: once for all the joins and once for all the leaves.

That seems to correlate with what you are seeing under your “if that same user leaves the chat room, two uneccessary calls are done”

# No one left so no users in the join list.
# Recommendation: in fetch/2, don't do any database call if the list is empty
[debug] QUERY OK source="members" db=1.1ms queue=3.1ms idle=0.0ms
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (false)

"run fetch - call database"
# New user joined - have to look them up
[debug] QUERY OK source="members" db=0.5ms queue=0.7ms idle=1553.2ms
SELECT m0.`id`, m0.`username`, m0.`gender`, m0.`status`, i1.`path` FROM `members` AS m0 LEFT OUTER JOIN `images` AS i1 ON i1.`id` = m0.`mainimageid` WHERE (m0.`id` IN (?)) [2000]

So maybe its a simple as not doing a DB call is the user list is empty because that means there was either no leave or no join.?

1 Like

Thanks for the response. Last week, I had actually figured out how to do just what you described: Help: Check My Work - Fetching Basic Presence Details With Ecto - Advice Welcome - #14 by John-Goff , to simply not do a DB call if the user list is empty.

That didn’t solve the issue entirely, because in the join, there are two duplicate calls to fetch all user details for all of users, plus one call to fetch the user details for the one user joining, and in the leave, there is an unnecessary call to fetch user details for a user who is about to leave the room, which doesn’t make sense since they are leaving the room anyways, why grab their details.

It would be nice if there was just one fetch - after someone joins the room (so both the room and new user get the full list of users in the room) - no fetching when a user leaves. How would I configure that?

I noticed the same thing and will open an issue on the Phoenix repo suggesting that fetch/2 be called in such a way that it’s possible to know if the call is for join or leave.

2 Likes

Thanks for the help. In the meantime, I may have fixed this by altering the deps code by customizing fetch/2 into fetch/3:

/deps/phoenix/lib/phoenix/presence.ex:

    def handle_diff(diff, state) do
      {module, task_supervisor, pubsub_server} = state

      Task.Supervisor.start_child(task_supervisor, fn ->
        for {topic, {joins, leaves}} <- diff do
          Phoenix.Channel.Server.local_broadcast(pubsub_server, topic, "presence_diff", %{
           joins: module.fetch(topic, Phoenix.Presence.group(joins), "join"),
           leaves: module.fetch(topic, Phoenix.Presence.group(leaves), "leave")
          })
        end
      end)

      {:ok, state}
    end

(note to self: Use command tmix deps.compile --force to update/recompile the deps changes. Because this is a deps file, it will probably get overwritten during any framework update.)

Then in the /lib/exchat_web/channels/presence.ex, I updated the fetch there as well to fetch/3:

 def fetch(_topic, presences, diff) do
    users =
    if(diff == "join") do
       presences |> Map.keys() |> Accounts.get_users_map()
    else
      %{}
    end

    for {key, %{metas: metas}} <- presences, into: %{} do
      {key, %{metas: metas, user: users[key]}}
    end
  end

What this should do is skip database calls for any “leave” events. Also, it appears that the script is now calling just one query when a user joins, just what I was hoping for. What seems really cool now is that the query is fetching the user details for just the one user who joins the room, instead of redundantly fetching details for every single user on every single join.

From what I can see (so far), there are no errors and everything works great. I’m not familiar with the entire phoenix/presence architecture, so I don’t know if there are any unintended problems that will arise elsewhere in the framework because of this change, or if I’m missing anything.

Let me know what you think.

Yes, when you update the framework your additions will be overwritten. Probably better to fork the repo and use the fork as a GitHub dependency. Not ideal of course, but if it works for you for now …

In your fetch/3 above, you should return the presences map even when its being called on "leave". A somewhat more idiomatic implementation would be:

def fetch(_topic, presences, "join") do
  users = presences |> Map.keys() |> Accounts.get_users_map()

  for {key, %{metas: metas}} <- presences, into: %{} do
    {key, %{metas: metas, user: users[key]}}
  end
end

def fetch(_topic, presences, _) do
  presences
end
3 Likes

I may have bungled this a bit… After doing tests, today, I am seeing that when a new user joins the room, the script only fetches the user details for just that one user. Everyone else in the room sees the user details for the new user, but the new users sees a bunch of blank details for everyone else already in the room. I could have sworn the other day, Phoenix was saving the user details to the presence, alongside the metas, and just fetching details only for just new users joining the room. It worked perfectly since I didn’t have to fetch the user details for every single user, every time a new user joins the room.

Just for clarification, when presence runs the “fetch” function, is the user detail data supposed to get saved to the “user” key in presence indefinitely like the “metas”, or is that just a one-time fetch that goes to the client, but doesn’t actually get saved in presence or anywhere else in Phoenix?

Going back to square one again, I set up fetch/3 for the joins/leaves, and set up fetch/2 as well, to get the script working the way it was:

def fetch(_topic, presences, "join") do
    users = presences |> Map.keys() |> get_int_list() |> Accounts.get_users_map()
    for {key, %{metas: metas}} <- presences, into: %{} do
      {key, %{metas: metas, user: users[key]}}
    end
  end

  def fetch(_topic, presences, _) do
    presences
  end

  def fetch(_topic, presences) do
    users = presences |> Map.keys() |> get_int_list() |> Accounts.get_users_map()
    for {key, %{metas: metas}} <- presences, into: %{} do
      {key, %{metas: metas, user: users[key]}}
    end
  end

I am now seeing that the list() function ( phoenix/presence.ex at master · phoenixframework/phoenix · GitHub ), is being called twice whenever a user joins the room, and it fetches all of the user details each time. Why does it get called twice? It should only be called once.