Add performance improvements for opening websocket with SQL backed storage

[revilo464]

• Allow querying for multiple deviceIds when getting latest positions
• Perform the (expensive) query for devices belonging to user only once
• Create cacheable PreparedStatement
• Collapse 0L to NULL for query params for more SQL performance

Background

We found some issues that caused the websocket opening process to bog down the server. The biggest issue was that the original code fetches the latest position for all devices before filtering for the ones belonging to the user. Before filtering, however, it constructs the full-fledged entity object and with a substantial amount of devices, this can cause the JVM to allocate and then garbage collect almost immediately. With enough users opening websockets, this can grind Traccar to a halt. We solve this by passing the deviceIds first to the SQL query.

Before:

After:

Notice the CPU time per minute has a drastic difference.

There is another expensive query which determines which devices are accessible to a user, and in the current flow this was called twice. We further enhance the websocket opening process by calling this query only once and passing it downstream.

In the end, the performance seems quite stable in load testing so far.

[tananaev]

This definitely doesn't look right. If a user has thousands of devices, they shouldn't be included here one by one.

[tananaev]

I think the right solution is to do a single query that would combine devices query with positions.

[revilo464]

I agree. I’ll refactor.

[revilo464]

While working on this suggestion, I found that org.traccar.storage.DatabaseStorage#getObjects only instantiates objects of a given type, and the members should match the SQL ResultSet. In the case of joining Positions to Devices in the Devices query, I would only be able to instantiate one or the other, unless I created another type of object (something akin to DevicePosition).

Would this be your recommendation?

[tananaev]

I'm not proposing to join tables, but filter positions by devices.

[revilo464]

From your first message in this thread proposing a single query that would combine devices query with positions I see no other way to do this than performing a join...

Here would be the proposed query which would seem to work:

-- only adding aliases and the INNER JOIN here
SELECT * FROM tc_devices td
INNER JOIN tc_positions tp on tp.id = td.positionid 
WHERE td.id IN (SELECT tc_user_device.deviceId FROM tc_user_device WHERE userId = ? UNION SELECT DISTINCT deviceId FROM tc_user_group INNER JOIN (SELECT id as parentId, id as groupId FROM tc_groups UNION SELECT groupId as parentId, id as groupId FROM tc_groups WHERE groupId IS NOT NULL UNION SELECT g2.groupId as parentId, g1.id as groupId FROM tc_groups AS g2 INNER JOIN tc_groups AS g1 ON g2.id = g1.groupId WHERE g2.groupId IS NOT NULL) AS all_groups ON tc_user_group.groupId = all_groups.parentId INNER JOIN (SELECT groupId as parentId, id as deviceId FROM tc_devices WHERE groupId IS NOT NULL) AS devices ON all_groups.groupId = devices.parentId WHERE userId = ?);

---

But as mentioned the Traccar DB interface in its current form only allows returning a single type of object.

When you suggest, however, filtering positions by devices, that seems to be exactly what I'm doing with the IN operator in this PR. Your current implementation also filters positions by devices, but it does this only after creating Java objects from all of the latest positions, flooding the heap when the amount of devices in the DB is large (>40k).

Would love to implement this in a way the project would appreciate. 😃

[tananaev]

I don't understand why you need joins. What I'm proposing is something like this:

SELECT * FROM tc_positions WHERE id IN (
  SELECT positionId FROM tc_devices WHERE id IN (
    ... something similar to the current permission query
  )
)

[revilo464]

Thanks for the clarification Anton. I understand now.

However, this again adds the issue that we can't reuse the list of devices accessible to the user and this is the most expensive part of the servlet request right now. In all of my measurements the permission query takes the majority (>80%) of the wall time. I already refactored so it would only happen once. If we cannot save the deviceIds from the permission query then we repeat it again which doubles the wall time requirement of the servlet request again.

See attached what the servlet request looks like before removing the second call. Your proposal would require the second call again.

[tananaev]

That's a different issue and I think it's a reasonable trade-off.

[revilo464]

EDIT: However while this does work with MySQL, apparently Oracle has a limit at 1000, and MSSQL can have issues around 2000 apparently. (source)

I also wanted to add a followup on performance testing with a user with many devices (1383) with the current proposed solution (using SELECT * FROM tc_positions WHERE id IN ( SELECT positionId FROM tc_devices WHERE id IN ( :device0, :device1, :device2... ) )).

The query is still quite performant with the SELECT only taking 19ms on our setup. The devices permission query is immediately to the left with 487ms.