Skip to content

Clarify cancel contract in computation managers #2090

Open
@sylvlecl

Description

@sylvlecl
  • Do you want to request a feature or report a bug?

API clarification / bug.

  • What is the current behavior?

Since #2051, the completable futures returned by LocalComputationManager::execute have a specific behaviour on chaining:
most chaining results will interrupt the computation task

CompletableFuture<R> future = localComputationManager.execute(...)
       .thenApply(res -> { ...});

future.cancel(true);   // WILL interrupt the computation task

However, this is not the standard behaviour of completable futures, and this is not documented either in the ComputationManager javadoc, so the user cannot rely on it.

Indeed, in particular, the SlurmComputationManager does not have the same behaviour.

  • What is the expected behavior?

The user MUST be able to rely on a well defined behaviour.

The solutions can be:

  1. Not adding javadoc and maybe reverting Propagate Cancel to CompletableFutureTask (kill external process of localcomputationmanager) #2051 to be consistent. This implies to detect places where we fail to day to transfer cancels from user-facing futures to inner futures.
  2. Adding javadoc to computation manager interface, and adapt SlurmComputationManager to comply.
  3. Changing the execute return type with a new class that would have a more clear behaviour than completable future. That class could provide more information about the task (like an ID, for example), and be convertible to completable future (possibly with the same chaining behaviour as in Propagate Cancel to CompletableFutureTask (kill external process of localcomputationmanager) #2051 )

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions