refuse to execute on broken corrupting jvms [LUCENE-4276]

[asfimport]

There are some jvms where we know lucene does not work at all and will just produce things like corrupt indexes.

We should detect this in a static block of Constants.java and refuse to run at all.

---

Migrated from LUCENE-4276 by Robert Muir (@rmuir), updated May 09 2016
Attachments: LUCENE-4276.patch (versions: 3)

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

first on the list is java 1.7.0 b147

I think this is most important, ive already seen people on the list
mention they are using it.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

We should detect this in a static block of Constants.java and refuse to run at all.

As I said on the mailing list, leaving it at that is too extreme.
At a minimum, there should be an override... -Dlucene.forceRun=true or something.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

what good can come from allowing someone to force lucene to run on 1.7.0?

This makes no sense to me.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

As said on the mailing list, disallowing specific JVM versions is fine, but only specific version ranges. Not open ranges, so something like disallowing JRockit completely with no version limit makes no sense.

For 1.6.x versions of Java we must also look into other options (you can get them using the HotspotBean like RamUsageEstimator does). So fail if version is 1.6.0_19 to 1.6.0_28 (I think, have to look up) with -XX:+AggressiveOpts (lots of stupid Solr and ElasticSearch users did this on their installation) or -XX+FooBarStringConcatOptimization.

If a bug is at release time not fixed in a JDK or there is a proposed fix in a hard stated future JDK version, we should set the upper limit of failing versions to the version currently on the market. So for the JRockit case, the version used here to test.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

One addition from mailing list:
The pattern list of vendors and versions should be in a resource file, not in code. So it is easy to edit on release.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

No it should not be Jrockit or any other JVM completely. But that exact version you tested has a problem!

I just want the list of exact versions that cause corruption. We add a one-line if statement to prevent the corruption, just like we would do in any other case.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

but only specific version ranges

Personally i'm not even arguing for ranges, i think thats dangerous.
Just a set of precise versions that are known to do this.

[asfimport]

Shai Erera (@shaie) (migrated from JIRA)

I didn't follow the email thread closely, but I don't want to see my JVM version on the list, if there's some use case that may lead to index corruption. What if I never step on that use case?

I'm just worried that such change would mean in the future preventing to run Lucene and a JVM that e.g. fails to properly detect the Codecs to use (i.e. ServiceLoader bug in IBM J9, which was fixed recently).

So as long as this is a list of specific JVM versions, that are known to cause index corruption, and not other runtime bugs (locale issues, ServiceLoader and such), and as long as this list is controlled by a file that can be edited by the app, I'm ok.

But if we're making all this trouble (file etc.), why fail to run? we can print a message to System.err – let the app choose.

Preventing Lucene from running on certain JVMs is too extreme IMO.

In general, I wish we'd be less extreme in our changes in Lucene code, but that's a matter for a separate discussion.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

what good can come from allowing someone to force lucene to run on 1.7.0? This makes no sense to me.

This is indicative of the larger problem in Lucene development recently - the tendency to prohibit something just because you can't think of the use-case off the top of your head.

So here's your use-case: one can normally get most versions of most major JVMs to correctly run applications - it's often just a matter of disabling certain optimisations for certain methods, or in extreme cases prohibiting JIT of the method altogether.

Hence, at a minimum, there should be a way for users to easily override this behavior w/o recompiling applications. We should inform, not enforce.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

This is indicative of the larger problem in Lucene development recently - the tendency to prohibit something just because you can't think of the use-case off the top of your head.

I don't think this is the case at all. I just want to prevent the creation of corrupt indexes.

Creating corrupt index is NOT A USE CASE

[asfimport]

Shai Erera (@shaie) (migrated from JIRA)

We should inform, not enforce.

+1 for that !

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

Creating corrupt index is NOT A USE CASE

Sigh.

-1
(my formal VETO for this issue unless it contains an easy user override)

[asfimport]

Shai Erera (@shaie) (migrated from JIRA)

Creating corrupt index is NOT A USE CASE

Depends :). If the corrupt index is a result of using DirectPF Codec, or using payloads etc., then it is a use case. If the corrupt index is a result of adding one field with one word, untokenized and all, then that's a more serious corruption, and how we handle/inform it should be discussed.

At any rate, I've seen apps preventing to run on old JVMs, but not future, potentially buggy, JVMs. What if we'll detect a corruption related to a certain Linux kernel version - would we prevent Lucene from running there too?

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

-1
(my formal VETO for this issue unless it contains an easy user override)

Thats ok: I will make the issue a blocker and we won't release until we resolve this.

I'm tired of seeing b147 in bug reports.

[asfimport]

Yonik Seeley (@yonik) (migrated from JIRA)

Thats ok: I will make the issue a blocker and we won't release until we resolve this.

Luckily, a release can't be vetoed.