You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by "Arun M. Krishnakumar" <ar...@gmail.com> on 2016/06/08 18:23:25 UTC
Re: Java 8 support for Bookkeeper
(Rebooting an old thread about running binaries compiled for jdk1.7 while
using a jdk1.8 compilation environment and runtime)
Just to be safe, I used maven's animal-sniffer plugin (
http://www.mojohaus.org/animal-sniffer/animal-sniffer-maven-plugin/) that's
recommended by maven and built bookkeeper. It found one issue in
ConcurrentHashMap.keySet whose definition changes from 1.7 to 1.8.
The patch that fixes this follows. Could we add this plugin to the codebase
so that we can safely use an older version of jdk ?
diff --git
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
index 12ad303..8ff1e98 100644
---
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
+++
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.nio.channels.ClosedChannelException;
import java.util.ArrayDeque;
import java.util.Collections;
+import java.util.Enumeration;
import java.util.Queue;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@@ -814,7 +815,9 @@ public class PerChannelBookieClient extends
SimpleChannelHandler implements Chan
// in case they get a write failure on the socket. The one who
// successfully removes the key from the map is the one
responsible for
// calling the application callback.
- for (CompletionKey key : completionObjects.keySet()) {
+ Enumeration<CompletionKey> keys = completionObjects.keys();
+ while (keys.hasMoreElements()) {
+ CompletionKey key = keys.nextElement();
switch (key.operationType) {
case ADD_ENTRY:
errorOutAddKey(key, rc);
diff --git a/pom.xml b/pom.xml
index 496a687..fec12f6 100644
--- a/pom.xml
+++ b/pom.xml
@@ -197,6 +197,31 @@
</configuration>
</plugin>
<plugin>
+ <groupId>org.codehaus.mojo</groupId>
+ <artifactId>animal-sniffer-maven-plugin</artifactId>
+ <version>1.15</version>
+ <executions>
+ <execution>
+ <id>check-java-version</id>
+ <phase>verify</phase>
+ <goals>
+ <goal>check</goal>
+ </goals>
+ <configuration>
+ <ignores>
+ <ignore>com.sun.management.OperatingSystemMXBean</ignore>
+
<ignore>com.sun.management.UnixOperatingSystemMXBean</ignore>
+ </ignores>
+ <signature>
+ <groupId>org.codehaus.mojo.signature</groupId>
+ <artifactId>java17</artifactId>
+ <version>1.0</version>
+ </signature>
+ </configuration>
+ </execution>
+ </executions>
+ </plugin>
+ <plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.19.1</version>
Thanks,
Arun
On Wed, Apr 20, 2016 at 1:32 PM, Enrico Olivelli <eo...@gmail.com>
wrote:
> Don't worry. Java code compiled for Java7 runs without changes for java8
> usually.
> That instruction tells that the java byte code is to be compatible with
> Java7 virtual jams and newer versions.
>
> Cheers
> Enrico
>
> Il Mer 20 Apr 2016 20:25 Arun M. Krishnakumar <ar...@gmail.com> ha
> scritto:
>
> > Thanks Enrico for the feedback.
> >
> > I am new to Maven so am trying to understand this better. I see that the
> > pom.xml in the base bookkeeper directory has the following lines:
> >
> > <plugin>
> > <artifactId>maven-compiler-plugin</artifactId>
> > <version>3.0</version>
> > <configuration>
> > * <source>1.7</source>*
> > * <target>1.7</target>*
> > <compilerArguments>
> > <Werror />
> > <Xlint:deprecation />
> > <Xlint:unchecked />
> > </compilerArguments>
> > </configuration>
> > </plugin>
> >
> > So it looks like the source can make use of only Java 7 features
> currently.
> >
> > Also, when I do the following:
> > 1. Build using "mvn clean install -DskipTests" from the bookkeeper
> > directory
> > 2. Get the Bookie.class from the "bookkeeper-server-4.4.0-SNAPSHOT.jar"
> > file
> > 3. javap -verbose Bookie.class | grep -i "Major"
> >
> > I get the value 51 which indicates that the class has been built for Java
> > 7. So my understanding of your mail is that the Java 7 binaries generated
> > run well on Java 8 JVM. Is that accurate ?
> >
> > My worry was about the incompatibilities that have been mentioned in the
> > compatibility docs.
> >
> > Thanks,
> > Arun
> >
> >
> >
> >
> > On Tue, Apr 19, 2016 at 11:35 PM, Enrico Olivelli <eo...@gmail.com>
> > wrote:
> >
> > > Hi Arun,
> > > BookKeeper 4.3.2 is compiled for java6. New upcoming release will
> > leverage
> > > Java7 features like autocloseable.
> > > There is no need to change the pom.xml in order to make it compatible
> > with
> > > java8.
> > > I'm running it on java8 since ever.
> > >
> > > --Enrico
> > >
> > > Il Mer 20 Apr 2016 05:16 Arun M. Krishnakumar <ar...@gmail.com> ha
> > > scritto:
> > >
> > > > Hi,
> > > >
> > > > We are moving to Java 8 at our company since Java 7 has reached its
> end
> > > of
> > > > life and our IT policies require the move.
> > > >
> > > > Is there a plan to officially announce support for Bookkeeper on JDK
> > 1.8
> > > ?
> > > > If it is already present or if a support matrix is documented
> somewhere
> > > > could someone please point me to it ?
> > > >
> > > > I see in the pom.xml that there is still the 1.7 version specified. I
> > > have
> > > > run the bookkeeper tests and our own higher level tests on 1.8 and
> > there
> > > > are no failures. If needed, I can raise a bug for it with the pom.xml
> > > > patch.
> > > >
> > > > Thanks,
> > > > Arun
> > > >
> > > --
> > >
> > >
> > > -- Enrico Olivelli
> > >
> >
> --
>
>
> -- Enrico Olivelli
>
Re: Java 8 support for Bookkeeper
Posted by "Arun M. Krishnakumar" <ar...@gmail.com>.
Sorry about the delayed reply.
We saw interesting issues when we were using eclipse with a different
environment. So the build on the prompt (using maven) would work well while
the following run on eclipse would through a class-related exception. I
agree that this was only a configuration issue (we had many jdks in one
machine and eclipse used a different one), but wanted to enforce the API
checks. Hence the patch.
+matteo since he's looking at moving the build to Java8. We could have a
signature check for jdk1.8 as well.
Thanks,
Arun
On Mon, Jun 13, 2016 at 1:24 PM, Enrico Olivelli <eo...@gmail.com>
wrote:
> Honestly I've been running BookKeeper on java8 in production since at
> least 2 years and I have never got java8-related issues.
> Do you think that that change on ConcurrentHashMap would be actually
> source of problems? Can you file a JIRA?
>
> Regards
> Enrico
>
> Il Mer 8 Giu 2016 20:23 Arun M. Krishnakumar <ar...@gmail.com> ha
> scritto:
>
>> (Rebooting an old thread about running binaries compiled for jdk1.7 while
>> using a jdk1.8 compilation environment and runtime)
>>
>> Just to be safe, I used maven's animal-sniffer plugin (
>> http://www.mojohaus.org/animal-sniffer/animal-sniffer-maven-plugin/)
>> that's recommended by maven and built bookkeeper. It found one issue in
>> ConcurrentHashMap.keySet whose definition changes from 1.7 to 1.8.
>>
>> The patch that fixes this follows. Could we add this plugin to the
>> codebase so that we can safely use an older version of jdk ?
>>
>>
>> diff --git
>> a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
>> b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
>> index 12ad303..8ff1e98 100644
>> ---
>> a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
>> +++
>> b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
>> @@ -21,6 +21,7 @@ import java.io.IOException;
>> import java.nio.channels.ClosedChannelException;
>> import java.util.ArrayDeque;
>> import java.util.Collections;
>> +import java.util.Enumeration;
>> import java.util.Queue;
>> import java.util.Set;
>> import java.util.concurrent.ConcurrentHashMap;
>> @@ -814,7 +815,9 @@ public class PerChannelBookieClient extends
>> SimpleChannelHandler implements Chan
>> // in case they get a write failure on the socket. The one who
>> // successfully removes the key from the map is the one
>> responsible for
>> // calling the application callback.
>> - for (CompletionKey key : completionObjects.keySet()) {
>> + Enumeration<CompletionKey> keys = completionObjects.keys();
>> + while (keys.hasMoreElements()) {
>> + CompletionKey key = keys.nextElement();
>> switch (key.operationType) {
>> case ADD_ENTRY:
>> errorOutAddKey(key, rc);
>>
>> diff --git a/pom.xml b/pom.xml
>> index 496a687..fec12f6 100644
>> --- a/pom.xml
>> +++ b/pom.xml
>> @@ -197,6 +197,31 @@
>> </configuration>
>> </plugin>
>> <plugin>
>> + <groupId>org.codehaus.mojo</groupId>
>> + <artifactId>animal-sniffer-maven-plugin</artifactId>
>> + <version>1.15</version>
>> + <executions>
>> + <execution>
>> + <id>check-java-version</id>
>> + <phase>verify</phase>
>> + <goals>
>> + <goal>check</goal>
>> + </goals>
>> + <configuration>
>> + <ignores>
>> +
>> <ignore>com.sun.management.OperatingSystemMXBean</ignore>
>> +
>> <ignore>com.sun.management.UnixOperatingSystemMXBean</ignore>
>> + </ignores>
>> + <signature>
>> + <groupId>org.codehaus.mojo.signature</groupId>
>> + <artifactId>java17</artifactId>
>> + <version>1.0</version>
>> + </signature>
>> + </configuration>
>> + </execution>
>> + </executions>
>> + </plugin>
>> + <plugin>
>> <groupId>org.apache.maven.plugins</groupId>
>> <artifactId>maven-surefire-plugin</artifactId>
>> <version>2.19.1</version>
>>
>>
>> Thanks,
>> Arun
>>
>> On Wed, Apr 20, 2016 at 1:32 PM, Enrico Olivelli <eo...@gmail.com>
>> wrote:
>>
>>> Don't worry. Java code compiled for Java7 runs without changes for java8
>>> usually.
>>> That instruction tells that the java byte code is to be compatible with
>>> Java7 virtual jams and newer versions.
>>>
>>> Cheers
>>> Enrico
>>>
>>> Il Mer 20 Apr 2016 20:25 Arun M. Krishnakumar <ar...@gmail.com> ha
>>> scritto:
>>>
>>> > Thanks Enrico for the feedback.
>>> >
>>> > I am new to Maven so am trying to understand this better. I see that
>>> the
>>> > pom.xml in the base bookkeeper directory has the following lines:
>>> >
>>> > <plugin>
>>> > <artifactId>maven-compiler-plugin</artifactId>
>>> > <version>3.0</version>
>>> > <configuration>
>>> > * <source>1.7</source>*
>>> > * <target>1.7</target>*
>>> > <compilerArguments>
>>> > <Werror />
>>> > <Xlint:deprecation />
>>> > <Xlint:unchecked />
>>> > </compilerArguments>
>>> > </configuration>
>>> > </plugin>
>>> >
>>> > So it looks like the source can make use of only Java 7 features
>>> currently.
>>> >
>>> > Also, when I do the following:
>>> > 1. Build using "mvn clean install -DskipTests" from the bookkeeper
>>> > directory
>>> > 2. Get the Bookie.class from the "bookkeeper-server-4.4.0-SNAPSHOT.jar"
>>> > file
>>> > 3. javap -verbose Bookie.class | grep -i "Major"
>>> >
>>> > I get the value 51 which indicates that the class has been built for
>>> Java
>>> > 7. So my understanding of your mail is that the Java 7 binaries
>>> generated
>>> > run well on Java 8 JVM. Is that accurate ?
>>> >
>>> > My worry was about the incompatibilities that have been mentioned in
>>> the
>>> > compatibility docs.
>>> >
>>> > Thanks,
>>> > Arun
>>> >
>>> >
>>> >
>>> >
>>> > On Tue, Apr 19, 2016 at 11:35 PM, Enrico Olivelli <eolivelli@gmail.com
>>> >
>>> > wrote:
>>> >
>>> > > Hi Arun,
>>> > > BookKeeper 4.3.2 is compiled for java6. New upcoming release will
>>> > leverage
>>> > > Java7 features like autocloseable.
>>> > > There is no need to change the pom.xml in order to make it compatible
>>> > with
>>> > > java8.
>>> > > I'm running it on java8 since ever.
>>> > >
>>> > > --Enrico
>>> > >
>>> > > Il Mer 20 Apr 2016 05:16 Arun M. Krishnakumar <ar...@gmail.com>
>>> ha
>>> > > scritto:
>>> > >
>>> > > > Hi,
>>> > > >
>>> > > > We are moving to Java 8 at our company since Java 7 has reached
>>> its end
>>> > > of
>>> > > > life and our IT policies require the move.
>>> > > >
>>> > > > Is there a plan to officially announce support for Bookkeeper on
>>> JDK
>>> > 1.8
>>> > > ?
>>> > > > If it is already present or if a support matrix is documented
>>> somewhere
>>> > > > could someone please point me to it ?
>>> > > >
>>> > > > I see in the pom.xml that there is still the 1.7 version
>>> specified. I
>>> > > have
>>> > > > run the bookkeeper tests and our own higher level tests on 1.8 and
>>> > there
>>> > > > are no failures. If needed, I can raise a bug for it with the
>>> pom.xml
>>> > > > patch.
>>> > > >
>>> > > > Thanks,
>>> > > > Arun
>>> > > >
>>> > > --
>>> > >
>>> > >
>>> > > -- Enrico Olivelli
>>> > >
>>> >
>>> --
>>>
>>>
>>> -- Enrico Olivelli
>>>
>>
>> --
>
>
> -- Enrico Olivelli
>
Re: Java 8 support for Bookkeeper
Posted by Enrico Olivelli <eo...@gmail.com>.
Honestly I've been running BookKeeper on java8 in production since at least
2 years and I have never got java8-related issues.
Do you think that that change on ConcurrentHashMap would be actually source
of problems? Can you file a JIRA?
Regards
Enrico
Il Mer 8 Giu 2016 20:23 Arun M. Krishnakumar <ar...@gmail.com> ha
scritto:
> (Rebooting an old thread about running binaries compiled for jdk1.7 while
> using a jdk1.8 compilation environment and runtime)
>
> Just to be safe, I used maven's animal-sniffer plugin (
> http://www.mojohaus.org/animal-sniffer/animal-sniffer-maven-plugin/)
> that's recommended by maven and built bookkeeper. It found one issue in
> ConcurrentHashMap.keySet whose definition changes from 1.7 to 1.8.
>
> The patch that fixes this follows. Could we add this plugin to the
> codebase so that we can safely use an older version of jdk ?
>
>
> diff --git
> a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
> b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
> index 12ad303..8ff1e98 100644
> ---
> a/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
> +++
> b/bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
> @@ -21,6 +21,7 @@ import java.io.IOException;
> import java.nio.channels.ClosedChannelException;
> import java.util.ArrayDeque;
> import java.util.Collections;
> +import java.util.Enumeration;
> import java.util.Queue;
> import java.util.Set;
> import java.util.concurrent.ConcurrentHashMap;
> @@ -814,7 +815,9 @@ public class PerChannelBookieClient extends
> SimpleChannelHandler implements Chan
> // in case they get a write failure on the socket. The one who
> // successfully removes the key from the map is the one
> responsible for
> // calling the application callback.
> - for (CompletionKey key : completionObjects.keySet()) {
> + Enumeration<CompletionKey> keys = completionObjects.keys();
> + while (keys.hasMoreElements()) {
> + CompletionKey key = keys.nextElement();
> switch (key.operationType) {
> case ADD_ENTRY:
> errorOutAddKey(key, rc);
>
> diff --git a/pom.xml b/pom.xml
> index 496a687..fec12f6 100644
> --- a/pom.xml
> +++ b/pom.xml
> @@ -197,6 +197,31 @@
> </configuration>
> </plugin>
> <plugin>
> + <groupId>org.codehaus.mojo</groupId>
> + <artifactId>animal-sniffer-maven-plugin</artifactId>
> + <version>1.15</version>
> + <executions>
> + <execution>
> + <id>check-java-version</id>
> + <phase>verify</phase>
> + <goals>
> + <goal>check</goal>
> + </goals>
> + <configuration>
> + <ignores>
> +
> <ignore>com.sun.management.OperatingSystemMXBean</ignore>
> +
> <ignore>com.sun.management.UnixOperatingSystemMXBean</ignore>
> + </ignores>
> + <signature>
> + <groupId>org.codehaus.mojo.signature</groupId>
> + <artifactId>java17</artifactId>
> + <version>1.0</version>
> + </signature>
> + </configuration>
> + </execution>
> + </executions>
> + </plugin>
> + <plugin>
> <groupId>org.apache.maven.plugins</groupId>
> <artifactId>maven-surefire-plugin</artifactId>
> <version>2.19.1</version>
>
>
> Thanks,
> Arun
>
> On Wed, Apr 20, 2016 at 1:32 PM, Enrico Olivelli <eo...@gmail.com>
> wrote:
>
>> Don't worry. Java code compiled for Java7 runs without changes for java8
>> usually.
>> That instruction tells that the java byte code is to be compatible with
>> Java7 virtual jams and newer versions.
>>
>> Cheers
>> Enrico
>>
>> Il Mer 20 Apr 2016 20:25 Arun M. Krishnakumar <ar...@gmail.com> ha
>> scritto:
>>
>> > Thanks Enrico for the feedback.
>> >
>> > I am new to Maven so am trying to understand this better. I see that the
>> > pom.xml in the base bookkeeper directory has the following lines:
>> >
>> > <plugin>
>> > <artifactId>maven-compiler-plugin</artifactId>
>> > <version>3.0</version>
>> > <configuration>
>> > * <source>1.7</source>*
>> > * <target>1.7</target>*
>> > <compilerArguments>
>> > <Werror />
>> > <Xlint:deprecation />
>> > <Xlint:unchecked />
>> > </compilerArguments>
>> > </configuration>
>> > </plugin>
>> >
>> > So it looks like the source can make use of only Java 7 features
>> currently.
>> >
>> > Also, when I do the following:
>> > 1. Build using "mvn clean install -DskipTests" from the bookkeeper
>> > directory
>> > 2. Get the Bookie.class from the "bookkeeper-server-4.4.0-SNAPSHOT.jar"
>> > file
>> > 3. javap -verbose Bookie.class | grep -i "Major"
>> >
>> > I get the value 51 which indicates that the class has been built for
>> Java
>> > 7. So my understanding of your mail is that the Java 7 binaries
>> generated
>> > run well on Java 8 JVM. Is that accurate ?
>> >
>> > My worry was about the incompatibilities that have been mentioned in the
>> > compatibility docs.
>> >
>> > Thanks,
>> > Arun
>> >
>> >
>> >
>> >
>> > On Tue, Apr 19, 2016 at 11:35 PM, Enrico Olivelli <eo...@gmail.com>
>> > wrote:
>> >
>> > > Hi Arun,
>> > > BookKeeper 4.3.2 is compiled for java6. New upcoming release will
>> > leverage
>> > > Java7 features like autocloseable.
>> > > There is no need to change the pom.xml in order to make it compatible
>> > with
>> > > java8.
>> > > I'm running it on java8 since ever.
>> > >
>> > > --Enrico
>> > >
>> > > Il Mer 20 Apr 2016 05:16 Arun M. Krishnakumar <ar...@gmail.com> ha
>> > > scritto:
>> > >
>> > > > Hi,
>> > > >
>> > > > We are moving to Java 8 at our company since Java 7 has reached its
>> end
>> > > of
>> > > > life and our IT policies require the move.
>> > > >
>> > > > Is there a plan to officially announce support for Bookkeeper on JDK
>> > 1.8
>> > > ?
>> > > > If it is already present or if a support matrix is documented
>> somewhere
>> > > > could someone please point me to it ?
>> > > >
>> > > > I see in the pom.xml that there is still the 1.7 version specified.
>> I
>> > > have
>> > > > run the bookkeeper tests and our own higher level tests on 1.8 and
>> > there
>> > > > are no failures. If needed, I can raise a bug for it with the
>> pom.xml
>> > > > patch.
>> > > >
>> > > > Thanks,
>> > > > Arun
>> > > >
>> > > --
>> > >
>> > >
>> > > -- Enrico Olivelli
>> > >
>> >
>> --
>>
>>
>> -- Enrico Olivelli
>>
>
> --
-- Enrico Olivelli