Wildfly jdk17 #2028


  WILDFLY_VERSION_FROM_JBOSS_AS=`awk '/wildfly-parent/ { while(!/<version>/) {getline;} print; }' ${WORKSPACE}/jboss-as/pom.xml | cut -d \< -f 2|cut -d \> -f 2`
  echo "AS version is ${WILDFLY_VERSION_FROM_JBOSS_AS}"
-  JBOSS_HOME=${WORKSPACE}/jboss-as/build/target/wildfly-${WILDFLY_VERSION_FROM_JBOSS_AS}
-  export JBOSS_HOME=`echo  $JBOSS_HOME`
+  export JBOSS_HOME=${WORKSPACE}/jboss-as/build/target/wildfly-${WILDFLY_VERSION_FROM_JBOSS_AS}


Am I right that this is tidy up?

I didn't understand the point of it, so yes I'd say it is.

I think it looks better as you propose it, but I think the tidy up should go in a separate commit

It's mad code, but I will reinstate the original!

tomjenkinson · 2022-08-02T10:55:02Z

scripts/hudson/narayana.sh


  # building WildFly
  [ "$_jdk" -lt 17 ] && export MAVEN_OPTS="-XX:MaxMetaspaceSize=512m -XX:+UseConcMarkSweepGC $MAVEN_OPTS"
  [ "$_jdk" -ge 17 ] && export MAVEN_OPTS="-XX:MaxMetaspaceSize=512m $MAVEN_OPTS"
-  JAVA_OPTS="-Xms1303m -Xmx1303m -XX:MaxMetaspaceSize=512m $JAVA_OPTS" ./build.sh clean install -B -DskipTests -Dts.smoke=false $IPV6_OPTS -Dversion.org.jboss.narayana=${NARAYANA_CURRENT_VERSION} "$@"
-  [ $? -eq 0 ] || fatal "AS build failed"
+  JAVA_OPTS="-Xms1303m -Xmx1303m -XX:MaxMetaspaceSize=512m $JAVA_OPTS" ./build.sh clean install -B -DskipTests -Dts.smoke=false $IPV6_OPTS -Dversion.org.jboss.narayana=${NARAYANA_CURRENT_VERSION} "$@" || fatal "AS build failed"


Is this tidy up?

It is a simplification to reduce the cognitive overload, if that's clean up then yes.

We use the pattern [ $? -eq 0 ] || fatal many places in https://github.com/jbosstm/narayana/blob/master/scripts/hudson/narayana.sh. If a change is necessary, I think they should all be proposed to be changed at once but I would suggest to discuss the change on Zulip and give time to receive feedback.

I will remove them. But discussing on Zulip is overkill, micro-managing software updates seems wrong and will turn people off from reading our chat so let's limit such discussions to the PR's.

But, at risk of repeating myself, anything we can do to simplify this behemoth of a script should be encouraged, so when I find the time I will raise a PR to simplify it - we have all been guilty of misunderstanding how this script works.

tomjenkinson · 2022-08-02T10:56:46Z

scripts/hudson/narayana.sh

@@ -468,15 +477,14 @@ function download_as {
  zip=$(ls wildfly-*-SNAPSHOT.zip) # example the current latest is wildfly-preview-27.0.0.Beta1-SNAPSHOT.zip

  export JBOSS_HOME=${JBOSS_HOME:-"${PWD}/${zip%.*}"}
+  echo "JBOSS_HOME=$JBOSS_HOME"


I think this is tidy up too, perhaps to state earlier in the build what JBOSS_HOME is set to? That seems a good thing to do in a tidy up commit - were the "" required?

The bash script runs with set -x so it just adds noise.

I am not sure if I understand what you are responding to, I could imagine your response being why this type of line would be removed (rather than moved).

If you do decide not to include this line here, then you might also want to keep your removal of the echo JBOSS_HOME=$JBOSS_HOME but if you do that should be in a commit that tidies up narayana.sh rather than connecting it to WildFly

The echo'ing of the variable is miles away from the place it was set so it just looks like a random statement - it is this kind of thing that adds to the complexity. But, even though I hear what you are saying, not being able to simplify things when it is safe to do so without the overhead of process means they will never get cleaned up because there are always more urgent things to be done.

Since I lingered here, I recall that the actual reason that prompted me to move it is because the echo statement is after JBOSS home gets used and the script can exit before the echo statement so the echo statement would not serve its intended purpose.

I will reinstate the debug statement to it original location.

I am not sure if I understand what you are responding to, I could imagine your response being why this type of line would be removed (rather than moved).

The echo JBOSS_HOME=$JBOSS_HOME statement is superfluous because the script runs with set -x which already logs the value and, moreover, it logs it at the point it was set.

tomjenkinson · 2022-08-02T10:58:05Z

scripts/hudson/narayana.sh

@@ -985,10 +993,10 @@ export ANT_OPTS="$ANT_OPTS $IPV6_OPTS"

 # run the job

-[ $NARAYANA_BUILD = 1 ] && build_narayana "$@"
-[ $AS_CLONE = 1 ] && clone_as "$@"
+[ $NARAYANA_BUILD = 1 ] && build_narayana "$@" # NB we should do this after building the AS since it takes longer


I don't understand the comment but I don't think there is a functional change here yet but what I would say is building Narayana after the AS would be wrong because then the AS would not be able to see (and include) the built Narayana

yes you are correct - I will remove the comment

jbosstm-bot · 2022-08-02T11:00:34Z

CORE profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk17.latest,label=linux/1047/

jbosstm-bot · 2022-08-02T11:28:46Z

CORE profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk11.latest,label=linux/1047/

jbosstm-bot · 2022-08-02T11:45:03Z

XTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk17.latest,label=linux/1047/

tomjenkinson · 2022-08-02T11:47:22Z

scripts/hudson/narayana.sh

-[ $AS_CLONE = 1 ] && clone_as "$@"
+[ $NARAYANA_BUILD = 1 ] && build_narayana "$@" # NB we should do this after building the AS since it takes longer
+[ $AS_CLONE = 1 ] && AS_BUILD=1 # keep it simple and assume that we are cloning the AS because we want to build it
+[ $AS_DOWNLOAD = 1 ] && AS_BUILD=1 # keep it simple and build it from scratch - we can revisit it later if required


I think this is confusing, I think you could just "re-program" the PROFILE for those that have AS_DOWLOAD=1 to be AS_CLONE=1 and AS_BUILD=1 instead

It seems it's only the LRA PROFILE: https://github.com/jbosstm/narayana/blob/master/scripts/hudson/narayana.sh#L192

If you change
export AS_BUILD=0 AS_CLONE=0 AS_DOWNLOAD=1 AS_TESTS=0 NARAYANA_BUILD=0 NARAYANA_TESTS=0 XTS_AS_TESTS=0 XTS_TESTS=0 TXF_TESTS=0 txbridge=0
to:
export AS_BUILD=1 AS_CLONE=1 AS_DOWNLOAD=0 AS_TESTS=0 NARAYANA_BUILD=0 NARAYANA_TESTS=0 XTS_AS_TESTS=0 XTS_TESTS=0 TXF_TESTS=0 txbridge=0 I think it would accomplish what you are trying to do with this change

Yes, I noticed that, the last push did something similar to what you suggested but I will take another look ...

tomjenkinson · 2022-08-02T11:52:06Z

scripts/hudson/narayana.sh

+  # download the latest wildfly nighly build (which we know supports Java 17)
+  # re-enable downloading main when narayana migrates to jakarta - see JBTM-3588
+  #AS_LOCATION=${AS_LOCATION:-https://ci.wildfly.org/guestAuth/repository/downloadAll/WF_Nightly/.lastSuccessful/artifacts.zip}
+  AS_LOCATION="https://ci.wildfly.org/guestAuth/repository/downloadAll/WF_26xNightlyJobs_Nightly/.lastSuccessful/artifacts.zip"


Given you aren't intending to use AS_LOCATION="https://ci.wildfly.org/guestAuth/repository/downloadAll/WF_26xNightlyJobs_Nightly/.lastSuccessful/artifacts.zip" from AS_LOCATION=${AS_LOCATION:-https://ci.wildfly.org/guestAuth/repository/downloadAll/WF_Nightly/.lastSuccessful/artifacts.zip} is not necessary. You could still add the comment you proposed

Yes I spotted that one too and my last push corrected it.

jbosstm-bot · 2022-08-02T11:52:21Z

XTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk11.latest,label=linux/1047/

scripts/hudson/narayana.sh

tomjenkinson · 2022-08-02T11:57:57Z

scripts/hudson/narayana.sh

@@ -189,7 +189,7 @@ function init_test_options {
    elif [[ $PROFILE == "LRA" ]]; then
        if [[ ! $PULL_DESCRIPTION_BODY == *!LRA* ]]; then
          comment_on_pull "Started testing this pull request with LRA profile: $BUILD_URL"
-          export AS_BUILD=0 AS_CLONE=0 AS_DOWNLOAD=1 AS_TESTS=0 NARAYANA_BUILD=0 NARAYANA_TESTS=0 XTS_AS_TESTS=0 XTS_TESTS=0 TXF_TESTS=0 txbridge=0
+          export AS_BUILD=0 AS_CLONE=0 AS_DOWNLOAD=1 AS_TESTS=0 NARAYANA_BUILD=1 NARAYANA_TESTS=0 XTS_AS_TESTS=0 XTS_TESTS=0 TXF_TESTS=0 txbridge=0


If you want to use a cloned and build version of the AS you could change AS_BUILD=0 AS_CLONE=0 AS_DOWNLOAD=1 to AS_BUILD=1 AS_CLONE=1 AS_DOWNLOAD=0

Okay but my script changes makes the AS_CLONE variable redundant since I always build WildFly if the build needs to test against WildFly.

I think reconfiguring the PROFILE here would be clearer

I'm not aware of any scenario where one would want to clone the AS and not build it.
But if you require it then I will look into re-instating AS_CLONE.

What I am suggesting at this particular line is to change "AS_BUILD=0 AS_CLONE=0 AS_DOWNLOAD=1" to "AS_BUILD=1 AS_CLONE=1 AS_DOWNLOAD=0" (and to keep your change "NARAYANA_BUILD=1")

jbosstm-bot · 2022-08-02T11:59:06Z

Started testing this pull request with AS_TESTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk17.latest,label=linux/1051/

jbosstm-bot · 2022-08-02T11:59:06Z

Started testing this pull request with XTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk17.latest,label=linux/1051/

jbosstm-bot · 2022-08-02T11:59:06Z

Started testing this pull request with CORE profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk17.latest,label=linux/1051/

jbosstm-bot · 2022-08-02T11:59:06Z

Started testing this pull request with AS_TESTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk11.latest,label=linux/1051/

jbosstm-bot · 2022-08-02T18:15:18Z

XTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk11.latest,label=linux/1058/

tomjenkinson · 2022-08-02T18:23:11Z

@mmusgrov do you think the XTS profile failure on JDK17 is similar (different test though) to https://issues.redhat.com/browse/JBTM-3667?

jbosstm-bot · 2022-08-02T20:29:18Z

AS_TESTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk17.latest,label=linux/1058/

jbosstm-bot · 2022-08-02T20:38:45Z

AS_TESTS profile tests failed (http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk11.latest,label=linux/1058/): AS tests failed

jbosstm-bot · 2022-08-03T10:54:10Z

Started testing this pull request with LRA profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:54:10Z

Started testing this pull request with AS_TESTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:54:10Z

Started testing this pull request with RTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=RTS,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:54:10Z

Started testing this pull request with AS_TESTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk11.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:54:10Z

Started testing this pull request with XTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:56:03Z

Started testing this pull request with CORE profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk11.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:56:03Z

Started testing this pull request with LRA profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=jdk11.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:56:03Z

Started testing this pull request with RTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=RTS,jdk=jdk11.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:56:03Z

Started testing this pull request with XTS profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk11.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T10:56:13Z

Started testing this pull request with CORE profile: http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk17.latest,label=linux/1059/

jmfinelli

LGTM! Thank you very much Michael +1

jmfinelli · 2022-08-03T11:01:42Z

scripts/hudson/narayana.sh

+# clone WildFly main
+# note that this version does not use 5_BRANCH
+# if we require changes to WildFly sources then we should either
+# a) raise a WildFly PR, or


Just to be picky, the or could be removed

...but feel free to ignore this comment ;-)

suitably ignored ;-)

I'll consider it if I ever get time to clean the whole script

jbosstm-bot · 2022-08-03T11:35:37Z

RTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=RTS,jdk=jdk11.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T11:37:03Z

RTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=RTS,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T11:38:40Z

LRA profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=jdk11.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T11:40:17Z

LRA profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=LRA,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T12:01:17Z

CORE profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T12:01:58Z

CORE profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=CORE,jdk=jdk11.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T12:44:28Z

XTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T12:44:53Z

XTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=XTS,jdk=jdk11.latest,label=linux/1059/

mmusgrov · 2022-08-03T13:14:16Z

@mmusgrov do you think the XTS profile failure on JDK17 is similar (different test though) to https://issues.redhat.com/browse/JBTM-3667?

We do periodically see the XTS test failure in the XTS crash recovery tests, see [1] for example, and is unlikely to be related to this particular PR, more likely a test artefact (remark it passed on the last run with both JDK's which lends weight to it being a transient failure).

Since the purpose of the PR is to release an alpha, I don't think that this transient failure needs to block the PR and, indeed, the problem with this kind of heisenbug related to test timing issues is that they are notoriously difficult to track down. We should monitor the regular CI job and if it starts happening more regularly we'd need to resolve it.

The AS-TESTS failure is unrelated and the failure is due to a timeout breach:
Integration - MicroProfile (org.wildfly.test.integration.microprofile.reactive.messaging.kafka.serializer.ReactiveMessagingKafkaSerializerTestCase).
The test passed on the last run of the axis.

[1] https://issues.redhat.com/browse/JBTM-2038?jql=project%20%3D%20JBTM%20AND%20component%20%3D%20XTS%20and%20text%20~%20%22was%20not%20killed%20by%20Byteman%22%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC

jbosstm-bot · 2022-08-03T15:47:17Z

AS_TESTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk17.latest,label=linux/1059/

jbosstm-bot · 2022-08-03T15:55:06Z

AS_TESTS profile tests passed - Job complete http://narayanaci1.eng.hst.ams2.redhat.com/job/btny-pulls-narayana/PROFILE=AS_TESTS,jdk=jdk11.latest,label=linux/1059/

tomjenkinson reviewed Aug 2, 2022

View reviewed changes

scripts/hudson/narayana.sh Show resolved Hide resolved

tomjenkinson reviewed Aug 2, 2022

View reviewed changes

wildfly-jdk17 use 27.0.0.Alpha1 for WildFly testing

3e67153

mmusgrov force-pushed the wildfly-jdk17 branch from fead1a8 to 3e67153 Compare August 3, 2022 10:48

jmfinelli approved these changes Aug 3, 2022

View reviewed changes

mmusgrov merged commit 2f453c6 into jbosstm:master Aug 3, 2022

mmusgrov mentioned this pull request Aug 3, 2022

JBTM-3595 jakarta transformed LRA artifacts #2025

Merged

Wildfly jdk17 #2028

Wildfly jdk17 #2028

Conversation

mmusgrov commented Aug 2, 2022 • edited

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmusgrov Aug 3, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbosstm-bot commented Aug 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

tomjenkinson commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 2, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jmfinelli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

jbosstm-bot commented Aug 3, 2022

mmusgrov commented Aug 2, 2022 •

edited

mmusgrov Aug 3, 2022 •

edited