Closing database connection while query is still pending

Question

Closing database connection while query is still pending

LouisTSpi opened this issue a year ago · 2 comments

Hello,

Using v2.1.3 of Amphp/Mysql, I need to execute queries on multiple db servers, with the following requirements:

queries run in parallel: one query per db server
if one or more server is in error (network issue, server down, sql error...) OR one or more servers take more than a given time to respond: ignore the response, execution should continue with the results from the other servers not in error or timeout
always close the opened db connection

The current implementation uses Amp\Promise\timeout to wrap the execution of each query, and AmpPromise\any combinator to execute the promises in parallel:

echo 'Begin' . PHP_EOL;

$start = microtime(true);

$globalPromise = Amp\call(function () use ($start) {
  $servers = ['server1', 'server2'];
  $user = 'db_user';
  $pass = 'db_pass';
  $dbName = 'db_name';

  $promises = [];

  foreach ($servers as $server) {
    // Create the promise for a single server
    $serverPromise = Amp\call(function () use ($start, $server, $user, $pass, $dbName): \Generator {
      try {
        echo "Executing {$server} promise" . PHP_EOL;

        // 1 second query for server1, slow 100 seconds query for server2
        $query = $server === 'server1' ? 'SELECT SLEEP(1)' : 'SELECT SLEEP(100)';

        // Retrieve the db connection
        $serverConfig = Amp\Mysql\ConnectionConfig::fromString("host={$server};user={$user};pass={$pass};db={$dbName}");
        $db = Amp\Mysql\pool($serverConfig);

        // Wrap the execution of the query in a 2 seconds timeout promise
        $timeoutDbPromise = Amp\Promise\timeout($db->execute($query), 2000);

        // Return the result when the promise resolves
        return yield $timeoutDbPromise;
      } catch (\Throwable $th) {
        // Catch connection errors, sql errors, Amp\TimeoutException...
        $errorTime = microtime(true) - $start;
        echo 'Caught: "' . $th->getMessage() . '" on ' . $server . ' after ' . $errorTime . PHP_EOL;

        // Rethrow to be handled by Amp\Promise\any combinator below
        throw $th;
      } finally {
        // Always try to close the connection to the db to prevent clogging mysql with useless connections
        if (isset($db)) {
          $db->close(); // However this does not close the connection when there is still a pending query
        }
      }
    });

    // Add the server promise to the list of all promises
    $promises[] = $serverPromise;
  }

  // Wait for all promises to settle and return the result
  [$errors, $resultSets] = yield Amp\Promise\any($promises);

  return [$errors, $resultSets];
});

$res = Amp\Promise\wait($globalPromise);
$time = microtime(true) - $start;
echo 'Got ' . count($res[1]) . ' results and ' . count($res[0]) . ' errors' . PHP_EOL;
echo "Done in {$time}" . PHP_EOL;

This will output

Begin
Executing server1 promise
Executing server2 promise
Caught: "Operation timed out" on server2 after 2.0047299861908
Got 1 result and 1 error
Done in 2.004860162735

The only thing that is not working as expected is that when $db->close is called in the finally, it does not close the connection if a query is still pending.
In this test case example with SLEEP(), the long query is still visible in mysql SHOW FULL PROCESSLIST, and after some time the connection is aborted and SHOW GLOBAL STATUS's Aborted_clients is incremented by 1.
(In my practical case the issue is even worse because the table is in a metadata lock state so the query end up 'waiting for metadata lock', and the connection is never aborted: the query stays in queue until the lock is released, meanwhile other queries try to get connections to the db and in the end the server reaches 'Too many connections')

Looking into the internals of Amphp\Mysql, it seems the $db->close() method ends up calling Amp\Mysql\Internal\Processor->sendClose() which itself calls startCommand() with a callback to actually send the COM_QUIT signal to the server. startCommand appends the task to the queue, but as the initial request is still pending, it seems the callback is never called and the connection is left open.

Is there a way to force the connection to close even when there is a pending query?
Or is there something wrong with this implementation?

Thanks

Answer 1 · 2023-12-30T19:07:01.000Z

Hi @LouisTSpi!

Graceful closing of the connection requires sending a command to the server, which as you noticed needs to go into the queue. Cancelling a MySQL query requires issuing another query on a different connection, KILL QUERY process_id. Unfortunately we don't have an API exposed for this, and one would not be trivial to design or implement. I'm considering adding support in v3, but it's certainly not something that will be added to v2.

The other option is to abruptly close the socket connection, but this will leave the table in a metadata lock state again.

I suspect the table is being left in a lock state because the close operation is not being allowed to complete before the script is terminated. Instead of using Amp\Promise\wait(), wrap everything into Amp\Loop::run() to ensure any further tasks are executed before the script ends.

I'd recommend upgrading to v3 if you're able. The API is much cleaner and the use of fibers would have avoided the close issue, as the fiber will suspend until the connection closes.

Answer 2 · 2024-01-10T18:17:28.000Z

Hello @trowski,

Thank you for your reply.

I think I was not clear enough about the metadata lock state, let me rephrase to try and clear things up.
My practical case arises when the table on one server is already in a metadata lock state (due to reasons independent from my app or amphp/mysql) when I issue the queries using amphp/mysql. It is not the queries made with amphp/mysql that render the table in the lock state, so I'm not expecting amphp/mysql to change the lock state of the table.

For example the situation would be:

there are 2 db servers (server1 and server2)
On server2, the table the query is selecting from is already in a "Waiting for table metadata lock" state
while server2 table is in this lock state, the app uses amphp/mysql as in the example above to query both servers in parallel
server1 promise resolves with its results
server2 does not respond in time due to the lock: its timeout promise fails and execution for this server goes through the catch and finally statements
$db->close() is called for connection to server2 but does not actually close the connection
php returns the results of server1 and execution stops.
the query issued on server2 stays in MySQL's processlist until the table metadata lock is released, even long after php execution stops

The other option is to abruptly close the socket connection, but this will leave the table in a metadata lock state again.
I suspect the table is being left in a lock state because the close operation is not being allowed to complete before the script is terminated.

I'll try force closing the socket. As said above, since the table was already in the lock state before issuing the queries, it does not really matter if the table is left in the lock state afterwards. What matters is that the connection is closed, so that mysql can accept new connections to answer other queries (not all queries use the locked table).

I'd recommend upgrading to v3 if you're able. The API is much cleaner and the use of fibers would have avoided the close issue, as the fiber will suspend until the connection closes.

I've tested an implementation using v3 beta but I get the same result. I'm considering migrating the production app once amphp/sql, amphp/sql-common and amphp/mysql are stable. As far as I can tell they are still in beta.