fix: deadlock in table lock

zhyass commented 4 weeks ago

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

Problem Overview

This PR addresses a deadlock issue that occurs when max_running_queries is set to 1 in Databend, resulting in errors like "table locked by other session." The root cause of the deadlock is the inconsistent timing of acquiring table locks during query execution, which leads to conflicts in acquiring the necessary locks.

Root Cause

The deadlock happens because queries acquire table locks at different stages of query processing:

Some queries acquire the lock during the binder phase.
Others acquire it during the interpreter phase.

This inconsistency creates a situation where:

A query already in the execution queue (and actively running) may not be able to acquire a lock.
Meanwhile, a query still waiting in the queue may hold the required table lock, blocking the active query and causing a deadlock.

Conditions That Trigger the Issue

The issue is most likely to occur when the max_running_queries setting is set to 1.
Under these conditions, only one query is allowed to run at a time, exacerbating lock contention between queued and running queries.

Solution

To resolve the deadlock, this PR introduces a new lock acquisition strategy for queries that need require a table lock(merge into, recluster, update, delete, compact, truncate, replace, ..):

Queueing After SQL Parsing: For queries that require a table lock, they are queued immediately after the SQL statement is parsed, rather than waiting after binder.
Lock Acquisition After Queuing: The table lock is only acquired once the query exits the queue and begins execution. This ensures that the query can safely acquire the lock when needed, without blocking other queries.

This change ensures a more consistent and deadlock-free process of table lock acquisition across different types of queries.

Impact of the Fix

The solution avoids the race condition between queued queries and running queries by ensuring that the lock acquisition only happens after queuing, preventing deadlocks and improving overall query execution stability in scenarios with limited concurrency.

Tests

[ ] Unit Test
[ ] Logic Test
[ ] Benchmark Test
[x] No Test - Explain why

Type of change

[x] Bug Fix (non-breaking change which fixes an issue)
[ ] New Feature (non-breaking change which adds functionality)
[ ] Breaking Change (fix or feature that could cause existing functionality not to work as expected)
[ ] Documentation Update
[ ] Refactoring
[ ] Performance Improvement
[ ] Other (please describe):

This change is

zhang2014 commented 3 weeks ago

Can we add new check for sampling queries in the is_heavy_action function to solve this?

github-actions[bot] commented 3 weeks ago

Docker Image for PR

tag: pr-16632-05d0254-1729432372

note: this image tag is only available for internal use, please check the internal doc for more details.

github-actions[bot] commented 3 weeks ago

ClickBench Report

github-actions[bot] commented 3 weeks ago

Docker Image for PR

tag: pr-16632-f36fe72-1729447292

note: this image tag is only available for internal use, please check the internal doc for more details.

github-actions[bot] commented 3 weeks ago

databendlabs / databend