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

Core: Don't forcefully release IW lock (this can cause corruption) #8892

Closed
wants to merge 2 commits into from

Conversation

mikemccand
Copy link
Contributor

This fixes the test failure at http://build-us-00.elasticsearch.org/job/es_core_1x_strong/1652/

I already fixed this in master with #8588 but I think we should back-port even though it's a behavior change: it's really dangerous to forcefully remove IW's lock on init (it can quickly cause index corruption as it did in this test failure, caught by our check index on close).

If the index is in fact locked it means somehow another ES instance is still open/writing to that store directory and it's better to get an exc saying that.

@@ -1432,11 +1432,6 @@ private static boolean isMergedSegment(AtomicReader reader) {

private IndexWriter createWriter() throws IOException {
try {
// release locks when started
Copy link
Contributor

Choose a reason for hiding this comment

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

huge +1

@s1monw
Copy link
Contributor

s1monw commented Dec 11, 2014

+1 I left a comment about the test fixes

mikemccand added a commit that referenced this pull request Dec 11, 2014
It's dangerous to release this lock because it means another ES
somehow still has an IndexWriter open on the directory, which can
lead to corruption.

Instead, we don't release the lock, and the user will see an exception
on init.

This was already fixed on master with #8588; I'm backporting to 1.x
here.

Closes #8892
mikemccand added a commit that referenced this pull request Dec 11, 2014
It's dangerous to release this lock because it means another ES
somehow still has an IndexWriter open on the directory, which can
lead to corruption.

Instead, we don't release the lock, and the user will see an exception
on init.

This was already fixed on master with #8588; I'm backporting to 1.x
here.

Closes #8892
mikemccand added a commit that referenced this pull request Dec 11, 2014
It's dangerous to release this lock because it means another ES
somehow still has an IndexWriter open on the directory, which can
lead to corruption.

Instead, we don't release the lock, and the user will see an exception
on init.

This was already fixed on master with #8588; I'm backporting to 1.x
here.

Closes #8892
@mikemccand mikemccand closed this Dec 11, 2014
@clintongormley clintongormley changed the title Core: don't forcefully release IW lock (this can cause corruption) Core: Don't forcefully release IW lock (this can cause corruption) Dec 16, 2014
@clintongormley clintongormley added :Core/Infra/Core Core issues without another label and removed review labels Mar 19, 2015
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
It's dangerous to release this lock because it means another ES
somehow still has an IndexWriter open on the directory, which can
lead to corruption.

Instead, we don't release the lock, and the user will see an exception
on init.

This was already fixed on master with elastic#8588; I'm backporting to 1.x
here.

Closes elastic#8892
mute pushed a commit to mute/elasticsearch that referenced this pull request Jul 29, 2015
It's dangerous to release this lock because it means another ES
somehow still has an IndexWriter open on the directory, which can
lead to corruption.

Instead, we don't release the lock, and the user will see an exception
on init.

This was already fixed on master with elastic#8588; I'm backporting to 1.x
here.

Closes elastic#8892
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants