add execution option for asyncpg_timeout

[blazewicz]

Describe the use case

In our company we use PostgreSQL, one of the tables that is quite big with over 1.5B records. We have few queries constructed with SQLAlchemy Core which we use on it and with help of indexes they're usually quite fast (100's of milliseconds at most). Lately we have discovered and issue with PG's query planner that would omit an index and would run a plan that was stalling the connection for multiple days. After running a particular API request many times our connection pool got exhausted and application basically crashed.

Natural solution to prevent this type of issues would be to implement some sort of timeout. Value of the timeout would depend on the expected execution time of given query. There are of course multiple ways to achieve this:

1. Set statement_timeout on current connection in case of PostgreSQL.
2. Wrap AsyncConnection.execute() in asyncio.wait_for().
3. Add option to pass timeout argument to asyncpg driver via for example execution_options argument.

Where 1. and 2. seems to be possible options I think it would be very useful if option 3. would be implemented in SQLAlchemy. Timeout mechanism is already built in in asyncpg driver but access to it would need to be added.

Databases / Backends / Drivers targeted

asyncpg for PostgreSQL

Example Use

try:
 row = await conn.execute(query, execution_options={'timeout': 3})
except TimeoutError:
 ... # handle case when query took to long to execute

Additional context

No response

[zzzeek]

it has to be qualified that it's an asyncpg-specific (edit: TBD. see next comment) setting but certainly we can accept PRs (With tests) for this.

[zzzeek]

there's no easy way to do driver neutrality on this one:

1. if we name it "timeout" or "query_timeout", or something generic, then we have to decide:
   a. drivers that dont have a timeout capabiltiy, do they silently ignore or raise?
   b. if we raise, then how does one make a driver-agnostic application including that driver, where they'd like "timeout" if available but they are OK with graceful degradation in behavior?
   c. if we silently ignore, that...i dont want to silently ignore. bad idea esp for a timeout
   d. we could maybe "warn" when it's ignored. but still...app hangs indefinitely, these are very hard debugging situations
   e. we run a huge risk of "timeout" behaviors being different on different backends. timeout in milliseconds, seconds, minutes, the timeouts being implemented with different strategies, etc. a "timeout" is really something people should look for explcitly in their driver
2. if we name it "asyncpg_timeout", then:
   a. applications might have to accommodate for different driver URLs, and anticipate all the possible drivers and set up their own "asyncmy_timeout" "psycopg3_timeout" etc which is both verbose and not future proof

we can ignore "1.e." above, or maybe call it something slightly more specific, like "async_timeout_secs", and then add another flag "timeout_degrade='error'|'warn'|'ignore' that defaults to 'error'. maybe do that. "async_timeout_secs" means if some driver has a timeout in minutes or something weird, we know that we have to do the math with the value.

cc @CaselIT for thoughts

[zzzeek]

let me just emphasize that we arent going to add any "timeout" mechanisms on our end using async workers, threads etc. , someone else had asked about that. this is strictly timeout params that the async (or sync) drivers accept explicitly in some way on a per-statement basis.

[zzzeek]

other drivers set up this timeout right now as a dialect-level option like mssql query_timeout and sqlite timeout.

is timeout as a fixed per-dialect setting mostly as useful as a per-execution option one?

[blazewicz]

Per-execution option is more useful because some queries are expected to be fast and we can tell something is going wrong after few seconds while other are expected to run for minutes or even hours. For example HTTP requests will timeout after 60 seconds so there's never point in waiting longer than that. On the other side, some scheduled tasks in the background that touch a lot of data do take 30 or so minutes. Its often impossible to find one perfect fit-them-all value.

[zzzeek]

per-dialect timeouts go right into the database URL so the issue of portability is solved.

for execution_option, an expedient patch is asyncpg_timeout, but something named "timeout" or anything that isn't asyncpg specific should raise for all other backends that dont support it, which means defaultdialect would need to pull it out and test and all that.

[CaselIT]

Why don't we instead add an execution option to pass kw arguments to the cursor? this way we could support all strange cursor options of any driver and would be portable?
The downside is what to do in case of multiple driver / database applications

something like

row = await conn.execute(query, execution_options={'driver_execute_kw': {'timeout': 3}})