Skip to content

Commit

Permalink
Make Pinot JDK 11 Compilable (#6424)
Browse files Browse the repository at this point in the history
* 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.1024@gmail.com>
  • Loading branch information
elonazoulay and xiangfu0 committed Jun 14, 2021
1 parent 46009e1 commit 4fe7153
Show file tree
Hide file tree
Showing 9 changed files with 87 additions and 99 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/pinot_tests.yml
Expand Up @@ -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
Expand All @@ -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
Expand Down
28 changes: 27 additions & 1 deletion .github/workflows/scripts/.pinot_quickstart.sh
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion docker/images/pinot/Dockerfile
Expand Up @@ -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=dev@pinot.apache.org
Expand Down
8 changes: 4 additions & 4 deletions kubernetes/helm/pinot/values.yaml
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -75,7 +75,7 @@ public void testGetLiveBrokersExceptionState() throws IOException {
// 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();
Expand Down Expand Up @@ -104,7 +104,7 @@ public void testGetTableToBrokersMapExceptionState() {
// 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();
Expand Down
Expand Up @@ -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;
Expand Down Expand Up @@ -104,7 +103,7 @@ public void testBytes() {

@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);
Expand Down
Expand Up @@ -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;
Expand All @@ -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);
}
Expand Down
102 changes: 33 additions & 69 deletions pinot-tools/src/main/resources/appAssemblerScriptTemplate
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand All @@ -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 \
Expand Down
12 changes: 7 additions & 5 deletions pom.xml
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down

0 comments on commit 4fe7153

Please sign in to comment.