aws / aws-swf-flow-library

AWS Simple Workflow Flow framework library
Apache License 2.0
61 stars 54 forks source link

Allow activity context to be inherited #14

Closed drautb closed 5 years ago

drautb commented 7 years ago

When a long-running activity needs to spawn child threads to do a set of tasks, those threads lose the ability to send SWF heartbeat messages due to the ActivityExecutionContext not being available in their local thread storage.

By using an InheritableThreadLocal, rather than just a ThreadLocal, child threads would have access to this context by default, enabling them to heartbeat as needed.

Custom code could be added to manually pass this contextual information to child threads, but this seems to be a more appropriate point of control.

It could be argued that multiple threads in a single activity like this should be broken up into individual activities. I agree with this, but such architectural changes require far more resources.

drautb commented 7 years ago

@manikandanrs is a pull request the appropriate channel for contributing to this project?

andrahol commented 6 years ago

Thank you for your suggestion @drautb. We investigated this issue, and came this conclusion: An ActivityExecutionContext represents the context for an activity. What an activity does in an application, that's up to the developer. With the principle in mind that every activity should start on a distinct thread, we are using the ThreadLocal concept. We don't want to hardcode any more restriction. In your case child threads still mean the same activity, but it could be different for other developers. As an SDK, we should support the most number of use cases. Everyone can code their own solutions, in your case passing the ActivityExecutionContext to your child thread instances, so they can heartbeat for the activity. Here is an example class for doing it:

class ActivityExecutionAware implements Runnable {

    private final Runnable delegate;

    private final ActivityExecutionContext context;

    public ActivityExecutionAware(Runnable delegate) {
        this.delegate = delegate;
        this.context = CurrentActivityExecutionContext.get();
    }

    @Override
    public void run() {
        CurrentActivityExecutionContext.set(context);
        delegate.run();
    }
}
drautb commented 6 years ago

@andrahol thanks for the feedback. I was a bit confused by this statement:

In your case child threads still mean the same activity, but it could be different for other developers.

I didn't know it was possible for activity to start a child thread that executes a different activity, can you give an example of how that could be done?