This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push: new 4fe7153 Make Pinot JDK 11 Compilable (#6424) 4fe7153 is described below commit 4fe7153eee33b2bb66a69a9b5e58fe3ecc832830 Author: Elon Azoulay <elon.azou...@gmail.com> AuthorDate: Mon Jun 14 13:45:09 2021 -0700 Make Pinot JDK 11 Compilable (#6424) * Fix PluginClassLoader when using JRE9+ The application classloader does not extend URLClassLoader in java9+. See https://community.oracle.com/tech/developers/discussion/4011800/base-classloader-no-longer-from-urlclassloader. By invoking addURL directly, if the class is not found in the delegate classLoader it will be found via findClass(). * Fix Schema.equals Add _complexFieldSpecs to equals and hashCode. Update MetadataEqualsHashCodeTest to ignore _virtualColumnProvider in equality/hashcode tests. * Fix ExternalViewReaderTest IOException is no longer a checked exception of ZkClient.readData. ZkClient.readData throws an unchecked ZkNoNodeException which extends RuntimeException. * Update maven compiler to use java11 * Remove internal jdk apis from appAssemblerScriptTemplate * use different jdk.version in quickstart * Make SegmentDeletionManagerTest single threaded The test works locally but in ci it fails sporadically. * Increase wait for scheduled tasks in BasicAuthRealtimeIntegrationTest This test passes locally but fails sporadically in ci. * fixup! Fix Schema.equals * fixing tests Co-authored-by: Xiang Fu <xiangfu.1...@gmail.com> --- .github/workflows/pinot_tests.yml | 8 +- .github/workflows/scripts/.pinot_quickstart.sh | 28 +++++- docker/images/pinot/Dockerfile | 2 +- kubernetes/helm/pinot/values.yaml | 8 +- .../pinot/client/ExternalViewReaderTest.java | 4 +- .../pinot/common/utils/PinotDataTypeTest.java | 3 +- .../apache/pinot/spi/plugin/PluginClassLoader.java | 19 ++-- .../src/main/resources/appAssemblerScriptTemplate | 102 +++++++-------------- pom.xml | 12 ++- 9 files changed, 87 insertions(+), 99 deletions(-) diff --git a/.github/workflows/pinot_tests.yml b/.github/workflows/pinot_tests.yml index e4ddb25..f4c3ed5 100644 --- a/.github/workflows/pinot_tests.yml +++ b/.github/workflows/pinot_tests.yml @@ -42,10 +42,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - name: Set up JDK 1.8 + - name: Set up JDK 11 uses: actions/setup-java@v1 with: - java-version: 1.8 + java-version: 11 - name: Unit Test env: RUN_INTEGRATION_TESTS: false @@ -59,10 +59,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - name: Set up JDK 1.8 + - name: Set up JDK 11 uses: actions/setup-java@v1 with: - java-version: 1.8 + java-version: 11 - name: Integration Test env: RUN_INTEGRATION_TESTS: true diff --git a/.github/workflows/scripts/.pinot_quickstart.sh b/.github/workflows/scripts/.pinot_quickstart.sh index 8fbe3ee..3d8175e 100755 --- a/.github/workflows/scripts/.pinot_quickstart.sh +++ b/.github/workflows/scripts/.pinot_quickstart.sh @@ -40,12 +40,38 @@ netstat -i # Java version java -version +jdk_version() { + IFS=' +' + # remove \r for Cygwin + lines=$(java -Xms32M -Xmx32M -version 2>&1 | tr '\r' '\n') + for line in $lines; do + if test -z $result && echo "$line" | grep -q 'version "' + then + ver=$(echo $line | sed -e 's/.*version "\(.*\)"\(.*\)/\1/; 1q') + # on macOS, sed doesn't support '?' + if case $ver in "1."*) true;; *) false;; esac; + then + result=$(echo $ver | sed -e 's/1\.\([0-9]*\)\(.*\)/\1/; 1q') + else + result=$(echo $ver | sed -e 's/\([0-9]*\)\(.*\)/\1/; 1q') + fi + fi + done + unset IFS + echo "$result" +} +JAVA_VER="$(jdk_version)" # Build PASS=0 for i in $(seq 1 2) do - mvn clean install -B -DskipTests=true -Pbin-dist -Dmaven.javadoc.skip=true + if [ "$JAVA_VER" -gt 11 ] ; then + mvn clean install -B -DskipTests=true -Pbin-dist -Dmaven.javadoc.skip=true -Djdk.version=11 + else + mvn clean install -B -DskipTests=true -Pbin-dist -Dmaven.javadoc.skip=true -Djdk.version=${JAVA_VER} + fi if [ $? -eq 0 ]; then PASS=1 break; diff --git a/docker/images/pinot/Dockerfile b/docker/images/pinot/Dockerfile index ac6a0fa..459590a 100644 --- a/docker/images/pinot/Dockerfile +++ b/docker/images/pinot/Dockerfile @@ -16,7 +16,7 @@ # specific language governing permissions and limitations # under the License. # -ARG JAVA_VERSION=8 +ARG JAVA_VERSION=11 FROM openjdk:${JAVA_VERSION} AS pinot_build_env LABEL MAINTAINER=d...@pinot.apache.org diff --git a/kubernetes/helm/pinot/values.yaml b/kubernetes/helm/pinot/values.yaml index 81d26e0..3791a10 100644 --- a/kubernetes/helm/pinot/values.yaml +++ b/kubernetes/helm/pinot/values.yaml @@ -83,7 +83,7 @@ controller: host: pinot-controller port: 9000 - jvmOpts: "-Xms256M -Xmx1G -XX:+UseG1GC -XX:MaxGCPauseMillis=200 -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime -Xloggc:/opt/pinot/gc-pinot-controller.log" + jvmOpts: "-Xms256M -Xmx1G -XX:+UseG1GC -XX:MaxGCPauseMillis=200 -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime -Xlog:gc*,safepoint:gc.log:time,uptime -Xlog:gc:/opt/pinot/gc-pinot-controller.log" log4j2ConfFile: /opt/pinot/conf/log4j2.xml pluginsDir: /opt/pinot/plugins @@ -152,7 +152,7 @@ broker: # fsGroup: 2000 securityContext: {} - jvmOpts: "-Xms256M -Xmx1G -XX:+UseG1GC -XX:MaxGCPauseMillis=200 -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime -Xloggc:/opt/pinot/gc-pinot-broker.log" + jvmOpts: "-Xms256M -Xmx1G -XX:+UseG1GC -XX:MaxGCPauseMillis=200 -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime -Xlog:gc*,safepoint:gc.log:time,uptime -Xlog:gc:/opt/pinot/gc-pinot-broker.log" log4j2ConfFile: /opt/pinot/conf/log4j2.xml pluginsDir: /opt/pinot/plugins @@ -244,7 +244,7 @@ server: storageClass: "" #storageClass: "ssd" - jvmOpts: "-Xms512M -Xmx1G -XX:+UseG1GC -XX:MaxGCPauseMillis=200 -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime -Xloggc:/opt/pinot/gc-pinot-server.log" + jvmOpts: "-Xms512M -Xmx1G -XX:+UseG1GC -XX:MaxGCPauseMillis=200 -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime -Xlog:gc*,safepoint:gc.log:time,uptime -Xlog:gc:/opt/pinot/gc-pinot-server.log" log4j2ConfFile: /opt/pinot/conf/log4j2.xml pluginsDir: /opt/pinot/plugins @@ -317,7 +317,7 @@ minion: readinessEnabled: false dataDir: /var/pinot/minion/data - jvmOpts: "-Xms256M -Xmx1G -XX:+UseG1GC -XX:MaxGCPauseMillis=200 -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime -Xloggc:/opt/pinot/gc-pinot-minion.log" + jvmOpts: "-Xms256M -Xmx1G -XX:+UseG1GC -XX:MaxGCPauseMillis=200 -XX:+PrintGCDetails -XX:+PrintGCDateStamps -XX:+PrintGCApplicationStoppedTime -XX:+PrintGCApplicationConcurrentTime -Xlog:gc*,safepoint:gc.log:time,uptime -Xlog:gc:/opt/pinot/gc-pinot-minion.log" log4j2ConfFile: /opt/pinot/conf/log4j2.xml pluginsDir: /opt/pinot/plugins diff --git a/pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/ExternalViewReaderTest.java b/pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/ExternalViewReaderTest.java index 02aae0d..5e35683 100644 --- a/pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/ExternalViewReaderTest.java +++ b/pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/ExternalViewReaderTest.java @@ -75,7 +75,7 @@ public class ExternalViewReaderTest { // Setup final List<String> expectedResult = Arrays.asList(); when(mockZkClient.readData(Mockito.anyString(), Mockito.anyBoolean())).thenThrow( - IOException.class); + RuntimeException.class); // Run the test final List<String> result = externalViewReaderUnderTest.getLiveBrokers(); @@ -104,7 +104,7 @@ public class ExternalViewReaderTest { // Setup final Map<String, List<String>> expectedResult = new HashMap<>(); when(mockZkClient.readData(Mockito.anyString(), Mockito.anyBoolean())).thenThrow( - IOException.class); + RuntimeException.class); // Run the test final Map<String, List<String>> result = externalViewReaderUnderTest.getTableToBrokersMap(); diff --git a/pinot-common/src/test/java/org/apache/pinot/common/utils/PinotDataTypeTest.java b/pinot-common/src/test/java/org/apache/pinot/common/utils/PinotDataTypeTest.java index 2970b4b..1d1e334 100644 --- a/pinot-common/src/test/java/org/apache/pinot/common/utils/PinotDataTypeTest.java +++ b/pinot-common/src/test/java/org/apache/pinot/common/utils/PinotDataTypeTest.java @@ -19,7 +19,6 @@ package org.apache.pinot.common.utils; import java.sql.Timestamp; -import java.time.LocalDateTime; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -104,7 +103,7 @@ public class PinotDataTypeTest { @Test public void testTimestamp() { - Timestamp timestamp = Timestamp.valueOf(LocalDateTime.now()); + Timestamp timestamp = new Timestamp(System.currentTimeMillis()); assertEquals(TIMESTAMP.convert(timestamp.getTime(), LONG), timestamp); assertEquals(TIMESTAMP.convert(timestamp.toString(), STRING), timestamp); assertEquals(TIMESTAMP.convert(timestamp.getTime(), JSON), timestamp); diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/plugin/PluginClassLoader.java b/pinot-spi/src/main/java/org/apache/pinot/spi/plugin/PluginClassLoader.java index b5b3965..3181aa6 100644 --- a/pinot-spi/src/main/java/org/apache/pinot/spi/plugin/PluginClassLoader.java +++ b/pinot-spi/src/main/java/org/apache/pinot/spi/plugin/PluginClassLoader.java @@ -19,7 +19,6 @@ package org.apache.pinot.spi.plugin; import java.io.IOException; -import java.lang.reflect.Method; import java.net.URL; import java.net.URLClassLoader; import java.util.Enumeration; @@ -36,18 +35,16 @@ public class PluginClassLoader extends URLClassLoader { public PluginClassLoader(URL[] urls, ClassLoader parent) { super(urls, parent); classLoader = PluginClassLoader.class.getClassLoader(); - Method method = null; - try { - method = URLClassLoader.class.getDeclaredMethod("addURL", URL.class); - - } catch (NoSuchMethodException e) { - //this should never happen - ExceptionUtils.rethrow(e); - } - method.setAccessible(true); for (URL url : urls) { try { - method.invoke(classLoader, url); + /** + * ClassLoader in java9+ does not extend URLClassLoader. + * If the class is not found in the parent classloader, + * it will be found in this classloader via findClass(). + * + * @see https://community.oracle.com/tech/developers/discussion/4011800/base-classloader-no-longer-from-urlclassloader + */ + addURL(url); } catch (Exception e) { ExceptionUtils.rethrow(e); } diff --git a/pinot-tools/src/main/resources/appAssemblerScriptTemplate b/pinot-tools/src/main/resources/appAssemblerScriptTemplate index 03ce9e0..1f8b2e9 100644 --- a/pinot-tools/src/main/resources/appAssemblerScriptTemplate +++ b/pinot-tools/src/main/resources/appAssemblerScriptTemplate @@ -18,28 +18,6 @@ # under the License. # -jdk_version() { - IFS=' -' - # remove \r for Cygwin - lines=$(java -Xms32M -Xmx32M -version 2>&1 | tr '\r' '\n') - for line in $lines; do - if test -z $result && echo "$line" | grep -q 'version "' - then - ver=$(echo $line | sed -e 's/.*version "\(.*\)"\(.*\)/\1/; 1q') - # on macOS, sed doesn't support '?' - if case $ver in "1."*) true;; *) false;; esac; - then - result=$(echo $ver | sed -e 's/1\.\([0-9]*\)\(.*\)/\1/; 1q') - else - result=$(echo $ver | sed -e 's/\([0-9]*\)\(.*\)/\1/; 1q') - fi - fi - done - unset IFS - echo "$result" -} - # resolve links - $0 may be a softlink PRG="$0" @@ -129,46 +107,40 @@ if [ -n "$CLASSPATH_PREFIX" ] ; then CLASSPATH=$CLASSPATH_PREFIX:$CLASSPATH fi - -JAVA_VER="$(jdk_version)" - -# For java 9 and later version, we need to explicitly set Pinot Plugins directory into classpath. -if [ "$JAVA_VER" -gt 8 ] ; then - # Set $PLUGINS_CLASSPATH for plugin jars to be put into classpath. - # $PLUGINS_DIR and $PLUGINS_INCLUDE are used if $PLUGINS_CLASSPATH is not set. - # $PLUGINS_DIR is the root directory of plugins directory, default to '"$BASEDIR"/plugins' if not set. - # $PLUGINS_INCLUDE is semi-colon separated plugins name, e.g. pinot-avro;pinot-batch-ingestion-standalone. Default is not set, which means load all the plugin jars. - if [ -z "$PLUGINS_CLASSPATH" ] ; then - if [ -z "$PLUGINS_DIR" ] ; then - PLUGINS_DIR="$BASEDIR"/plugins - fi - if [ -d "$PLUGINS_DIR" ] ; then - if [ -n "$PLUGINS_INCLUDE" ] ; then - IFS=';' echo "$PLUGINS_INCLUDE" | read -ra PLUGINS_ARR - for PLUGIN_JAR in "${PLUGINS_ARR[@]}"; do - PLUGIN_JAR_PATH=$(find "$PLUGINS_DIR" -path \*/"$PLUGIN_JAR"/"$PLUGIN_JAR"-\*.jar) - if [ -n "$PLUGINS_CLASSPATH" ] ; then - PLUGINS_CLASSPATH=$PLUGINS_CLASSPATH:$PLUGIN_JAR_PATH - else - PLUGINS_CLASSPATH=$PLUGIN_JAR_PATH - fi - done - else - PLUGIN_JARS=$(find "$PLUGINS_DIR" -name \*.jar) - for PLUGIN_JAR in $PLUGIN_JARS ; do +# Set $PLUGINS_CLASSPATH for plugin jars to be put into classpath. +# $PLUGINS_DIR and $PLUGINS_INCLUDE are used if $PLUGINS_CLASSPATH is not set. +# $PLUGINS_DIR is the root directory of plugins directory, default to '"$BASEDIR"/plugins' if not set. +# $PLUGINS_INCLUDE is semi-colon separated plugins name, e.g. pinot-avro;pinot-batch-ingestion-standalone. Default is not set, which means load all the plugin jars. +if [ -z "$PLUGINS_CLASSPATH" ] ; then + if [ -z "$PLUGINS_DIR" ] ; then + PLUGINS_DIR="$BASEDIR"/plugins + fi + if [ -d "$PLUGINS_DIR" ] ; then + if [ -n "$PLUGINS_INCLUDE" ] ; then + IFS=';' echo "$PLUGINS_INCLUDE" | read -ra PLUGINS_ARR + for PLUGIN_JAR in "${PLUGINS_ARR[@]}"; do + PLUGIN_JAR_PATH=$(find "$PLUGINS_DIR" -path \*/"$PLUGIN_JAR"/"$PLUGIN_JAR"-\*.jar) if [ -n "$PLUGINS_CLASSPATH" ] ; then - PLUGINS_CLASSPATH=$PLUGINS_CLASSPATH:$PLUGIN_JAR + PLUGINS_CLASSPATH=$PLUGINS_CLASSPATH:$PLUGIN_JAR_PATH else - PLUGINS_CLASSPATH=$PLUGIN_JAR + PLUGINS_CLASSPATH=$PLUGIN_JAR_PATH fi - done - fi + done + else + PLUGIN_JARS=$(find "$PLUGINS_DIR" -name \*.jar) + for PLUGIN_JAR in $PLUGIN_JARS ; do + if [ -n "$PLUGINS_CLASSPATH" ] ; then + PLUGINS_CLASSPATH=$PLUGINS_CLASSPATH:$PLUGIN_JAR + else + PLUGINS_CLASSPATH=$PLUGIN_JAR + fi + done fi fi +fi - if [ -n "$PLUGINS_CLASSPATH" ] ; then - CLASSPATH=$CLASSPATH:$PLUGINS_CLASSPATH - fi +if [ -n "$PLUGINS_CLASSPATH" ] ; then + CLASSPATH=$CLASSPATH:$PLUGINS_CLASSPATH fi # For Cygwin, switch paths to Windows format before running java @@ -186,20 +158,12 @@ else ALL_JAVA_OPTS=$JAVA_OPTS fi -# For java 8, we set jvm system property `plugins.dir` to load Pinot plugins -if [ "$JAVA_VER" -eq 8 ] ; then - if [ -z "$PLUGINS_DIR" ] ; then - PLUGINS_DIR=$BASEDIR/plugins - fi - ALL_JAVA_OPTS="$ALL_JAVA_OPTS -Dplugins.dir=$PLUGINS_DIR" - if [ -n "$PLUGINS_INCLUDE" ] ; then - ALL_JAVA_OPTS="$ALL_JAVA_OPTS -Dplugins.include=$PLUGINS_INCLUDE" - fi +if [ -z "$PLUGINS_DIR" ] ; then + PLUGINS_DIR=$BASEDIR/plugins fi - -# For java 9 and later, we need to set extra java options to access JDK’s internal APIs. -if [ "$JAVA_VER" -gt 8 ] ; then - ALL_JAVA_OPTS="--add-exports java.base/jdk.internal.ref=ALL-UNNAMED $ALL_JAVA_OPTS" +ALL_JAVA_OPTS="$ALL_JAVA_OPTS -Dplugins.dir=$PLUGINS_DIR" +if [ -n "$PLUGINS_INCLUDE" ] ; then + ALL_JAVA_OPTS="$ALL_JAVA_OPTS -Dplugins.include=$PLUGINS_INCLUDE" fi exec "$JAVACMD" $ALL_JAVA_OPTS \ diff --git a/pom.xml b/pom.xml index aaf122f..bc21f56 100644 --- a/pom.xml +++ b/pom.xml @@ -103,7 +103,7 @@ <properties> <pinot.root>${basedir}</pinot.root> <build.profile.id>dev</build.profile.id> - <jdk.version>1.8</jdk.version> + <jdk.version>11</jdk.version> <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> <!-- Only unit tests are run by default. --> <skip.integration.tests>true</skip.integration.tests> @@ -151,7 +151,7 @@ <jts.version>1.16.1</jts.version> <h3.version>3.0.3</h3.version> <jmh.version>1.26</jmh.version> - <audienceannotations.version>0.11.0</audienceannotations.version> + <audienceannotations.version>0.13.0</audienceannotations.version> <!-- Sets the VM argument line used when unit tests are run. --> <argLine>-Xms4g -Xmx4g</argLine> @@ -350,7 +350,7 @@ <dependency> <groupId>nl.jqno.equalsverifier</groupId> <artifactId>equalsverifier</artifactId> - <version>1.7.2</version> + <version>3.6</version> <scope>test</scope> </dependency> <dependency> @@ -1171,7 +1171,7 @@ <dependency> <groupId>org.mockito</groupId> <artifactId>mockito-core</artifactId> - <version>2.10.0</version> + <version>3.9.0</version> <scope>test</scope> </dependency> <dependency> @@ -1290,6 +1290,8 @@ <source>${jdk.version}</source> <target>${jdk.version}</target> <compilerVersion>${jdk.version}</compilerVersion> + <fork>true</fork> + <encoding>${project.build.sourceEncoding}</encoding> </configuration> </plugin> <plugin> @@ -1553,7 +1555,7 @@ <plugin> <groupId>org.jacoco</groupId> <artifactId>jacoco-maven-plugin</artifactId> - <version>0.7.7.201606060606</version> + <version>0.8.6</version> <executions> <execution> <goals> --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org