On Fri, Mar 20, 2020 at 2:30 AM Enrico Olivelli <eolive...@gmail.com> wrote: > > Yes > Do you have problems with a specific JDK?
Every one I've used with the `-release` flag set for the compiler. This is expected, because that's what the `-release` flag is supposed to do: reveal and prevent non-compliant APIs from being used. This affects OpenJDK11, 13, Eclipse (with compiler-compliance settings set correctly, which performs a similar function to `-release`). Although I haven't tried on any other JVMs, I would it expect it to outright fail on any Java (regardless of compiler compliance settings) that isn't derived from OpenJDK/OracleJDK. Such JVMs would likely not have the class present, because the class is not part of any Java spec and cannot be assumed to be present. Eventually, those internal classes will be encapsulated or removed, so that they are inaccessible to user code (see https://openjdk.java.net/jeps/260). I would strongly prefer to address it sooner, rather than later, so people aren't forced to upgrade ZooKeeper at the same time as they upgrade Java, in order to avoid runtime failures. Upgrading multiple components at the same time on a large cluster can be very disruptive to devops teams trying to isolate problems to a specific upgrade. Also, the use of this internal API blocks the ability to have automatic module names for ZooKeeper in the jar manifests (an intermediate step to full module use within ZooKeeper, which is probably a long way off, and far less important that cross-compilation strict compatibility, which is why I didn't mention it before now). > > Btw I had already approved that patch days ago. We need another sponsor. I > am ok with committing it to 3.6.1 Technically, you did not mark it as approved. It's still marked as "Changes requested". But, you did review it, and I appreciate that. I brought it up again mainly because I thought the 3.6.1 conversation would be a good chance to get another person's approval on it. :) > > Regarding CI: we have different jobs with different JDKs not just for byte > code and API compatibility but because internal behaviour of JDK is really > different from version to version, think about networking (they recently > rewritten all the blocking IO system), security (Kerberos stuff is the > most tricky)... That's fair. Those are good reasons to test on various platforms. I would probably approach it as a: "build once, test many", rather than have multiple build jobs on various platforms (since the byte code itself shouldn't be platform dependent). However, I'm not in a position to question the PMC's testing strategies. I trust the community's judgment. I only have casual observations... I haven't looked into it deeply. :) > > We have old jobs that we can drop and they are not running anymore. Yeah, I'm pursuing that clean up in the thread I started regarding build notifications to the separate notifications list, and hoping to clean some of that up, once I'm certain there's clear consensus, and no objections. :) > > > Enrico > > Il Ven 20 Mar 2020, 06:32 Ted Dunning <ted.dunn...@gmail.com> ha scritto: > > > What internal class? > > > > Where is it used? > > > > Why are others using JDK 13 not seeing the problem that you described? > > > > > > > > On Thu, Mar 19, 2020 at 8:58 PM Christopher <ctubb...@apache.org> wrote: > > > > > Apologies, but it's a bit hard for me to explain briefly. Here's an > > attempt > > > to be as brief as I can: > > > > > > Newer JDKs add a `-release` option to cross-compile with strict > > > compatibility enforced. When this flag is enabled, as my PR does > > > automatically with the profiles (when it detects building with a newer > > JDK > > > or in m2e Eclipse), the compiler identifies the use of a class that is > > not > > > strictly compatible with Java 8. My PR removes this not strictly > > compatible > > > class. The use of this class should be considered a bug because it will > > > cause a failure in some JDKs (those without the com.sun class)... a bug > > > that is caught by the strict compatibility checks of the `-release` flag > > in > > > newer JDKs. > > > > > > The problem with the class is that it only is available on some versions > > of > > > Java (those with com.sun internals). It is not only not strictly > > compatible > > > with Java 8 ("not guaranteed to work"), it is specifically "guaranteed to > > > not work" in future Java versions, because it is planned for explicit > > > removal. It already causes problems in current versions if you have an > > > application that uses modules and depends on ZooKeeper, because the > > > unsupported class is hidden away in an internal module. > > > > > > What you say about ensuring that releases are built with JDK8 is actually > > > not necessary any longer with the use of the `-release` flag. Newer JDKs > > > enforce cross-compilation compatibility better. This release flag option > > > eliminates the need to hold yourself back, doing release builds on an > > older > > > end-of-life JDK to retain compatibility. Now, you can build with a newer > > > JDK (taking advantage of newer Maven plugins, etc.), avoid complex uses > > of > > > Maven toolchains, the annoying `-bootclasspath`, or the > > > maven-enforcer-plugin's animal-sniffer rule. The cross-compilation strict > > > compatibility is now enforced as a built in feature of newer JDKs. > > > > > > Sorry I couldn't explain more briefly... it's a very tiny change, but > > > there's a lot of backstory to communicate why it matters. > > > > > > Incidentally, you can probably get rid of a few build jobs in Jenkins if > > > you were to use the maven-enforcer-plugin to *only* support builds on > > newer > > > JDKs (but still using the release flag to target Java 8, of course). This > > > is what Apache Accumulo does for the stuff that we want to be compatible > > > with Java 8: we require the build to use at least 11, but cross-compile > > to > > > 8, because some of the Maven plugins we use for quality control checks > > are > > > now starting to require Java 11. > > > > > > > > > On Thu, Mar 19, 2020 at 3:32 PM Enrico Olivelli <eolive...@gmail.com> > > > wrote: > > > > > > > Il Gio 19 Mar 2020, 20:11 Christopher <ctubb...@apache.org> ha > > scritto: > > > > > > > > > I would very much like my pull request in > > > > > https://issues.apache.org/jira/browse/ZOOKEEPER-3739 to be merged in > > > > 3.6.1 > > > > > and newer, because it helps ensure newer java APIs don't leak into > > pull > > > > > requests/patches when developing stuff to contribute using a newer > > JDK, > > > > by > > > > > leveraging the "release" flag of newer JDKs. However, in order to do > > > so, > > > > it > > > > > needs to remove the use of an internal com.sun API which is not > > > supported > > > > > in newer JDKs. I would like it included, because it makes it easier > > for > > > > me > > > > > (and others using newer JDKs) to contribute. > > > > > > > > > > > > > I usually use jdk13 for my applications and while I work on ZK code. > > > > What problems are you facing? > > > > > > > > It is important to remember to build with jdk8 while making releases. > > But > > > > for dev it is not needed. > > > > > > > > Maybe we can merge that patch on master but I don't feel it is so > > > important > > > > at the moment. (With jdk13) > > > > > > > > Enrico > > > > > > > > > > > > > The patch is currently pending additional reviewers. > > > > > > > > > > On Thu, Mar 19, 2020 at 11:17 AM Patrick Hunt <ph...@apache.org> > > > wrote: > > > > > > > > > > > Great idea, thanks Enrico! > > > > > > > > > > > > Patrick > > > > > > > > > > > > On Thu, Mar 19, 2020 at 4:58 AM Enrico Olivelli < > > eolive...@gmail.com > > > > > > > > > > wrote: > > > > > > > > > > > > > Hi folks, > > > > > > > after a couple of weeks after the release of 3.6.0 we are seeing > > a > > > > few > > > > > > > blocker issues related to the upgrade from 3.5. > > > > > > > > > > > > > > There is already a good list of issues already ported to 3.6 and > > a > > > > few > > > > > > > pending PRs that fix problems reported by users. > > > > > > > > > > > > > > You can find the list here, I have already did some clean up and > > > left > > > > > > > comments to "reporters" in order to remove a few issues that are > > > > > > > tagged 3.6.1 but they don't have an active contributor working on > > > it. > > > > > > > I have also created 3.6.2 release in order to ease moving those > > > > issues > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://issues.apache.org/jira/issues/?jql=project%20%3D%20ZOOKEEPER%20AND%20fixVersion%20%3D%203.6.1%20ORDER%20BY%20resolution%20DESC > > > > > > > > > > > > > > My proposal: > > > > > > > - commit pending blocker patches related to the upgrade > > > > > > > - start a release within a couple of weeks, in the beginning of > > > April > > > > > > > > > > > > > > I volunteer to do the release, this way I can finish the "new > > > release > > > > > > > procedure" based on the Maven Release Plugin > > > > > > > > > > > > > > Enrico > > > > > > > > > > > > > > > > > > > > > > > > > > >
