Skip to content

Commit

Permalink
[FLINK-22765][test] Hardens ExceptionUtilsITCase#testIsMetaspaceOutOf…
Browse files Browse the repository at this point in the history 
…MemoryError in JDK21

This test started to fail quite regularly in JDK21. The problem was that the low heap size could have caused an OutOfMemoryError to appear when compiling the dummy classes. An OOM in the compilation phase results in a different error message being printed to stdout that wasn't captured by the test.

The solution is to pre-compile the classes upfront (with the normal heap size). The test main method will only load the classes. No compilation is necessary.
  • Loading branch information
@XComp
XComp committed Feb 19, 2024
1 parent f903ce2 commit ec73bc5
Show file tree
Hide file tree
Showing 2 changed files with 99 additions and 29 deletions.
41 changes: 40 additions & 1 deletion flink-core/src/test/java/org/apache/flink/testutils/ClassLoaderUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

package org.apache.flink.testutils;

import org.apache.flink.util.Preconditions;

import javax.annotation.Nullable;
import javax.tools.JavaCompiler;
import javax.tools.ToolProvider;
Expand All @@ -40,6 +42,7 @@
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.UUID;

/** Utilities to create class loaders. */
Expand Down Expand Up @@ -142,6 +145,15 @@ public ClassLoaderBuilder addService(String serviceClass, String implClass) {
return this;
}

public ClassLoaderBuilder addClass(String className) {
Preconditions.checkState(
new File(root, className + ".java").exists(),
"The class %s was added without any source code being present.",
className);

return addClass(className, null);
}

public ClassLoaderBuilder addClass(String className, String source) {
String oldValue = classes.putIfAbsent(className, source);

Expand All @@ -158,7 +170,7 @@ public ClassLoaderBuilder withParentClassLoader(ClassLoader classLoader) {
return this;
}

public URLClassLoader build() throws IOException {
public void generateSourcesAndCompile() throws IOException {
for (Map.Entry<String, String> classInfo : classes.entrySet()) {
writeAndCompile(root, createFileName(classInfo.getKey()), classInfo.getValue());
}
Expand All @@ -171,10 +183,37 @@ public URLClassLoader build() throws IOException {
for (Map.Entry<String, String> resource : resources.entrySet()) {
writeSourceFile(root, resource.getKey(), resource.getValue());
}
}

public URLClassLoader buildWithoutCompilation() throws MalformedURLException {
final int generatedSourceClassesCount =
Objects.requireNonNull(
root.listFiles(
(dir, name) -> {
if (!name.endsWith(".java")) {
return false;
}
final String derivedClassName =
name.substring(0, name.lastIndexOf('.'));
return classes.containsKey(derivedClassName);
}))
.length;
Preconditions.checkState(
generatedSourceClassesCount == classes.size(),
"The generated Java sources in %s (%s) do not match the classes in this %s (%s).",
root.getAbsolutePath(),
generatedSourceClassesCount,
ClassLoaderBuilder.class.getSimpleName(),
classes.size());

return createClassLoader(root, parent);
}

public URLClassLoader build() throws IOException {
generateSourcesAndCompile();
return buildWithoutCompilation();
}

private String createFileName(String className) {
return className + ".java";
}
Expand Down
87 changes: 59 additions & 28 deletions flink-tests/src/test/java/org/apache/flink/runtime/util/ExceptionUtilsITCase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@
import java.lang.management.MemoryPoolMXBean;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.text.IsEmptyString.isEmptyString;
import static org.junit.Assert.assertThat;

/** Tests for {@link ExceptionUtils} which require to spawn JVM process and set JVM memory args. */
Expand All @@ -55,26 +54,63 @@ public class ExceptionUtilsITCase extends TestLogger {
@Test
public void testIsDirectOutOfMemoryError() throws IOException, InterruptedException {
String className = DummyDirectAllocatingProgram.class.getName();
RunResult result = run(className, Collections.emptyList(), DIRECT_MEMORY_SIZE, -1);
RunResult result = run(className, DIRECT_MEMORY_SIZE, -1);
assertThat(result.getErrorOut() + "|" + result.getStandardOut(), is("|"));
}

@Test
public void testIsMetaspaceOutOfMemoryError() throws IOException, InterruptedException {
String className = DummyClassLoadingProgram.class.getName();
final File compiledClassesFolder = TEMPORARY_FOLDER.getRoot();
final int classCount = 10;

// compile the classes first
final String sourcePattern =
"public class %s { @Override public String toString() { return \"dummy\"; } }";
final ClassLoaderBuilder classLoaderBuilder =
ClassLoaderUtils.withRoot(compiledClassesFolder);
for (int i = 0; i < classCount; i++) {
final String dummyClassName = "DummyClass" + i;
final String source = String.format(sourcePattern, dummyClassName);
classLoaderBuilder.addClass(dummyClassName, source);
}
classLoaderBuilder.generateSourcesAndCompile();

// load only one class and record required Metaspace
RunResult normalOut =
run(className, getDummyClassLoadingProgramArgs(1), -1, INITIAL_BIG_METASPACE_SIZE);
long okMetaspace = Long.parseLong(normalOut.getStandardOut());
run(
className,
-1,
INITIAL_BIG_METASPACE_SIZE,
1,
compiledClassesFolder.getAbsolutePath());

// multiply the Metaspace size to stabilize the test - relying solely on the Metaspace size
// of the initial run might cause OOMs to appear in the main thread (due to JVM-specific
// artifacts being loaded)
long okMetaspace = 3 * Long.parseLong(normalOut.getStandardOut());
assertThat("No error is expected here.", normalOut.getErrorOut(), is(""));

// load more classes to cause 'OutOfMemoryError: Metaspace'
RunResult oomOut = run(className, getDummyClassLoadingProgramArgs(1000), -1, okMetaspace);
// 'OutOfMemoryError: Metaspace' errors are caught, hence no output means we produced the
// expected exception.
assertThat(oomOut.getErrorOut() + "|" + oomOut.getStandardOut(), is("|"));
RunResult oomOut =
run(
className,
-1,
okMetaspace,
classCount,
compiledClassesFolder.getAbsolutePath());
assertThat(
"OutOfMemoryError: Metaspace errors are caught and don't generate any output.",
oomOut.getStandardOut(),
isEmptyString());
assertThat("Nothing should have been printed to stderr.", oomOut.getErrorOut(), is(""));
}

private static RunResult run(
String className, Iterable<String> args, long directMemorySize, long metaspaceSize)
String className,
long directMemorySize,
long metaspaceSize,
Object... mainClassParameters)
throws InterruptedException, IOException {
TestProcessBuilder taskManagerProcessBuilder = new TestProcessBuilder(className);
if (directMemorySize > 0) {
Expand All @@ -86,9 +122,11 @@ private static RunResult run(
taskManagerProcessBuilder.addJvmArg(
String.format("-XX:MaxMetaspaceSize=%d", metaspaceSize));
}
for (String arg : args) {
taskManagerProcessBuilder.addMainClassArg(arg);

for (Object parameterValue : mainClassParameters) {
taskManagerProcessBuilder.addMainClassArg(String.valueOf(parameterValue));
}

// JAVA_TOOL_OPTIONS is configured on CI which would affect the process output
taskManagerProcessBuilder.withCleanEnvironment();
TestProcess p = taskManagerProcessBuilder.start();
Expand All @@ -115,12 +153,6 @@ public String getStandardOut() {
}
}

private static Collection<String> getDummyClassLoadingProgramArgs(int numberOfLoadedClasses) {
return Arrays.asList(
Integer.toString(numberOfLoadedClasses),
TEMPORARY_FOLDER.getRoot().getAbsolutePath());
}

/** Dummy java program to generate Direct OOM. */
public static class DummyDirectAllocatingProgram {
private DummyDirectAllocatingProgram() {}
Expand Down Expand Up @@ -158,8 +190,10 @@ public static void main(String[] args) {
for (int index = 0; index < numberOfLoadedClasses; index++) {
classes.add(loadDummyClass(index, args[1]));
}
String out = classes.size() > 1 ? "Exception is not thrown, metaspace usage: " : "";
output(out + getMetaspaceUsage());

if (classes.size() == 1) {
output(String.valueOf(getMetaspaceUsage()));
}
} catch (Throwable t) {
if (ExceptionUtils.isMetaspaceOutOfMemoryError(t)) {
return;
Expand All @@ -170,14 +204,11 @@ public static void main(String[] args) {

private static Class<?> loadDummyClass(int index, String folderToSaveSource)
throws ClassNotFoundException, IOException {
String className = "DummyClass" + index;
String sourcePattern =
"public class %s { @Override public String toString() { return \"%s\"; } }";
ClassLoaderBuilder classLoaderBuilder =
ClassLoaderUtils.withRoot(new File(folderToSaveSource));
classLoaderBuilder.addClass(
className, String.format(sourcePattern, className, "dummy"));
ClassLoader classLoader = classLoaderBuilder.build();
final String className = "DummyClass" + index;
final ClassLoader classLoader =
ClassLoaderUtils.withRoot(new File(folderToSaveSource))
.addClass(className)
.buildWithoutCompilation();
return Class.forName(className, true, classLoader);
}

Expand Down

0 comments on commit ec73bc5

Please sign in to comment.