micronaut-projects / micronaut-sql

Projects to support SQL Database access in Micronaut
Apache License 2.0
74 stars 42 forks source link

Reactive postgres client doesn't create more than one connection to the DB #79

Closed guest-acc closed 3 years ago

guest-acc commented 5 years ago

For some reason, PgPool provided with micronaut 1.2.5 doesn't create more than one connection to the db, which is a serious perf bottleneck.

I do have simple controller:

@Controller("/")
public class TestController {

    @Inject
    PgPool client;

    @Get("/test")
    public Single<HttpResponse> test() {
        return client.rxQuery("select * from config")
                .map(rows -> HttpResponse.ok(rows.rowCount()));
    }
}

complete code: https://github.com/guest-acc/micronaut-my-postgres-app

The expectation is that with higher load (I do use ab: ab -t60s -c100 http://localhost:8080/test ), it will scale and create more connections.

All I do see is only one; I'm checking it via SELECT count(*) FROM pg_stat_activity where datname='postgres'; executed in the db.

When I did the same experiment with micronaut-data, micronaut actually created mutliple connections. I already did an experiment with standalone PgPool client and it also works as expected.

During investigation, I did rewrite the simple case into an async handling and noticed that the "handler" was always executed from the same thread; my assumption is that something like that could be happening in "native" reactive processing as well. The thread is named "Thread[vert.x-eventloop-thread-0,5,main]".

graemerocher commented 5 years ago

I am not sure why it behaves this way. Maybe @BillyYccc can comment. I know for the benchmark example with Micronaut we had to create multiple clients. See https://github.com/TechEmpower/FrameworkBenchmarks/blob/master/frameworks/Java/micronaut/src/main/java/benchmark/repository/PgClientFactory.java

This may be the "correct" way to set this up.

guest-acc commented 5 years ago

@graemerocher if that is a default, it should be documented.. (with super big exclamation mark, since it doesn't really make sense..)

graemerocher commented 5 years ago

Agreed, the module is/was an external contribution just like the jasync-sql support. If you wish to contribute to it to improve either the documentation or implementation feel free to send a PR

BillyYccc commented 5 years ago

Hi, I have taken a brief look at your code and that is expected behavior, let me make things clearer.

The Pool#query method will first get a connection from the connection pool, execute the query and then automatically returns it to the pool. So it’s possible there’s only one pooled connection in the pool which is capable of handling queries under a low load. But on a higher load, the number of connections will grow up to handle the enqueued queries, you’ll also find the max connections which are retained in the pool is configurable. If you want to manage the lifecycles of pooling connections by yourself you can use Pool#getConnection.

In addition, I would recommend you using the vertx-pg-client with Micronaut, the original Reactive PG Client has been donated to the Eclipse Vert.x community and only remains security fixes, so if you want newest features and updates you may need to try that, there is also good documentation in https://vertx.io/docs/vertx-pg-client/java/.

Hope my answer helps you :-)

guest-acc commented 5 years ago

@BillyYccc please run the example I shared and put any load there. There won't ever be more than single connection to the dabatase.

From my perspective, this is Blocker bug for whole reactive postgres module in micronaut, it's not usable for any reasonable production use - postgress.reactive.client.maxSize always behaves like it's set to 1, no matter what the value is there (it's correctly passed to the client, if I execute client in standalone java program, it creates more than 1 connection. If I'm using micronaut provided integration, it creates only 1 connection).

Also, as I stated, the issue is not in the client itself. It's in the way how it's integrated to micronaut / connected to netty event loops.

I know about vetrx-pg-client - but there is no published integration with micronaut yet, correct? (I did notice that directory in git repo, but I can't find anything in the docs or published artifacts..)

guest-acc commented 5 years ago

@graemerocher @BillyYccc I took another look and .. when I build https://github.com/micronaut-projects/micronaut-sql.git

locally (and publish it to maven local), I can see

~/.m2/repository/io/micronaut/configuration/micronaut-vertx-pg-client/1.3.0.BUILD-SNAPSHOT/

being created. But that artifact doesn't seem to be published anywhere:

https://search.maven.org/search?q=micronaut-vertx-pg-client https://dl.bintray.com/micronaut/core-releases-local/io/micronaut/configuration/

Is that an issue with build / release process?

guest-acc commented 5 years ago

@BillyYccc I did reproduce the original issue even with micronaut-vertx-pg-client, see:

https://github.com/guest-acc/micronaut-my-postgres-app/tree/micronaut-vertx-pg-client

no matter what load you run, it creates only single connection to the db.

BillyYccc commented 5 years ago

@guest-acc can you try the other tools such as wrk?

guest-acc commented 5 years ago

@guest-acc can you try the other tools such as wrk?

I already did that before starting this thread, produces same result.

guest-acc commented 5 years ago

@BillyYccc also, if you'd read first response from @graemerocher, you'll see that he confirms the observation with a hack he did for some perf test which is using the same client / micronaut integration.

guest-acc commented 5 years ago

We've made another observation today: when an implementation breaks thread barrier (meaning it gets off netty worker thread), it starts using multiple connections

    @Get("/test")
    public Single<? extends HttpResponse> test() {
        executorService.submit(() ->
                client.rxQuery("select * from config").map(rows -> HttpResponse.ok(rows.rowCount())).blockingGet());

        return Single.just(HttpResponse.ok());
    }

(the problem here is that I wasn't able to process the result into a response in a way which wouldn't stop creating additional db connections, but it's not relevant at this point).

@vietj I believe that there might be some unwanted connection with how Micronaut and Vert.x / Reactive PG client uses netty, resulting in the serialization of all queries to single vertex event loop / context.

I have to admit that I'm not so knowledgeable about Vert.x, so it's not simple to evaluate and analyze possible cause; @vietj could you please chime in? Have you seen anything like this already? Is there any setting or configuration which should be applied to reactive pg client to prevent such behavior?

Thanks for any reply, even if it would be just a suggestion of what we could try ;)

graemerocher commented 5 years ago

Probably would work adding the @Blocking annotation too as that would shift it off the event loop

guest-acc commented 5 years ago

Probably would work adding the @Blocking annotation too as that would shift it off the event loop

for some reason it doesn't.

I modified the REST endpoint:

    @Get("/test")
    @Blocking
    public Single<? extends HttpResponse> test() {

        System.out.println("XXX: " + Thread.currentThread().getName());

        return client
                .rxQuery("select * from config")
                .map(rows -> HttpResponse.ok(rows.rowCount()));
    }

and I'm still getting only one connection to the database.

The output says that the method is being invoked on threads "nioEventLoopGroup-1-", where is anywhere between 1 and 17.

BillyYccc commented 5 years ago

Sorry for having been too busy to take a deeper look into this, indeed this should be unexpected behavior for the sql client module, I will investigate this.

guest-acc commented 4 years ago

Hello @BillyYccc,

can we expect any activity on this in foreseeable future?

BillyYccc commented 4 years ago

I have tried with postgres client directly with Micronaut not in a DI way but also with no luck, currently we're reworking the SQL client and the internal connection pool to make it able to integrate with other frameworks in a friendlier way, we will revisit this issue when we progress on that.

tompoch commented 4 years ago

The issue still persists in micronaut-sql 3.0.0

dstepanov commented 3 years ago

I have tested it with Vertx 4 and it looks like it's fixed.