I don't think the build ID is needed.

I didn't try and change the query too much beyond the seq scan.

It looks like it was never used at all:

Ok, so in the query plan you have now, it's doing this nested loop where it finds the workspace IDs, then for each workspace ID it's doing this bitmap query:

                          ->  Bitmap Heap Scan on workspace_builds wb  (cost=4.16..9.50 rows=2 width=28)
                                Recheck Cond: (workspace_id = workspaces_1.id)
                                ->  Bitmap Index Scan on workspace_builds_workspace_id_build_number_key  (cost=0.00..4.16 rows=2 width=0)
                                      Index Cond: (workspace_id = workspaces_1.id)

That is, it first scans the index to find the pages to load, then scans the pages with the Recheck Cond.

Do you know whether this results in page locks for the transaction, or tuple locks (I'm assuming these are row-level locks)? Page locks have a greater likelihood of catching unrelated transactions.

And, some suggestions:

1. Can we just move the daily_cost directly to the workspace table? We only ever do computations with the most recent cost. If we really needed to keep it on the workspace_build for compatibility or querying history, we could put it in both places, and have this query only look at workspaces. That would remove a join as well. If we built an index of (org_id, owner_id) then we'd also be very unlikely to ever need to Seq scan the workspaces table for quotas.

2. If we don't want to go that far, we could add an index ON workspace_builds (workspace_id, build_number, daily_cost) that would allow the quota query to compute the results right from the index, so it'd never have to a bitmap scan and read whole pages.

The particular locks acquired during execution of a query will depend on the plan used by the query, and multiple finer-grained locks (e.g., tuple locks) may be combined into fewer coarser-grained locks (e.g., page locks) during the course of the transaction to prevent exhaustion of the memory used to track the locks.

I was under the impression it was row-level locks, but I admit that was a result of my experiments with very few rows. The docs seem to imply it could take out larger locks.

Can we just move the daily_cost directly to the workspace table? We only ever do computations with the most recent cost. If we really needed to keep it on the workspace_build for compatibility or querying history, we could put it in both places, and have this query only look at workspaces. That would remove a join as well. If we built an index of (org_id, owner_id) then we'd also be very unlikely to ever need to Seq scan the workspaces table for quotas.

Originally there was a suspicion we were getting this error from different transactions interring with CommitQuota. But the error reported can only occur between 2 serializable transactions, meaning it is CommitQuota interfering with itself.

I thought about moving daily_cost to it's own table entirely, but I'm not sure that would actually improve much.

I think this index + sorting by build number would be a large win: workspace_builds (workspace_id, build_number, daily_cost)

As I look at this query more, I don't see why we need to inner join workspaces again. All the information can be pulled from the latest_build subquery.

If we add the index (org_id, owner_id) on workspaces as well that is another win here.

According to the quote I pulled though, is the goal just to reduce the number of rows touched to prevent the lock from having to go more "coarse" to the page lock? The text implies the behavior depends on the memory availability.

I agree that we don't need to join to workspaces twice.

And yeah, my understanding is that with page locks, updating quota for an unrelated user could cause a serialization error if the builds happen to share a page. So it's undesirable to use page locks if we could get away with finer grained locking.

I realize that postgres can use page locks for memory reasons, and maybe there isn't anything we can do about that, but I'm also wondering whether it automatically uses page locks when it does a bitmap query, rather than tuple locks if we were able to use the index.

It might be faster to use wb.build_number, since the query planner has access to the workspace_builds_workspace_id_build_number_key index. That could be why it's having to do the

              ->  Sort  (cost=22.66..22.66 rows=2 width=44)
"                    Sort Key: wb.workspace_id, wb.created_at DESC"

I wondered about that too. I had a specific goal and did not want to start mutating the query too much to solicit feedback on different things.

Can I throw this up in another PR?

yeah, that works