Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISPN-1042 - Enable distributed and Map/Reduce task interruption/cancellation #1369

Closed
wants to merge 3 commits into from

Conversation

vblagoje
Copy link

@vblagoje vblagoje commented Oct 9, 2012

Master only. https://issues.jboss.org/browse/ISPN-1042

There are three separate commits in this PR.

  1. convert DistributedExecuteCommand

This commit converts DistributedExecuteCommand to CacheRpcCommand as we need it to be CacheRpcCommand to be able to cancel it's execution in remote VM

  1. split and update MapReduceFuture

We simplify MapReduceTask by encapsulating each task/command sent to a remote VM into TaskPart abstraction. MapTaskPart captures logic of map function execution and ReduceTaskPart of reduce. Such an abstraction is easier to cancel in 3) and overall code is greatly simplified as well.

  1. Actual command cancelation

This commit is debatable (and I welcome it); it is a matter of style how to introduce UUID to commands. I chose to have CacheRpcCommand extend CancellableCommand. Therefore each CacheRpcCommand is cancellable - although we never need to do actual cancelation of some commands. We could have only specific commands implement CancellableCommand, however we can only cancel CacheRpcCommand as other non CacheRpcCommand are actually wrapped into CacheRpcCommand for wire transport so it becomes rather messy to deal with cancellation that way. We could rename CancellableCommand to UniqueIdentifiableCommand because UUID of command might be used for something else as well. Anyways, lets discuss this one further. _Update_*: Discussions are done - we are going to use CancellableCommand which extends CacheRpcCommand; commands that are cancellable should extend CancellableCommand.

@tristantarrant
Copy link
Member

I would really like to be able to retrieve the list of running cancelable commands and cancel them via jmx. Maybe we should create a separate jira for this task

@vblagoje
Copy link
Author

@tristantarrant so all running tasks should register with this service? How would you do this? Separate JIRA for sure.

@Override
public Object perform(InvocationContext ctx) throws Throwable {
// grab CancellaltionService and cancel command
log.debug("Cancelling " + commandToCancel);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a big one, but this should be logged as trace as it happens on a per request basis. Debug is used for lifecycle events..
http://docs.jboss.org/process-guide/en/html/logging.html

@vblagoje
Copy link
Author

@mmarkus Mircea have another look

* @author Manik Surtani
* @since 5.2
*/
public class CancelCommandCommand extends BaseRpcCommand {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CancelCommandCommand? :) Why not just call it a CancelCommand?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah because we have CancellableCommand to denote commands that are
cancellable... and then CancelCommandCommand....well I can see your
point. Matter of taste I'd say but I'll change it.
On 12-10-23 10:49 AM, Manik Surtani wrote:

In core/src/main/java/org/infinispan/commands/CancelCommandCommand.java:

+import java.util.UUID;
+
+import org.infinispan.commands.remote.BaseRpcCommand;
+import org.infinispan.context.InvocationContext;
+import org.infinispan.util.logging.Log;
+import org.infinispan.util.logging.LogFactory;
+
+/**

  • * Command to cancel commands executing in remote VM
  • * @author Vladimir Blagojevic
  • * @author Manik Surtani
  • * @SInCE 5.2
  • */
    +public class CancelCommandCommand extends BaseRpcCommand {

|CancelCommandCommand|? :) Why not just call it a |CancelCommand|?


Reply to this email directly or view it on GitHub
https://github.com/infinispan/infinispan/pull/1369/files#r1916859.

@mmarkus
Copy link
Contributor

mmarkus commented Oct 30, 2012

thanks dude!

@mmarkus mmarkus closed this Oct 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants