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

Ban all usage of Future#cancel(true) #8494

Merged
merged 1 commit into from
Nov 18, 2014
Merged

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Nov 16, 2014

Interrupting a thread while blocking on an NIO Read / Write Operation
can cause a file to be closed due to the interrupts. This can have unpredictable
effects when files are open by index readers etc. we should prevent interruptions
across the board if possible.


/**
*/
public class FutureUtil {
Copy link
Member

Choose a reason for hiding this comment

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

All of our utility classes are plural, and you reference FutureUtils in the forbidden apis message also, I think this should be FutureUtils

@dakrone
Copy link
Member

dakrone commented Nov 17, 2014

LGTM, left two minor comments

@s1monw
Copy link
Contributor Author

s1monw commented Nov 17, 2014

@dakrone thanks for the review I push changes

@dakrone
Copy link
Member

dakrone commented Nov 17, 2014

LGTM!

@s1monw
Copy link
Contributor Author

s1monw commented Nov 18, 2014

@kimchy any comments on that

@kimchy
Copy link
Member

kimchy commented Nov 18, 2014

LGTM

Interrupting a thread while blocking on an NIO Read / Write Operation
can cause a file to be closed due to the interrupts. This can have unpredictable
effects when files are open by index readers etc. we should prevent interruptions
across the board if possible.

Closes elastic#8494
@s1monw s1monw merged commit 5c6fe25 into elastic:master Nov 18, 2014
@dakrone
Copy link
Member

dakrone commented Nov 19, 2014

@s1monw I think this didn't make it into 1.x? (I can't find it and FutureUtils doesn't exist on 1.x), can you backport it?

@s1monw
Copy link
Contributor Author

s1monw commented Nov 19, 2014

@dakrone yeah I wanted it to bake in a bit - will backport

s1monw added a commit that referenced this pull request Nov 19, 2014
Interrupting a thread while blocking on an NIO Read / Write Operation
can cause a file to be closed due to the interrupts. This can have unpredictable
effects when files are open by index readers etc. we should prevent interruptions
across the board if possible.

Closes #8494

Conflicts:
	src/main/java/org/elasticsearch/gateway/local/state/meta/LocalGatewayMetaState.java
@s1monw s1monw deleted the ban_future_cancel branch November 19, 2014 20:25
@clintongormley clintongormley added :Core/Infra/Core Core issues without another label and removed review labels Mar 19, 2015
@clintongormley clintongormley changed the title [CORE] Ban all usage of Future#cancel(true) Ban all usage of Future#cancel(true) Jun 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants