quarkus icon indicating copy to clipboard operation
quarkus copied to clipboard

Agroal Connection Leak with Panache Streaming and without @Transaction

Open brunohpg opened this issue 2 years ago • 1 comments

Describe the bug

There's a connection leak when use hibernate stream without transaction.

The leak occur even when stream is closed.

There's no connection leak with @Transaction.

I started having this problem in version 2.13.2, but I don't know if it happens in older version.

Expected behavior

The connection should be closed (return to the pool) in the end of request or at least when stream is closed.

In case of non closed stream, the connection should be closed also. I think a warning message can be printed istead.

Actual behavior

The connection still allocated and when reach 20 connections an exception is throwed: Sorry, acquisition timeout!.

How to Reproduce?

I using postgres with non reactive api of ORM with Panache.

From Resource class:

@ApplicationScoped
public class ...

    @POST
    @Path("thePath")
    @Produces(MediaType.APPLICATION_JSON)
    public List<Dto> list() {
        return service.list();
    }

From Service class:

@ApplicationScoped
public class ...

    public List<Dto> list() {
            try (Stream<Ent> stream = repository.myQuery().stream()) {
                return stream .map(ent -> mapper.toDto(ent))
                        .collect(Collectors.toList());
            }
    }

Panache Class:

@ApplicationScoped
public class RepositoryX implements PanacheRepository<Ent> {

    public PanacheQuery<Ent> myQuery() {
            return findAll(Sort.ascending("myField"));
    }
  • Call the endpoint 21 times.

Output of uname -a or ver

Microsoft Windows [versão 10.0.19043.2006]

Output of java -version

OpenJDK Runtime Environment Zulu15.36+13-CA (build 15.0.5+3-MTS)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.13.2

Build tool (ie. output of mvnw --version or gradlew --version)

gradle-7.4.1

Additional information

Extra: sometimes a have tried to return Stream direct from endpoint but when response takes long time, the transaction is closed by timeout. There's some way to "revalidate" transaction timeout? It'll work with munity?

brunohpg avatar Oct 15 '22 22:10 brunohpg

/cc @FroMage, @Sanne, @barreiro, @loicmathieu, @yrodiere

quarkus-bot[bot] avatar Oct 15 '22 22:10 quarkus-bot[bot]

This is documented that all usage of stream() needs a transaction, see the end of this section https://quarkus.io/guides/hibernate-orm-panache#most-useful-operations

We already discussed years ago with @Sanne that we may want to enforce this but never make the move.

loicmathieu avatar Oct 17 '22 14:10 loicmathieu

It's interesting that it worked before.

It didn't work very well when I return a Stream as a response to a request.

Anyway, thanks. If this has already been planned, you can close the issue.

brunohpg avatar Oct 17 '22 15:10 brunohpg

@brunohpg it's been discussed but nothing has been planned. Someone from agroal may want to check why it stops working after 2.13 (even if it was not a good implementation before).

loicmathieu avatar Oct 17 '22 15:10 loicmathieu

@loicmathieu , @brunohpg : Is this known to be true for JPA/Hibernate persistence (without panache) as well? For instance, it looks like the following code never returns the connection to the pool when used in Quarkus with Agroal:

            TypedQuery<Car> query = em.createQuery(CAR_QUERY, Car.class);
            query.setParameter(carIdParam, carId);
            Stream<Car> carStream = query.getResultStream();
            Optional<Car> carOptional = carStream.findFirst();
            carStream.close();
            return car;

It doesn't seem to be documented in the quarkus hibernate-orm documentation. Would that warrant a separate issue?

mrickly avatar Oct 23 '23 07:10 mrickly

Thanks for bringing this up, and yes, please open another issue, because IMO this issue about Panache should be closed (this is not supported, and the fact it's not supported is documented). The new issue should focus on the fact that streams in Hibernate ORM (even without Panache) don't work well and that we should either document this or prevent using streams outside of transactions.

Also, FWIW:

  1. You should always use transactions anyway: https://quarkus.io/guides/transaction#why-always-having-a-transaction-manager
  2. When Agroal tells you "there is a connection leak", what it doesn't tell you is that it actually plugs the leak by closing the connection automatically. So you don't technically have a leak anymore. I do understand there's a bug regardless.

yrodiere avatar Oct 23 '23 09:10 yrodiere

@yrodiere : Ok, thank you, will open a new issue.

mrickly avatar Oct 23 '23 10:10 mrickly

Thanks. I'll close this one as the limitation in Panache is known and documented in https://quarkus.io/guides/hibernate-orm-panache#most-useful-operations

yrodiere avatar Oct 23 '23 10:10 yrodiere