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-5106 Deadlock on GlobalComponentRegistry when starting a cluster #3416

Closed
wants to merge 2 commits into from

Conversation

danberindei
Copy link
Member

https://issues.jboss.org/browse/ISPN-5106

Move the handling of the initial view to a separate thread.

@@ -565,10 +583,11 @@ public void forceAvailabilityMode(String cacheName, AvailabilityMode availabilit
public void handleViewChange(final ViewChangedEvent e) {
// Need to recover existing caches asynchronously (in case we just became the coordinator).
// Cannot use the async notification thread pool, by default it only has 1 thread.
asyncTransportExecutor.submit(new Runnable() {
viewHandlingCompletionService.submit(new Callable<Void>() {
Copy link
Member

Choose a reason for hiding this comment

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

it would be good to have a void submit(Runnable)

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, the way the SemaphoreCompletionService works, it would also need a return value with a Runnable, so it wouldn't make any difference. I'm considering making it an ExecutorService instead though, since I haven't really used the tracking of futures.

Copy link
Member

Choose a reason for hiding this comment

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

what I have in mind is the submit(Runnable) to invoke the submit(Runnable,T) and ignore the return value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but then I couldn't declare the variable as CompletionService...

Copy link
Member

Choose a reason for hiding this comment

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

didn't get it. the CompletionService has a submit(Runnable, T). Also, the field is declared as SemaphoreCompletionService

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking that having an extra null parameter is no better than return null. Or is there any other reason not to use Callable<Void>?

And yeah, I was thinking about the other submit() call for merge, with uses a local variable that's a CompletionService.

Anyway, I'm replacing the completion service with an executor service, so I'll have a void submit(Runnable) option. Running the tests now...

Copy link
Member

Choose a reason for hiding this comment

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

I have this rule in my head: I use Callable when a return value is expected, otherwise I use Runnable. And since the original code was a Runnable I would like to keep it :)

@danberindei
Copy link
Member Author

Updated

@danberindei
Copy link
Member Author

Updated again, including the SemaphoreExecutorService change.

@pruivo
Copy link
Member

pruivo commented Apr 30, 2015

testing and pushing...

@pruivo
Copy link
Member

pruivo commented Apr 30, 2015

integrated! thanks @danberindei !

@pruivo pruivo closed this Apr 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants