Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1179] feat(build): Make Gravitino build and run against JDK8, 11, 17 #1199

Merged
merged 1 commit into from
Dec 19, 2023
Merged

[#1179] feat(build): Make Gravitino build and run against JDK8, 11, 17 #1199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 43 additions & 3 deletions build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ plugins {
alias(libs.plugins.spotless)
} else {
throw GradleException(
"Gravitino Gradle current doesn't support " +
"Gravitino Gradle toolchain current doesn't support " +
"Java version: ${JavaVersion.current()}. Please use JDK8 to 17."
)
}
Expand All @@ -44,6 +44,42 @@ plugins {
id("org.cyclonedx.bom") version "1.5.0" // Newer version fail due to our setup
}

if (extra["jdkVersion"] !in listOf("8", "11", "17")) {
throw GradleException(
"Gravitino current doesn't support building with " +
"Java version: ${extra["jdkVersion"]}. Please use JDK8, 11 or 17."
)
}

project.extra["extraJvmArgs"] = if (extra["jdkVersion"] in listOf("8", "11")) {
listOf()
} else {
listOf(
"-XX:+IgnoreUnrecognizedVMOptions",
"--add-opens", "java.base/java.io=ALL-UNNAMED",
"--add-opens", "java.base/java.lang.invoke=ALL-UNNAMED",
"--add-opens", "java.base/java.lang.reflect=ALL-UNNAMED",
"--add-opens", "java.base/java.lang=ALL-UNNAMED",
"--add-opens", "java.base/java.math=ALL-UNNAMED",
"--add-opens", "java.base/java.net=ALL-UNNAMED",
"--add-opens", "java.base/java.nio=ALL-UNNAMED",
"--add-opens", "java.base/java.text=ALL-UNNAMED",
"--add-opens", "java.base/java.time=ALL-UNNAMED",
"--add-opens", "java.base/java.util.concurrent.atomic=ALL-UNNAMED",
"--add-opens", "java.base/java.util.concurrent=ALL-UNNAMED",
"--add-opens", "java.base/java.util.regex=ALL-UNNAMED",
"--add-opens", "java.base/java.util=ALL-UNNAMED",
"--add-opens", "java.base/jdk.internal.ref=ALL-UNNAMED",
"--add-opens", "java.base/jdk.internal.reflect=ALL-UNNAMED",
"--add-opens", "java.sql/java.sql=ALL-UNNAMED",
"--add-opens", "java.base/sun.util.calendar=ALL-UNNAMED",
"--add-opens", "java.base/sun.nio.ch=ALL-UNNAMED",
"--add-opens", "java.base/sun.nio.cs=ALL-UNNAMED",
"--add-opens", "java.base/sun.security.action=ALL-UNNAMED",
"--add-opens", "java.base/sun.util.calendar=ALL-UNNAMED"
)
}

licenseReport {
renderers = arrayOf<ReportRenderer>(InventoryHtmlReportRenderer("report.html", "Backend"))
filters = arrayOf<DependencyFilter>(LicenseBundleNormalizer())
Expand Down Expand Up @@ -129,7 +165,9 @@ subprojects {
if (project.name == "trino-connector") {
languageVersion.set(JavaLanguageVersion.of(17))
} else {
languageVersion.set(JavaLanguageVersion.of(8))
languageVersion.set(JavaLanguageVersion.of(extra["jdkVersion"].toString().toInt()))
sourceCompatibility = JavaVersion.VERSION_1_8
targetCompatibility = JavaVersion.VERSION_1_8
}
}
}
Expand Down Expand Up @@ -208,7 +246,7 @@ subprojects {
sign(publishing.publications)
}

tasks.configureEach<Test> {
tasks.withType<Test> {
testLogging {
exceptionFormat = TestExceptionFormat.FULL
showExceptions = true
Expand All @@ -224,6 +262,8 @@ subprojects {
} else {
useJUnitPlatform()
}

jvmArgs(project.property("extraJvmArgs") as List<*>)
finalizedBy(tasks.getByName("jacocoTestReport"))
}
}
Expand Down
4 changes: 2 additions & 2 deletions ...catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/dyn/DynConstructors.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.google.common.collect.Maps;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Arrays;
import java.util.Map;
Expand Down Expand Up @@ -245,6 +244,7 @@ public Builder hiddenImpl(String className, Class<?>... types) {
return this;
}

@SuppressWarnings("removal")
public <T> Builder hiddenImpl(Class<T> targetClass, Class<?>... types) {
// don't do any work if an implementation has been found
if (ctor != null) {
Expand All @@ -253,7 +253,7 @@ public <T> Builder hiddenImpl(Class<T> targetClass, Class<?>... types) {

try {
Constructor<T> hidden = targetClass.getDeclaredConstructor(types);
AccessController.doPrivileged(new MakeAccessible(hidden));
java.security.AccessController.doPrivileged(new MakeAccessible(hidden));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AccessController is deprecated in JDK17 with no replacement, so currently suppress the warning as a workaround.

ctor = new Ctor<T>(hidden, targetClass);
} catch (SecurityException e) {
// unusable
Expand Down
4 changes: 2 additions & 2 deletions catalogs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/dyn/DynFields.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import com.google.common.collect.Sets;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Set;

Expand Down Expand Up @@ -302,6 +301,7 @@ public Builder hiddenImpl(String className, String fieldName) {
* @see Class#forName(String)
* @see Class#getField(String)
*/
@SuppressWarnings("removal")
public Builder hiddenImpl(Class<?> targetClass, String fieldName) {
// don't do any work if an implementation has been found
if (field != null || targetClass == null) {
Expand All @@ -310,7 +310,7 @@ public Builder hiddenImpl(Class<?> targetClass, String fieldName) {

try {
Field hidden = targetClass.getDeclaredField(fieldName);
AccessController.doPrivileged(new MakeFieldAccessible(hidden));
java.security.AccessController.doPrivileged(new MakeFieldAccessible(hidden));
this.field = new UnboundField(hidden, fieldName);
} catch (SecurityException | NoSuchFieldException e) {
// unusable
Expand Down
4 changes: 2 additions & 2 deletions ...logs/catalog-hive/src/main/java/com/datastrato/gravitino/catalog/hive/dyn/DynMethods.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.Arrays;

Expand Down Expand Up @@ -393,6 +392,7 @@ public Builder hiddenImpl(String className, Class<?>... argClasses) {
* @see Class#forName(String)
* @see Class#getMethod(String, Class[])
*/
@SuppressWarnings("removal")
public Builder hiddenImpl(Class<?> targetClass, String methodName, Class<?>... argClasses) {
// don't do any work if an implementation has been found
if (method != null) {
Expand All @@ -401,7 +401,7 @@ public Builder hiddenImpl(Class<?> targetClass, String methodName, Class<?>... a

try {
Method hidden = targetClass.getDeclaredMethod(methodName, argClasses);
AccessController.doPrivileged(new MakeAccessible(hidden));
java.security.AccessController.doPrivileged(new MakeAccessible(hidden));
this.method = new UnboundMethod(hidden, name);
} catch (SecurityException | NoSuchMethodException e) {
// unusable or not the right implementation
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/datastrato/gravitino/EntitySerDeFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public static EntitySerDe createEntitySerDe(String name) {
String className = ENTITY_SERDES.getOrDefault(name, name);

try {
return (EntitySerDe) Class.forName(className).newInstance();
return (EntitySerDe) Class.forName(className).getDeclaredConstructor().newInstance();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is deprecated in Java 11+

} catch (Exception e) {
LOG.error("Failed to create EntitySerDe by name {}.", name, e);
throw new RuntimeException("Failed to create EntitySerDe: " + name, e);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/datastrato/gravitino/EntityStoreFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public static EntityStore createEntityStore(Config config) {
String className = ENTITY_STORES.getOrDefault(name, name);

try {
return (EntityStore) Class.forName(className).newInstance();
return (EntityStore) Class.forName(className).getDeclaredConstructor().newInstance();
} catch (Exception e) {
LOG.error("Failed to create and initialize EntityStore by name {}.", name, e);
throw new RuntimeException("Failed to create and initialize EntityStore: " + name, e);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/datastrato/gravitino/aux/AuxiliaryServiceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public GravitinoAuxiliaryService loadAuxService(
try {
Class<? extends GravitinoAuxiliaryService> providerClz =
lookupAuxService(auxServiceName, cl);
return providerClz.newInstance();
return providerClz.getDeclaredConstructor().newInstance();
} catch (Exception e) {
throw new RuntimeException(e);
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/datastrato/gravitino/catalog/CatalogManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ private BaseCatalog<?> createCatalogInstance(IsolatedClassLoader classLoader, St
try {
Class<? extends CatalogProvider> providerClz =
lookupCatalogProvider(provider, cl);
return (BaseCatalog) providerClz.newInstance();
return (BaseCatalog) providerClz.getDeclaredConstructor().newInstance();
} catch (Exception e) {
LOG.error("Failed to load catalog with provider: {}", provider, e);
throw new RuntimeException(e);
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/com/datastrato/gravitino/proto/ProtoEntitySerDe.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ private <T extends Entity, M extends Message> ProtoSerDe<T, M> getProtoSerde(
Class<? extends ProtoSerDe<? extends Entity, ? extends Message>> serdeClazz =
(Class<? extends ProtoSerDe<? extends Entity, ? extends Message>>)
loadClass(ENTITY_TO_SERDE.get(k.getCanonicalName()), classLoader);
return serdeClazz.newInstance();
return serdeClazz.getDeclaredConstructor().newInstance();
} catch (Exception e) {
throw new RuntimeException(
"Failed to instantiate serde class " + k.getCanonicalName(), e);
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/com/datastrato/gravitino/storage/kv/KvEntityStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,8 @@ private static KvBackend createKvEntityBackend(Config config) {
}

try {
KvBackend kvBackend = (KvBackend) Class.forName(className).newInstance();
KvBackend kvBackend =
(KvBackend) Class.forName(className).getDeclaredConstructor().newInstance();
kvBackend.initialize(config);
return kvBackend;
} catch (Exception e) {
Expand Down
2 changes: 1 addition & 1 deletion docs/trino-connector/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ Gravitino connector index:
- [Iceberg](catalog-iceberg.md)
- [MySQL](catalog-mysql.md)
- [PostgreSQL](catalog-postgresql.md)
- [Supported SQL](sql-support.md)
- [Supported SQL](sql-support.md)
4 changes: 4 additions & 0 deletions gradle.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ version = 0.4.0-SNAPSHOT
#sonatype credentials
SONATYPE_USER = admin
SONATYPE_PASSWORD = password

#jdkVersion is used to specify the version of JDK to build and test against Gravitino, current
# supported version is 8, 11, and 17.
jdkVersion = 8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the value of jdkVersion differs from that of the JDK installed actually?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the same as before, if the specified jdkVersion is not installed locally, it will download automatically by gradle.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean if the value of jdkVersion is 8 and the JDK you installed is JDK11 or JDK17, so the final behavior is?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will build, test against jdk8, no matter which version you installed in your environment.

4 changes: 2 additions & 2 deletions server-common/src/main/java/com/datastrato/gravitino/server/web/JettyServer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.EnumSet;
import java.util.Optional;
Expand Down Expand Up @@ -393,6 +392,7 @@ private ServerConnector createServerConnector(
return new ServerConnector(server, null, serverExecutor, null, -1, -1, connectionFactories);
}

@SuppressWarnings("removal")
private ThreadPool createThreadPool(int minThreads, int maxThreads, int threadPoolWorkQueueSize) {

ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
Expand All @@ -404,7 +404,7 @@ private ThreadPool createThreadPool(int minThreads, int maxThreads, int threadPo

@Override
public Thread newThread(Runnable runnable) {
return AccessController.doPrivileged(
return java.security.AccessController.doPrivileged(
new PrivilegedAction<Thread>() {
@Override
public Thread run() {
Expand Down
Loading