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

Threads must wait to be ready #10

Closed
wants to merge 4 commits into from
Closed

Threads must wait to be ready #10

wants to merge 4 commits into from

Conversation

olamy
Copy link
Member

@olamy olamy commented Feb 21, 2017

fix for #8

@olamy
Copy link
Member Author

olamy commented Feb 21, 2017

@sbordet please have a look

@@ -437,6 +440,8 @@ public void run( int transactionNumber )
}
}

List<Future<Void>> futures = this.runnersExecutorService.invokeAll( callables );
Copy link
Member

Choose a reason for hiding this comment

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

Not good, invokeAll() blocks until all tasks are complete.

@@ -139,25 +144,28 @@ private void handleResource( Resource resource )
// it's a group so we can request in parallel but wait all responses before next step
ExecutorService executorService = Executors.newWorkStealingPool();
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why we want to create a new ExecutorService here ? Can't we just use the existing one ?

@olamy olamy closed this Feb 24, 2017
@olamy olamy deleted the bugfix/8 branch February 24, 2017 07:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants