Allow multiple JDKs in smoke test #13108

stefanvodita · 2024-02-15T17:28:10Z

Introduce --test-alternative-java tosmokeTestRelease.py, allowing the tests to run on multiple JDKs.

uschindler · 2024-02-15T17:30:38Z

Can we remove the java 17 one? Or at least map the java 17 command line parameter as additional alternative.

stefanvodita · 2024-02-15T17:37:28Z

We can, that would be cleaner. But this should work for testing 9.10, right?
Marking as draft, since I don’t think we would merge as-is.

uschindler · 2024-02-15T17:48:42Z

It's already in testing: https://jenkins.thetaphi.de/job/Lucene-Release-Tester-v2/1/console

uschindler · 2024-02-15T17:50:35Z

waaah it doesnt like the version :-(

+ python3 -u dev-tools/scripts/smokeTestRelease.py --test-java17 /home/jenkins/tools/java/64bit/hotspot/latest-jdk17 --test-alternative-java /home/jenkins/tools/java/64bit/hotspot/latest-jdk19 --test-alternative-java /home/jenkins/tools/java/64bit/hotspot/latest-jdk20 --test-alternative-java /home/jenkins/tools/java/64bit/hotspot/latest-jdk21 --tmp-dir /home/jenkins/workspace/Lucene-Release-Tester-v2/smoketmp https://dist.apache.org/repos/dist/dev/lucene/lucene-9.10.0-RC1-rev-695c0ac84508438302cd346a812cfa2fdc5a10df/ -Ptests.multiplier=1
Revision: 695c0ac84508438302cd346a812cfa2fdc5a10df
Java 11 JAVA_HOME=/home/jenkins/tools/java/64bit/hotspot/latest-jdk11
Java 17 JAVA_HOME=/home/jenkins/tools/java/64bit/hotspot/latest-jdk17
Java None JAVA_HOME=/home/jenkins/tools/java/64bit/hotspot/latest-jdk19
Java None JAVA_HOME=/home/jenkins/tools/java/64bit/hotspot/latest-jdk20
Java None JAVA_HOME=/home/jenkins/tools/java/64bit/hotspot/latest-jdk21
Traceback (most recent call last):
  File "/home/jenkins/workspace/Lucene-Release-Tester-v2/dev-tools/scripts/smokeTestRelease.py", line 1200, in <module>
    main()
  File "/home/jenkins/workspace/Lucene-Release-Tester-v2/dev-tools/scripts/smokeTestRelease.py", line 1128, in main
    raise RuntimeError('smokeTestRelease.py for %s.X is incompatible with a %s release.' % (scriptVersion, c.version))
RuntimeError: smokeTestRelease.py for 9.11.X is incompatible with a 9.10.0 release.
Build step 'Execute shell' marked build as failure
Finished: FAILURE

Can you comment out the version check in your branch?

uschindler · 2024-02-15T17:52:31Z

Thanks, will retrigger the job!

uschindler · 2024-02-15T17:53:18Z

Runs: https://jenkins.thetaphi.de/job/Lucene-Release-Tester-v2/3/console

I had a stupid build timeout, so retriggered.

uschindler · 2024-02-15T21:25:49Z

Smoke tester succeeded (see mailing list).

stefanvodita · 2024-02-15T22:57:49Z

That's great! If we extract the JDK version number as well, maybe we can commit this.
@uschindler - would you want the parameter for a specific JDK version removed on both main and branch_9x i.e. remove --test-java19 from main and remove --test-java17 from branch_9x?

uschindler · 2024-02-15T23:08:47Z

That's great! If we extract the JDK version number as well, maybe we can commit this.

Extracting the JDK version number is already implemented, because there's a check for Java 11 and Java 17 already.

@uschindler - would you want the parameter for a specific JDK version removed on both main and branch_9x i.e. remove --test-java19 from main and remove --test-java17 from branch_9x?

I would remove all of them. Only the base version should be checked (so in 9.x base version must be 11). The other versions passed as cmd line parameter should maybe just checked to be >= base version.

If you want backwards compatibility, you can keep the old cmd line parameters and just add the arguments from the specific parameters to the list of alternative versions. But as this is a developer tools only, I don't think we need any backwards compatibility.

ChrisHegarty · 2024-02-23T10:04:40Z

That's great! If we extract the JDK version number as well, maybe we can commit this.

Extracting the JDK version number is already implemented, because there's a check for Java 11 and Java 17 already.

@uschindler - would you want the parameter for a specific JDK version removed on both main and branch_9x i.e. remove --test-java19 from main and remove --test-java17 from branch_9x?

+1

I would remove all of them. Only the base version should be checked (so in 9.x base version must be 11). The other versions passed as cmd line parameter should maybe just checked to be >= base version.

+1

What's the latest status of this PR? It looks like it is in reasonable shape and just needs to address some comments, right?

stefanvodita · 2024-02-23T10:21:47Z

Right, there are only a couple things left to do. I'll wrap this one up during the weekend.

uschindler · 2024-02-23T10:56:16Z

Basically, to make the whole script better maintainable:

remove the hardcoded Java 17, Java 11 variable names.
have a central setting with the "baseline" java version.
When running the code it would just check that JAVA_HOME is exactly the baseline version and all other additionally added JVM versions should be >= baseVersion
Finally, the gradle tasks should not be executed with a changed JAVA_HOME but always with the compatible base one. Instead the RUNTIME_JAVA_HOME env var should be poulated and Gradle be running with the base version. This removes the problem of not being able to execute Gradle with Java 22. Maybe we can have a setting to choose between both variants.
Special case: Everything executed using without Gradle (like the Lucene Demo) should of course be executed directly directly executed on correct JVM (here the env vars may need to be different).

Allow multiple JDKs in smoke test Comment out version check Remove special handling for JDK 17 Consolidate base version into a single constant Handle version checks and logs

stefanvodita · 2024-02-25T21:57:54Z

@uschindler - I addressed your first three points. On the last two points I think I'm missing some of the context to understand the issues. I'll try to dig a bit to understand those better. Is this a "progress not perfection" situation, where we can treat the last 2 points in a separate PR or are they tightly bound with the rest of this work?

As far as testing, it's a bit annoying to do, but I think I've got the script working correctly. I used the latest available RC (9.9.0):

JAVA_HOME=/usr/lib/jvm/java-11-amazon-corretto python3 -u dev-tools/scripts/smokeTestRelease.py --test-alternative-java /usr/lib/jvm/java-20-amazon-corretto --test-alternative-java /usr/lib/jvm/java-21-amazon-corretto \
https://dist.apache.org/repos/dist/dev/lucene/lucene-9.9.0-RC1-rev-92a5e5b02e0e083126c4122f2b7a02426c21a037

There was an error about 8.11.3 back compatibility not being tested, but I think that has to do with it being released after 9.9.0, and not with the changes to the script.

uschindler

I just found a minor problem and suggest to change the messages a bit. Otherwise looks fine +1

We can open another issue about the problem with Java 22+ (Gradle does not start with Java 22, so you can't test it). What you do is the following (Jenkins does this for example): JAVA_HOME always points to base version, but RUNTIME_JAVA_HOME is used to instruct Gradle to run all compilation and tests with that JDK.

I agree, this can be a separate issue. The workaround at moment is to run the smoketester with RUNTIME_JAVA_HOME env var :-)

uschindler · 2024-02-25T22:29:16Z

dev-tools/scripts/smokeTestRelease.py

+    testDemo(java.run_java, isSrc, version, BASE_JAVA_VERSION)
+    if java.run_alt_javas:
+      for run_alt_java, alt_java_version in zip(java.run_alt_javas, java.alt_java_versions):
+        print("DELETEME")


Yes, DELETE ME ! :-)

Thanks for noticing, I forgot about this one.

uschindler · 2024-02-25T22:38:23Z

dev-tools/scripts/smokeTestRelease.py

+    print('Java %s JAVA_HOME=%s' % (actual_version, java_home))
+
+    if (is_base_version and BASE_JAVA_VERSION != actual_version) \
+    or int(BASE_JAVA_VERSION) > int(actual_version):


I think the messages are a bit misleading depending on context. Maybe differentiate between is_base_version and the alternate version mode:

if is_base_version == true then compare with exact same version and complain about that

if is_base_version == false print a message about actual_versionneed to be >= BASE_JAVA_VERSION

The >= is important, because I want to test OpenJ9 vs Hotspot.

Sure, I'll do separate messages for the two cases.

uschindler · 2024-02-25T22:42:30Z

There was an error about 8.11.3 back compatibility not being tested, but I think that has to do with it being released after 9.9.0, and not with the changes to the script.

If you run with Lucene 9.10.0 (just released last week), it should work.

stefanvodita · 2024-02-26T13:06:16Z

If you run with Lucene 9.10.0 (just released last week), it should work.

I think so, but the 9.10.0 RC artefacts were no longer available at https://dist.apache.org/repos/dist/dev/lucene/. Not sure why some versions are still there and some aren't. It was easier just to use the 9.9.0 RC that is still there.

dweiss · 2024-02-26T21:13:29Z

Just FYI - I've returned to trying to add a nightly smoketester workflow check. The main branch required some changes to the script as well, which sort of proves Rob's point that we need to run it regularly, otherwise it goes out of sync with the code quickly...

I'll try to forward-port @stefanvodita 's changes to main and will create a pr when I think it presents a reasonable state (still working my way through some python checks).

uschindler · 2024-02-26T21:18:49Z

I checked the script. Forward port should be more or less:

copy file to main branch
change BASE_VERSION constant in the script

The new script is no longer a search replace Desaster.

dweiss · 2024-02-26T21:21:53Z

I don't think it's going to be so easy - some things just didn't work for me with local artifacts. I'll follow up, perhaps tomorrow.

dweiss · 2024-02-27T07:27:29Z

I think we can merge this?

uschindler · 2024-02-27T09:13:39Z

I don't think it's going to be so easy - some things just didn't work for me with local artifacts. I'll follow up, perhaps tomorrow.

That was always a problem, I know. Therefore we had in the old ant build some target that created a "release folder" in the exact way how it is deployed on web server. After that it worked with file URLs: https://github.com/apache/lucene-solr/blob/1f6bd6cbf80513327054239b610076e3804133cd/build.xml#L420-L466

There is the Jenkins task still available: https://ci-builds.apache.org/job/Lucene/job/Lucene-Solr-SmokeRelease-8.11/, the 9.x and main ones were disabled and later removed after the gradle switch.

I'd like to add the Jenkins task back, Github is IMHO too expensive unless we run no tests.

+1 to merge this.

uschindler · 2024-02-27T11:34:58Z

Will you forward port the patch to main branch? I can help with that, have some freetime.

This will allow us to make everything Java 21 ready tomorrow, so we have one file less to search/replace version numbers.

Allow multiple JDKs in smoke test Remove special handling for JDK 17/19 Consolidate base version into a single constant Handle version checks and logs

stefanvodita · 2024-02-27T11:39:08Z

Here it is: #13139.
I wasn't sure about the difficulties @dweiss was encountering - I expect this to just work.

uschindler · 2024-02-27T12:01:20Z

Here it is: #13139. I wasn't sure about the difficulties @dweiss was encountering - I expect this to just work.

I think Dawid's problems were preexisting, not related to this PR and porting it forward.

uschindler · 2024-02-27T12:02:17Z

The other PR looks straight-forward to me. Have you compared the two files directly after merging? They should be mostly identical and only the BASE_VERSION line should be different.

stefanvodita · 2024-02-27T12:11:48Z

I have. The diff looks correct.

stefanvodita mentioned this pull request Feb 15, 2024

Improve smoketester JDK versions support #13107

Open

stefanvodita marked this pull request as draft February 15, 2024 17:37

uschindler linked an issue Feb 16, 2024 that may be closed by this pull request

Improve smoketester JDK versions support #13107

Open

ChrisHegarty mentioned this pull request Feb 23, 2024

Bump minimum required Java version to 21 #12753

Closed

Improve handling of different JDK versions

d49369c

Allow multiple JDKs in smoke test Comment out version check Remove special handling for JDK 17 Consolidate base version into a single constant Handle version checks and logs

stefanvodita force-pushed the smoke-tester branch from 87a083f to d49369c Compare February 25, 2024 21:56

stefanvodita marked this pull request as ready for review February 25, 2024 21:57

uschindler reviewed Feb 25, 2024

View reviewed changes

Update logging

b36614d

uschindler approved these changes Feb 26, 2024

View reviewed changes

dweiss approved these changes Feb 27, 2024

View reviewed changes

stefanvodita merged commit 510074c into apache:branch_9x Feb 27, 2024
2 checks passed

stefanvodita mentioned this pull request Feb 27, 2024

Allow multiple JDKs in smoke test on main #13139

Merged

iamsanjay mentioned this pull request Aug 30, 2024

SmokeTestRelease Added support for multiple JDK apache/solr#2685

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow multiple JDKs in smoke test #13108

Allow multiple JDKs in smoke test #13108

stefanvodita commented Feb 15, 2024

uschindler commented Feb 15, 2024

stefanvodita commented Feb 15, 2024

uschindler commented Feb 15, 2024

uschindler commented Feb 15, 2024 •

edited

Loading

uschindler commented Feb 15, 2024

uschindler commented Feb 15, 2024 •

edited

Loading

uschindler commented Feb 15, 2024

stefanvodita commented Feb 15, 2024

uschindler commented Feb 15, 2024

ChrisHegarty commented Feb 23, 2024

stefanvodita commented Feb 23, 2024

uschindler commented Feb 23, 2024 •

edited

Loading

stefanvodita commented Feb 25, 2024

uschindler left a comment

uschindler Feb 25, 2024

stefanvodita Feb 26, 2024

uschindler Feb 25, 2024

stefanvodita Feb 26, 2024

uschindler commented Feb 25, 2024

stefanvodita commented Feb 26, 2024

dweiss commented Feb 26, 2024

uschindler commented Feb 26, 2024

dweiss commented Feb 26, 2024

dweiss commented Feb 27, 2024

uschindler commented Feb 27, 2024 •

edited

Loading

uschindler commented Feb 27, 2024

stefanvodita commented Feb 27, 2024

uschindler commented Feb 27, 2024

uschindler commented Feb 27, 2024

stefanvodita commented Feb 27, 2024

Allow multiple JDKs in smoke test #13108

Allow multiple JDKs in smoke test #13108

Conversation

stefanvodita commented Feb 15, 2024

uschindler commented Feb 15, 2024

stefanvodita commented Feb 15, 2024

uschindler commented Feb 15, 2024

uschindler commented Feb 15, 2024 • edited Loading

uschindler commented Feb 15, 2024

uschindler commented Feb 15, 2024 • edited Loading

uschindler commented Feb 15, 2024

stefanvodita commented Feb 15, 2024

uschindler commented Feb 15, 2024

ChrisHegarty commented Feb 23, 2024

stefanvodita commented Feb 23, 2024

uschindler commented Feb 23, 2024 • edited Loading

stefanvodita commented Feb 25, 2024

uschindler left a comment

Choose a reason for hiding this comment

uschindler Feb 25, 2024

Choose a reason for hiding this comment

stefanvodita Feb 26, 2024

Choose a reason for hiding this comment

uschindler Feb 25, 2024

Choose a reason for hiding this comment

stefanvodita Feb 26, 2024

Choose a reason for hiding this comment

uschindler commented Feb 25, 2024

stefanvodita commented Feb 26, 2024

dweiss commented Feb 26, 2024

uschindler commented Feb 26, 2024

dweiss commented Feb 26, 2024

dweiss commented Feb 27, 2024

uschindler commented Feb 27, 2024 • edited Loading

uschindler commented Feb 27, 2024

stefanvodita commented Feb 27, 2024

uschindler commented Feb 27, 2024

uschindler commented Feb 27, 2024

stefanvodita commented Feb 27, 2024

uschindler commented Feb 15, 2024 •

edited

Loading

uschindler commented Feb 15, 2024 •

edited

Loading

uschindler commented Feb 23, 2024 •

edited

Loading

uschindler commented Feb 27, 2024 •

edited

Loading