You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by StephanEwen <gi...@git.apache.org> on 2016/08/11 17:22:36 UTC

[GitHub] flink pull request #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the...

GitHub user StephanEwen opened a pull request:

    https://github.com/apache/flink/pull/2360

    [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the RpcEndpoint

    NOTE: This builds on top of #2357 - only the second commit belongs actually to this pull request is relevant.
    
    Add a `scheduleRunAsync()` method to the `RpcEndpoint`. It behaves like `runAsync()` but delays the call by a given number of milliseconds. The delay does not happen by a thread sleep, but by scheduling the message that triggers the Runnable it into the future of the message dispatcher.
    
    This also adds tests for the `runAsync()`, `scheduleRunAsync()`, and `callAsync()` that validate that all these calls actually run in the same thread (the RPC endpoint's main thread).

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/StephanEwen/incubator-flink schedule_future

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/flink/pull/2360.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2360
    
----
commit 13f6f392943e32b14bf0d08c7ded2d88496911ab
Author: Till Rohrmann <tr...@apache.org>
Date:   2016-08-10T16:42:26Z

    [FLINK-4362] [rpc] Auto generate rpc gateways via Java proxies
    
    This PR introduces a generic AkkaRpcActor which receives rpc calls as a
    RpcInvocation message. The RpcInvocation message is generated by the
    AkkaInvocationHandler which gets them from automatically generated Java Proxies.
    
    Add documentation for proxy based akka rpc service

commit 7b9eb2f3de23a7bee28346664ff39e0c28235a6e
Author: Stephan Ewen <se...@apache.org>
Date:   2016-08-11T17:10:48Z

    [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the RpcEndpoint

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the RpcEnd...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/2360
  
    Will address the comments and merge this.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen closed the pull request at:

    https://github.com/apache/flink/pull/2360


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2360#discussion_r74563211
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rpc/MainThreadExecutor.java ---
    @@ -52,4 +52,13 @@
     	 * @return Future of the callable result
     	 */
     	<V> Future<V> callAsync(Callable<V> callable, Timeout callTimeout);
    +
    +	/**
    +	 * Execute the runnable in the main thread of the underlying RPC endpoint, with
    +	 * a delay of the given number of milliseconds.
    +	 *
    +	 * @param runnable Runnable to be executed
    +	 * @param delay    The delay after which the runnable will be executed
    +	 */
    +	void scheduleRunAsync(Runnable runnable, long delay);
    --- End diff --
    
    That is true. The `TimeUnit` variant would make it a little bit more explicit for the user that he has to think about the actual delay (unit-wise). On the other hand, milliseconds are also kind of the default time unit used also in other places.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2360#discussion_r74562696
  
    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/rpc/akka/AsyncCallsTest.java ---
    @@ -0,0 +1,178 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.runtime.rpc.akka;
    +
    +import akka.actor.ActorSystem;
    +import akka.util.Timeout;
    +
    +import org.apache.flink.runtime.akka.AkkaUtils;
    +import org.apache.flink.runtime.rpc.RpcEndpoint;
    +import org.apache.flink.runtime.rpc.RpcGateway;
    +import org.apache.flink.runtime.rpc.RpcMethod;
    +import org.apache.flink.runtime.rpc.RpcService;
    +
    +import org.junit.AfterClass;
    +import org.junit.Test;
    +
    +import scala.concurrent.Await;
    +import scala.concurrent.Future;
    +import scala.concurrent.duration.FiniteDuration;
    +
    +import java.util.concurrent.BlockingQueue;
    +import java.util.concurrent.Callable;
    +import java.util.concurrent.LinkedBlockingQueue;
    +import java.util.concurrent.TimeUnit;
    +
    +import static org.junit.Assert.*;
    +
    +public class AsyncCallsTest {
    +
    +	// ------------------------------------------------------------------------
    +	//  shared test members
    +	// ------------------------------------------------------------------------
    +
    +	private static ActorSystem actorSystem = AkkaUtils.createDefaultActorSystem();
    +
    +	private static AkkaRpcService akkaRpcService = 
    +			new AkkaRpcService(actorSystem, new Timeout(10000, TimeUnit.MILLISECONDS));
    +
    +	@AfterClass
    +	public static void shutdown() {
    +		akkaRpcService.stopService();
    +		actorSystem.shutdown();
    +	}
    +
    +
    +	// ------------------------------------------------------------------------
    +	//  tests
    +	// ------------------------------------------------------------------------
    +
    +	@Test
    +	public void testScheduleWithNoDelay() throws Exception {
    +
    +		// to collect all the thread references
    +		final BlockingQueue<Thread> queue = new LinkedBlockingQueue<>();
    +
    +		TestEndpoint testEndpoint = new TestEndpoint(akkaRpcService, queue);
    +		TestGateway gateway = testEndpoint.getSelf();
    +
    +		// a bunch of gateway calls
    +		gateway.someCall();
    +		gateway.anotherCall();
    +		gateway.someCall();
    +
    +		// run something asynchronously
    +		testEndpoint.runAsync(new Runnable() {
    +			@Override
    +			public void run() {
    +				queue.add(Thread.currentThread());
    +			}
    +		});
    +	
    +		Future<String> result = testEndpoint.callAsync(new Callable<String>() {
    +			@Override
    +			public String call() throws Exception {
    +				return "test";
    --- End diff --
    
    I think we don't check that callAsync is executed in the same thread as the others.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2360#discussion_r74562511
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rpc/MainThreadExecutor.java ---
    @@ -52,4 +52,13 @@
     	 * @return Future of the callable result
     	 */
     	<V> Future<V> callAsync(Callable<V> callable, Timeout callTimeout);
    +
    +	/**
    +	 * Execute the runnable in the main thread of the underlying RPC endpoint, with
    +	 * a delay of the given number of milliseconds.
    +	 *
    +	 * @param runnable Runnable to be executed
    +	 * @param delay    The delay after which the runnable will be executed
    +	 */
    +	void scheduleRunAsync(Runnable runnable, long delay);
    --- End diff --
    
    We could, but then let's have either or. Else it would get too blown up in my opinion.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2360#discussion_r74568505
  
    --- Diff: flink-runtime/src/test/java/org/apache/flink/runtime/rpc/akka/AsyncCallsTest.java ---
    @@ -0,0 +1,178 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +
    +package org.apache.flink.runtime.rpc.akka;
    +
    +import akka.actor.ActorSystem;
    +import akka.util.Timeout;
    +
    +import org.apache.flink.runtime.akka.AkkaUtils;
    +import org.apache.flink.runtime.rpc.RpcEndpoint;
    +import org.apache.flink.runtime.rpc.RpcGateway;
    +import org.apache.flink.runtime.rpc.RpcMethod;
    +import org.apache.flink.runtime.rpc.RpcService;
    +
    +import org.junit.AfterClass;
    +import org.junit.Test;
    +
    +import scala.concurrent.Await;
    +import scala.concurrent.Future;
    +import scala.concurrent.duration.FiniteDuration;
    +
    +import java.util.concurrent.BlockingQueue;
    +import java.util.concurrent.Callable;
    +import java.util.concurrent.LinkedBlockingQueue;
    +import java.util.concurrent.TimeUnit;
    +
    +import static org.junit.Assert.*;
    +
    +public class AsyncCallsTest {
    +
    +	// ------------------------------------------------------------------------
    +	//  shared test members
    +	// ------------------------------------------------------------------------
    +
    +	private static ActorSystem actorSystem = AkkaUtils.createDefaultActorSystem();
    +
    +	private static AkkaRpcService akkaRpcService = 
    +			new AkkaRpcService(actorSystem, new Timeout(10000, TimeUnit.MILLISECONDS));
    +
    +	@AfterClass
    +	public static void shutdown() {
    +		akkaRpcService.stopService();
    +		actorSystem.shutdown();
    +	}
    +
    +
    +	// ------------------------------------------------------------------------
    +	//  tests
    +	// ------------------------------------------------------------------------
    +
    +	@Test
    +	public void testScheduleWithNoDelay() throws Exception {
    +
    +		// to collect all the thread references
    +		final BlockingQueue<Thread> queue = new LinkedBlockingQueue<>();
    +
    +		TestEndpoint testEndpoint = new TestEndpoint(akkaRpcService, queue);
    +		TestGateway gateway = testEndpoint.getSelf();
    +
    +		// a bunch of gateway calls
    +		gateway.someCall();
    +		gateway.anotherCall();
    +		gateway.someCall();
    +
    +		// run something asynchronously
    +		testEndpoint.runAsync(new Runnable() {
    +			@Override
    +			public void run() {
    +				queue.add(Thread.currentThread());
    +			}
    +		});
    +	
    +		Future<String> result = testEndpoint.callAsync(new Callable<String>() {
    +			@Override
    +			public String call() throws Exception {
    +				return "test";
    --- End diff --
    
    ah, true, this should queue its current thread. Will fix that.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the RpcEnd...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the issue:

    https://github.com/apache/flink/pull/2360
  
    Great addition @StephanEwen. +1 for merging.
    
    Things are getting in shape :-)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink issue #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the RpcEnd...

Posted by StephanEwen <gi...@git.apache.org>.
Github user StephanEwen commented on the issue:

    https://github.com/apache/flink/pull/2360
  
    Manually merged into the `flip-6` branch in f74f44bb56e50d1714e935df2980a6c8213faf89


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] flink pull request #2360: [FLINK-4384] [rpc] Add "scheduleRunAsync()" to the...

Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on a diff in the pull request:

    https://github.com/apache/flink/pull/2360#discussion_r74561951
  
    --- Diff: flink-runtime/src/main/java/org/apache/flink/runtime/rpc/MainThreadExecutor.java ---
    @@ -52,4 +52,13 @@
     	 * @return Future of the callable result
     	 */
     	<V> Future<V> callAsync(Callable<V> callable, Timeout callTimeout);
    +
    +	/**
    +	 * Execute the runnable in the main thread of the underlying RPC endpoint, with
    +	 * a delay of the given number of milliseconds.
    +	 *
    +	 * @param runnable Runnable to be executed
    +	 * @param delay    The delay after which the runnable will be executed
    +	 */
    +	void scheduleRunAsync(Runnable runnable, long delay);
    --- End diff --
    
    Shall we also add a `scheduleRunAsync` method where we allow to pass a `TimeUnit` argument for convenience?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---