Details

    • Type: Task Task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 4.0-ALPHA
    • Component/s: general/test
    • Labels:
      None
    • Lucene Fields:
      New

      Description

      This is currently not possible because of how the SPI loader works,
      we cannot simulate Lucene3x codec with PreFlexRWCodec.

      But we should still allow basic testing (even though we cannot test preflex).

      After hacking around the issue, I get interesting fails with this JRE so I think its worth it.

      1. LUCENE-3927.patch
        4 kB
        Robert Muir
      2. LUCENE-3927.patch
        3 kB
        Uwe Schindler
      3. LUCENE-3927.patch
        3 kB
        Robert Muir

        Activity

        Hide
        Robert Muir added a comment -

        Thanks Uwe!

        I really like this solution: if people have screwed up classpaths they get a warning. If they have a buggy IBM JRE they get a warning. but 'ant test' works.

        Show
        Robert Muir added a comment - Thanks Uwe! I really like this solution: if people have screwed up classpaths they get a warning. If they have a buggy IBM JRE they get a warning. but 'ant test' works.
        Hide
        Robert Muir added a comment -

        Uwe's patch but with the wording tweaked for non-IBM JRE

        I think this is ready to commit. Thanks Uwe!

        Show
        Robert Muir added a comment - Uwe's patch but with the wording tweaked for non-IBM JRE I think this is ready to commit. Thanks Uwe!
        Hide
        Robert Muir added a comment -
        if (!(Codec.forName("Lucene3x") instanceof PreFlexRWCodec)) {
                System.err.println("ERROR: Your VM's java.util.ServiceLoader implementation is buggy"+
                  " and does not respect classpath order, please report this to the vendor.");
        

        I think this could be confusing to people using Sun JVMs, but just with the wrong
        classpath? There is currently an assert for this:

        assert (codec instanceof PreFlexRWCodec) : "fix your classpath to have tests-framework.jar before lucene-core.jar";
        

        So I think we should just be careful about the warning to not confuse (maybe check Constants.JRE_VENDOR to
        make sure we send the correct message?)

        Show
        Robert Muir added a comment - if (!(Codec.forName( "Lucene3x" ) instanceof PreFlexRWCodec)) { System .err.println( "ERROR: Your VM's java.util.ServiceLoader implementation is buggy" + " and does not respect classpath order, please report this to the vendor." ); I think this could be confusing to people using Sun JVMs, but just with the wrong classpath? There is currently an assert for this: assert (codec instanceof PreFlexRWCodec) : "fix your classpath to have tests-framework.jar before lucene-core.jar" ; So I think we should just be careful about the warning to not confuse (maybe check Constants.JRE_VENDOR to make sure we send the correct message?)
        Hide
        Uwe Schindler added a comment -

        Here is a patch with a workaround for the JRE bug. It forcefully registers the preflex rw codec by reflection and prints warning (once per JVM).

        Show
        Uwe Schindler added a comment - Here is a patch with a workaround for the JRE bug. It forcefully registers the preflex rw codec by reflection and prints warning (once per JVM).
        Hide
        Robert Muir added a comment -

        The hack in the patch is not hardcoded at all IBM: it checks if the impersonation actually works too.

        Constants.JAVA_VENDOR.startsWith("IBM") && (!(Codec.forName("Lucene3x") instanceof PreFlexRWCodec));
        

        For other JREs we still throw an assertion error saying to fix your classpath.

        But again, we don't necessarily need to commit this patch. Again more interesting is the fact that,
        with the patch, if i run tests I get 'too many open files' from TestShardSearching, and I get test
        failures from Solr's CSV support...

        Show
        Robert Muir added a comment - The hack in the patch is not hardcoded at all IBM: it checks if the impersonation actually works too. Constants.JAVA_VENDOR.startsWith( "IBM" ) && (!(Codec.forName( "Lucene3x" ) instanceof PreFlexRWCodec)); For other JREs we still throw an assertion error saying to fix your classpath. But again, we don't necessarily need to commit this patch. Again more interesting is the fact that, with the patch, if i run tests I get 'too many open files' from TestShardSearching, and I get test failures from Solr's CSV support...
        Hide
        Shai Erera added a comment -

        I agree. But if we know in which version it has been fixed, we can at least put a comment to remove that hack. Or if have an alternative solution that does not require that fix, it's better.

        Because as I understand, that patch disables PreFlex for all IBM JREs right?

        Anyway, feel free to proceed with that patch because it at least enables running trunk tests with an IBM JRE.

        Show
        Shai Erera added a comment - I agree. But if we know in which version it has been fixed, we can at least put a comment to remove that hack. Or if have an alternative solution that does not require that fix, it's better. Because as I understand, that patch disables PreFlex for all IBM JREs right? Anyway, feel free to proceed with that patch because it at least enables running trunk tests with an IBM JRE.
        Hide
        Robert Muir added a comment -

        Well mainly I wanted to get past this (at least have a patch available!) because i see interesting
        test failures in trunk with the IBM JVM...

        Show
        Robert Muir added a comment - Well mainly I wanted to get past this (at least have a patch available!) because i see interesting test failures in trunk with the IBM JVM...
        Hide
        Shai Erera added a comment -

        Still can't find it. I tried with the latest JRE "IBM J9 VM (build 2.4, JRE 1.6.0 IBM J9 2.4 Windows 7 amd64-64 jvmwa6460sr10-20111207_96808 (JIT enabled, AOT enabled)" but TestImpersonation fails. So I guess the fix is not yet included in that version.

        I know it has been fixed (because as I said, I have an unofficial JAR with which I can run Trunk tests), so I will try newer releases when they're available.

        But if you can resolve it such that it works without that fix, that will be great.

        Show
        Shai Erera added a comment - Still can't find it. I tried with the latest JRE "IBM J9 VM (build 2.4, JRE 1.6.0 IBM J9 2.4 Windows 7 amd64-64 jvmwa6460sr10-20111207_96808 (JIT enabled, AOT enabled)" but TestImpersonation fails. So I guess the fix is not yet included in that version. I know it has been fixed (because as I said, I have an unofficial JAR with which I can run Trunk tests), so I will try newer releases when they're available. But if you can resolve it such that it works without that fix, that will be great.
        Hide
        Shai Erera added a comment -

        I reported a bug report on that and it was fixed. I cannot locate now my report (the link I have comes up empty) nor in which version they fixed it. But I have at hand a fixed (unofficial) jar and tests pass with it (without Robert's patch). I will try to find the version which contains the fix.

        Show
        Shai Erera added a comment - I reported a bug report on that and it was fixed. I cannot locate now my report (the link I have comes up empty) nor in which version they fixed it. But I have at hand a fixed (unofficial) jar and tests pass with it (without Robert's patch). I will try to find the version which contains the fix.
        Hide
        Dawid Weiss added a comment -

        t uses a HashMap instead of LinkedHashMap as lookup cache and that mixes the SPI classes up.

        This is sad. It's a simple bug to fix but will never be probably...

        Show
        Dawid Weiss added a comment - t uses a HashMap instead of LinkedHashMap as lookup cache and that mixes the SPI classes up. This is sad. It's a simple bug to fix but will never be probably...
        Hide
        Robert Muir added a comment -

        That would be a better solution than my hack (which swaps in SimpleText as punishment).

        Though such an override is pretty dangerous, anyone messing with this is likely to just not
        be able to read their index. Maybe we can make it private/package-private and setAccessible?

        Show
        Robert Muir added a comment - That would be a better solution than my hack (which swaps in SimpleText as punishment). Though such an override is pretty dangerous, anyone messing with this is likely to just not be able to read their index. Maybe we can make it private/package-private and setAccessible?
        Hide
        Uwe Schindler added a comment -

        This bug in IBM JDK goes back to Apache Harmony, where you can see how the SPI code is implemented. It uses a HashMap instead of LinkedHashMap as lookup cache and that mixes the SPI classes up. This is a bug, but unfortunately the API is not clear about that.

        Maybe we add a "override" option to SPILoader, so you can force NamedSPILoader to return a specific impl, ignoring the classpath? By this we can override the "Lucene3x" codec to be "new PreflexRWCodec()" programmatically.

        Show
        Uwe Schindler added a comment - This bug in IBM JDK goes back to Apache Harmony, where you can see how the SPI code is implemented. It uses a HashMap instead of LinkedHashMap as lookup cache and that mixes the SPI classes up. This is a bug, but unfortunately the API is not clear about that. Maybe we add a "override" option to SPILoader, so you can force NamedSPILoader to return a specific impl, ignoring the classpath? By this we can override the "Lucene3x" codec to be "new PreflexRWCodec()" programmatically.

          People

          • Assignee:
            Unassigned
            Reporter:
            Robert Muir
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development