Relax the locking on INSERT INTO ... SELECT queries
- Dominant language
- C
- Stars
- 12.8k
- Forks
- 794
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 31
Description
For replication factor > 1, by default, we acquire `ExclusiveLock` on all the shards that the `SELECT` query hits during the query execution. This ends up blocking `INSERTs` (`UPDATEs` and `UPSERTs` as well) on those shards.
The locking logic is implemented on [`RequiresConsistendSnapshot()`](https://github.com/citusdata/citus/blob/master/src/backend/distributed/executor/multi_router_executor.c#L486) function. In summary, acquire `ExclusiveLock` on the shards that the subquery hits when (a) the replication factor > 1 and (b)` all_modifications_commutative` flag is set to false. This is the default behaviour for replication factor > 1.
Some previous discussion on this [issue](https://github.com/citusdata/citus/pull/855#discussion_r82634586) which explains why we need the locks.
Some discussion in the code which [explains](https://github.com/citusdata/citus/blob/master/src/backend/distributed/executor/multi_router_executor.c#L518) how to avoid this locking with ` all_modifications_commutative` flag.
This locking behavior may introduce performance issues. A common example could be running long roll-up query from a big raw data table into an aggregated table. In that case, the raw data table is blocked for writes.
We discussed some ways to relax this, and I'm noting them:
1. Don't solve it, document the need to have converging INSERT/SELECT
2. Take an exclusive lock on the shards from which we are selecting for as long as the INSERT/SELECT is ongoing
3. Set the transaction isolation level on the workers to repeatable read and take an exclusive lock on the shards from which we are selecting for as long as it takes to get a snapshot.
4. Disallow INSERT/SELECT when replication factor > 1
We picked the option 2 above to progress fast during the PR. But, it is obvious that option 3 prevents excessive locking that we've introduced with the option 2 for replication factor > 1.
Any other options to improve this?
Contributor guide
Research direction
Start with RequiresConsistendSnapshot() in src/backend/distributed/executor/multi_router_executor.c and read the linked discussion in PR 855, including the comments around the all_modifications_commutative logic. Compare the four listed approaches and establish the intended locking and isolation behavior; no test file is named, so completion criteria and validation tests still need to be defined.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, postgresql
- Domain
- databases, distributed-systems
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 20/100