eclipse-vertx/vertx-sql-client

Uncompleted transaction Future after close connection

kkoderok opened this issue · 4 comments

Version

vertx-pg-client:4.4.5

Context

Some requests may stuck in case with network problems. As a result such requests cannot be reprocess or implement failure logic.

Do you have a reproducer?

Test in CloseConnectionTest in the vertx-pg-client module:

@Test
public void testCompleteTransactionFuture(TestContext ctx) {
  ProxyServer proxy = ProxyServer.create(vertx, options.getPort(), options.getHost());
  proxy.proxyHandler(conn -> {
    conn.connect();
    vertx.setTimer(1_000, l -> conn.close());
  });

  proxy.listen(8080, "localhost", ctx.asyncAssertSuccess(v1 -> {
    options.setPort(8080).setHost("localhost");

    PgPool pool = PgPool.pool(vertx, options, new PoolOptions().setMaxSize(1));
    pool.withTransaction(conn -> conn.query("select pg_sleep(60)").execute())
      .onComplete(ctx.asyncAssertFailure())
    ;
  }));
}

Steps to reproduce

  1. Run test

Expectation

Complete transaction Future with exception

Extra

In my case I changed call order in the PgCodec#channelInactive (first call super.channelInactive and then fail). It allows to set closed status in PgSocketConnection and other all requests will be failed.

vietj commented

I can see that too, can you elaborate how the control flow changes by setting the status to closed when the channel inactive is set first @kkoderok ?

As I can see, when a connection is closed, the method PgCodec#fail completes query execution future with exception, by calling ctx.fireChannelRead(failure). This result leads to rollback of current transaction in the withTransaction method. The PgSocketConnection has no disconnection status, so it sends rollback command. In this case we add ClosePortalCommandCodec in the deque (PgCodec.inflight), but current iterator doesn't notice this command.
If we change call order, rollback request will be immediately failed with exception by PgSocketConnection.

vietj commented

thanks , I missed the rollback

vietj commented

thanks for the reproducer and analysis, the codec classes should actually record the failure and fail new command registered against the codec, so in response of the failure, the rollback command will be fail immediately by the codec. Your suggestion was also correct, but relying on the timing instead on the failed state that should be recorded to prevent execution of the pipeline for new commands.