VertxExecutionContext issue

[Narigo]

@galderz I get errors when trying to run the tests here. If you want to check these out, you'll need a MySQL and/or PostgreSQL installed and see the relevant tests that pass but throw errors. The relevant branch is this:

https://github.com/vert-x/mod-mysql-postgresql/tree/execution-context-bug

Check out AsyncConnectionPool - this is where I get the ExecutionContext.

And here is a gist of the test output, maybe it helps already:
https://gist.github.com/Narigo/8739a91c014f69c55846

[galderz]

Looking...

[galderz]

@Narigo I've started mysql locally and run the tests, and all I see is: https://gist.github.com/galderz/f87aabb3453f6ce46d8b

[galderz]

I'm running mysql on localhost, on port 3306.

[galderz]

@Narigo I think there's a mismatch between what you are trying to do and what Vert.x does. Let me explain.

The whole point of the VertxExecutionContext.fromVertxAccess is that you get an execution context that runs within the same thread as the Vert.x event loop, instead of spinning a new thread. For this to work, you need that whatever access you make to execution context happen within the event loop thread. This is because the context is a thread local variable which is set in a vertx-worker-* or vertx-eventloop-* thread.

The problem that you are having is that you're asking to access the execution context of vert.x when you are running in a db-async-default-thread-* thread. IOW, you're not running this code from within a verticle. You are outside of a verticle and you expect to get the event loop thread and just reused that for whatever you are doing. That doesn't fly and not sure it really makes sense. You don't want to be hijacking the event loop to do stuff that is not linked to a verticle.

[jfrabaute]

Hi,

I'm using vertx and scala for a rest api. My rest api is calling a service returning a future.
Because the current VertxExecutionContext is not switching to the vertx event loop, it does not work correctly.

I've looked at the code and I'm stuck on the latest VertxExecutionContext change:

24a4146

First

the old code was doing this:

vertx.runOnContext(runnable.run())

This looks suspicious: First, the runnable will run, then the function runOnContext is called with the result of the function, which is nothing. This still compiles fine with scala :-O
I suppose vertx still switches to its event loop to execute a "noop" function.

Then

the fix, changes it by running the runnable directly, which means that we won't switch to the vertx event loop, and that should be the purpose of this code.

Do I understand this code correctly ? If so, A basic fix would be:

override def execute(runnable: Runnable): Unit =
  vertx.runOnContext(new Handler[Void] {
    override def handle(v: Void): Unit = { runnable.run() }
  }

Can you confirm?

Thanks.

[galderz]

@jfrabaute I think it depends who has created the future. If the future is created from within Vert.x, then the thread will already be a Vert.x thread and hence we don't need to call runConContext (see here for an in-depth discussion). If that Future is created by a another component that's not running in the Vert.x event loop, then I think your suggestion might work.

Did you try out your solution?

[tsuna]

Yes we implemented this and it helped remove the flakiness in our CI in our environment.

[purplefox]

Executing vertx.runOnContext() from a non Vert.x thread will not cause the action to be run on a new context, not on the context of the verticle (which is what you want). If you think about it, how is Vert.x supposed to know which verticle you're referring to with just vertx.runOnContext().

The correct way to do this is in two steps:

1. Get a reference to the context of the Verticle. This is available as a member of the verticle or you can use Vertx.currentContext(), call this "myContext"
2. When you have your async result that you want to execute in the context of the verticle, do:

myContext.runOnContext(...)

[jfrabaute]

@purplefox : You're right, my fix is using the context of the verticle that I'm passing when I create the ExecutionContext.
The snippet I put on the comment is not the entire code.
Here is the method I'm using as a workaround for the actual problem:

  // The returned execution context will ensure the future will be executed
  // in the vertx event loop
  def getExecutionContext(context: Context, logger: Logger) = {
    new ExecutionContext {
      override def reportFailure(t: Throwable): Unit =
        logger.error("Error executing Future in VertxHelper.getExecutionContext", t)
      override def execute(runnable: Runnable): Unit =
        context.runOnContext(new Handler[Void] {
          override def handle(v: Void): Unit = {
            runnable.run()
          }
        })
    }
  }

I'm calling this method in the verticle, passing the current vertx context, and assign it to an implicit, so the futures will automatically use this execution context.

@galderz If the future is created within vertx (specific case), you're right that there is an extra method call, and extra switching to thread pools, etc. But it should still works, and works in the general case, as the current version right now does not work in the general case but only in the specific case.
If you want to optimize the specific case, you could have a singleton instance of an ExecutionContext that's calling directly the runnable, and pass it to the Future that you know will already run in the vertx context.
Something similar to this might work (scala pseudo-code, the syntax might be wrong... :-O):

object SameExecutionContext extends ExecutionContext {
  override def reportFailure(t: Throwable): Unit =
    logger.error("Error executing Future in VertxHelper.getExecutionContext", t)
  override def execute(runnable: Runnable): Unit = runnable.run()
}

This can be used like this (still pseudo-code):

import ...SameExecutionContext

class ... extends Verticle {

  ...
  futureThaRunsInTheVertxContext.andThen({ case r => println(r) })(SameExecutionContext)
  ...

}

[jfrabaute]

@galderz BTW, I was partially wrong in my initial comment. The old version was correct. I didn't check carefully the signature of your method, and passing runnable.run() will not run the method directly but will run it in the method call, which is correct.

Sorry about that.