Truncate causes performance issues with locks

[EricSoroos]

Describe the bug

datapusher-plus/ckanext/datapusher_plus/jobs.py

Line 1056 in 7b8bc6f

# truncate table to use copy freeze option and further increase

Here, you note that truncate increases the speed that copy runs at. We're finding that with larger CSVs, the combination of truncate + a long running operation causes lock issues.

Truncate acquires an AccessExclusive lock, which essentially blocks all other queries, including select. This includes the select count(_id) from tbl that datastore's resource_fields runs .

This has the effect of blocking datastore_info or any datastore_search queries until the copy command terminates, which can be well over the time that the load balancer is willing to wait. Since we're (datastore) not takaing explicit nowait AccessShare locks prior to running queries, we just wait and wait and wait until the copy completes.

The PG documentation that you link to goes on to say:

However, this consideration only applies when wal_level is minimal as all commands must write WAL otherwise.

AFAIK, WAL Minimal isn't a common configuration for organizations interested in durable storage.

To Reproduce
Steps to reproduce the behavior:

1. Upload a big CSV. OMM, 40Mb will cause a 100 sec copy command
2. Run datastore_info, or possibly indexing, or other operations on that resource using datastore at that time.
3. See select count(*) block till the copy is complete and committed.

This might be clearer if you upload enough to generate the view, then upload a new, huge CSV, and visit the view.

[jqnatividad]

Thanks for detailed report @EricSoroos .

I'm thinking we can keep it simple by removing the FREEZE option from COPY, obviating the need for TRUNCATE.

Alternatively, we can have a configurable size threshold, below which, we do the TRUNCATE/COPY FREEZE optimization. Over the threshold, we skip the TRUNCATE and COPY without FREEZE.

One other approach is to use a staging table - i.e. COPY to a new temporary table (so no need to truncate), then do an ALTER TABLE RENAME on success.

WDYT?

[EricSoroos]

I think we're going simpler than that -- basically backing out the optimization. In cases where we need to truncate, we'll do it in a separate transaction so that the exclusive lock is only held for ms instead of minutes. The only drawback here is that without the truncate in the same transaction, a copy error will result in an empty table rather than a rollback. However, we'd have a (somewhat) silent failure there where the uploaded file was out of sync with datastore, so maybe this more obvious failure would be better.

My reading of the WAL level sentence is that the optimization perf boost was never something we'd see, because we're running wal_level=replica and not wal_level=minimal.

I'm definitely not interested in threshold tunings -- small copies are fast enough anyway.

I think the table rename is going to be a problem in at least a few of my environments, because we're doing things with views that are going to block if the underlying table is deleted and recreated.

My gut feeling here, prior to patching on our systems (which should happen Real Soon Now) is that the speed up is theoretical, and the lock contention is real.

[jqnatividad]

Agreed! Let's use the KISS principle 😄

Since you have the sample workloads that trigger this, do you mind preparing a PR?

FYI, you may want to check @a5dur's DP+ Testing Framework while you're at it - #256 (comment)

[EricSoroos]

I've run a few tests on one of my systems:

• As expected, the truncated/copy freeze didn't really help performance. In fact, running truncate in a separate transaction actually improved the speed, 149 sec to 157 sec.
• Selects aren't blocked.

I'm adding a PR, though I'm really not sure how to test the lock issue. All other behavior should be the same.

(note that we're running on an older fork, but this is a super simple change)