Trigger eager session initialization at startup (fixes #99)

[adutra]

@UnvirtualHH we think that eager initialization of the session should fix your issue. Would you mind trying out this branch if you have time? Thanks!

[UnvirtualHH]

I will test it this evening/tomorrow morning. I've also talked with Clement Escoffier about that issue and he checked it a little bit and we are both not sure if your Cassandra Client is really non-blocking. Maybe you should also check this again and if needed investigate some more work into changing it to non blocking. I'm pretty sure Clement could also help or give you a hint, if it is needed. Maybe I'm totally wrong and your client is already non-blocking, I'm absolut not an expert it in this.

[adutra]

I will test it this evening/tomorrow morning. I've also talked with Clement Escoffier about that issue and he checked it a little bit and we are both not sure if your Cassandra Client is really non-blocking. Maybe you should also check this again and if needed investigate some more work into changing it to non blocking. I'm pretty sure Clement could also help or give you a hint, if it is needed. Maybe I'm totally wrong and your client is already non-blocking, I'm absolut not an expert it in this.

That's a very good question, and we probably should add a note to the docs about being non-blocking.

In short the driver is non-blocking except for the following methods:

1. Blocking API execution methods such as Session.execute(String).
2. SessionBuilder.build() when creating the session;

For 1) it's the user responsibility to only use the non-blocking API methods when querying. I think it is your case already if you are using executeReactive methods exclusively.

For 2) however, it's indeed tricky: we do offer SessionBuilder.buildAsync() but that returns a CompletionStage which is I think useless in the context of dependency injection (you would be injecting the future, not the result of that future).

So we've been using the blocking method SessionBuilder.build() so far in this extension, and that is why you were having trouble. I think that this will be fixed with this PR since SessionBuilder.build() is now being called eagerly at startup, and not inside a Vertx thread.

However if you or @cescoffier have better ideas or any suggestion to improve this aspect of the extension, we would love to hear from you. Thanks!

[cescoffier]

I think the SessionBuilder.buildAsync() should be ok in @UnvirtualHH's case (if I'm not mistaken it's a reactive route - which explains why it ended up on the event loop.

[UnvirtualHH]

@cescoffier SessionBuilder.buildAsync() isn't possible to use for me, cause I'm using the injected session instance of the client.

I will give you all feedback after my integration tests where run and I've intensively checked all cases with this branch.

[UnvirtualHH]

@adutra I've tested it for every of my microservices and it works like expected. Also with forced errors that previously raised those blocking failures it works now. Much thanks for your quick fix!

[absurdfarce]

The effect of this change is to create the QuarkusCqlSession on the Quarkus main thread at startup time. In general this seems right since it guarantees a singleton instance (which is what we want for now anyway) and avoids any possibility of trying to do that setup on the Vertx event loop threads.

Note that this also means that the session is created as soon as the app starts and that any failure to successfully create this session will cause the entire app to fail to start successfully. Again, I don't necessarily think this is a fatal problem... but it's definitely a change in behaviour. For example, switch the contact points to some made-up IP address and watch the errors fly by when you start the app. In prior versions this behaviour wouldn't have been seen until somebody tried to access a resource which actually used the session.

I mention this here only to be very explicit about these changes in behaviours and to confirm that we're all right with them.

[absurdfarce]

To be clear: my concern here is for an app which contains many JAX-RS resources, only some of which use Cassandra. Under the prior impl these other resources would be available (and functioning) even if Cassandra was not. Under the impl contained in this PR this would no longer be the case. There's certainly an argument that this represents an improvement (in the spirit of failing fast) but it's not immediately clear to me that this is true across the board.

So I'm wondering... should be make this a configurable option users could choose to go to in cases where it affects them? Keep in mind that in the majority of cases lazy initialization works reasonably well. That said, I'm quite sympathetic to an argument which says that adding configuration options almost always represents an increase in complexity and that this increase may not be worth the flexibility in this case.

[adutra]

I am totally on board with you @absurdfarce. Have you seen JAVA-2832? I've started the discussion around providing an uninitialized session in the driver API, @olim7t already replied.

Note that this also means that the session is created as soon as the app starts and that any failure to successfully create this session will cause the entire app to fail to start successfully.

@olim7t suggested that advanced.reconnect-on-init addresses that.

should be make this a configurable option users could choose to go to in cases where it affects them?

Agreed, I will do that and push a commit shortly.

[adutra]

Note that one of @olim7t 's suggestions to avoid eager initialization at startup is the ability to inject CompletionStage<QuarkusCqlSession>.

But I am not sure this is possible.

First, the deployment module currently requires a QuarkusCqlSession instance to register a shutdown hook, see here.

Secondly, the Quarkus DI container does not seem very comfortable with injecting generic types, BeanContainer has no method to retrieve an instance of a generic type, only instance of a raw Class.

But even assuming that we solve these two issues, this may not be the ideal solution. Injecting a stage instead of a session makes it hard to implement service components, especially if the mapper is involved. IOW, it's not possible to inject a mapper or dao instance if all you have at hand is a session future, you have to inject a future of your mapper/dao:

@ApplicationScoped
public class FruitReactiveService {

  private final CompletionStage<FruitDaoReactive> fruitDaoStage;

  @Inject
  public FruitReactiveService(CompletionStage<FruitDaoReactive> fruitDaoStage) {
    this.fruitDaoStage = fruitDaoStage;
  }

  public Uni<Void> add(Fruit fruit) {
    return Uni.createFrom()
        .completionStage(fruitDaoStage)
        .onItem()
        .produceUni(dao -> dao.update(fruit));
  }

  public Multi<Fruit> get(String id) {
    return Uni.createFrom()
        .completionStage(fruitDaoStage)
        .onItem()
        .produceMulti(dao -> dao.findById(id));
  }
}

So I am playing now with the idea of a lazy wrapper for QuarkusCqlSession. This seems to work well for cases where the session itself is injected. The code is here.

However it doesn't solve the problem when a dao must be constructed and injected. The generated dao code is inherently blocking and blocks until all statements have been prepared.

So it seems that if users want the Object Mapper, they have to accept eager initialization at startup.

[adutra]

I'm going to merge this, the user that opened #104 is facing issues with blocked threads.

I also opened #105 for the reconnect-on-init stuff.