You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by jasobrown <gi...@git.apache.org> on 2018/06/16 22:10:25 UTC
[GitHub] cassandra pull request #236: 9608 trunk
GitHub user jasobrown opened a pull request:
https://github.com/apache/cassandra/pull/236
9608 trunk
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/snazy/cassandra 9608-trunk
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cassandra/pull/236.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #236
----
commit 49971feea0be714bfa7ce2461dbddd0452b702a3
Author: Robert Stupp <sn...@...>
Date: 2017-09-12T18:04:30Z
make C* compile and run on Java 11 and Java 8
patch by Robert Stupp; reviewed by Jason Brown and Eduard Tudenhöfner for CASSANDRA-9608
commit 649e2385a29665c47285a4161ceb6948b6f73b2e
Author: Robert Stupp <sn...@...>
Date: 2018-06-07T18:39:24Z
update chronicle libs
use chronicle-core snapshot build to run on Java 11
the snapshot-build of chronicle-core solves the "junit.framework.AssertionFailedError: java.lang.NoSuchFieldException: reservedMemory" issue at net.openhft.chronicle.core.Jvm.<clinit>(Jvm.java:87)
see https://github.com/OpenHFT/Chronicle-Core/pull/68
commit e85301855cedc7d5ab1a49ca0c3358315d49b2f8
Author: Robert Stupp <sn...@...>
Date: 2018-06-13T07:03:07Z
Update asm to 6.2
commit 67473f94b776224c99a2962ef547812b6600d87a
Author: Robert Stupp <sn...@...>
Date: 2018-06-13T12:12:54Z
address ecj resource-warnings
commit b46f4b314bded18810595b5c45ab61594f4990d8
Author: Robert Stupp <sn...@...>
Date: 2018-06-13T12:43:02Z
enable dtests
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199331081
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -106,11 +171,57 @@ public static void createHardLink(File from, File to)
}
}
+ private static final File tempDir = new File(System.getProperty("java.io.tmpdir"));
+ private static final AtomicLong tempFileNum = new AtomicLong();
+
+ public static File getTempDir()
+ {
+ return tempDir;
+ }
+
+ /**
+ * Pretty much like {@link File#createTempFile(String, String, File)}, but with
+ * the guarantee that the "random" part of the generated file name between
+ * {@code prefix} and {@code suffix} is a positive, increasing {@code long} value.
+ */
public static File createTempFile(String prefix, String suffix, File directory)
{
try
{
- return File.createTempFile(prefix, suffix, directory);
+ // Do not use java.io.File.createTempFile(), because some tests rely on the
--- End diff --
Do the tests that require monotonicity in the file version numbers also require the versions to be related to wall-clock time? If not, and the monotonicity / uniqueness guarantees are sufficient, you can simplify the `while` loop below to get just the next value by `tempFileNum.getAndIncrement()`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra issue #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on the issue:
https://github.com/apache/cassandra/pull/236
No, it's no longer needed. I removed it.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r195913343
--- Diff: build.xml ---
@@ -110,16 +118,35 @@
<property name="jacoco.finalexecfile" value="${jacoco.export.dir}/jacoco.exec" />
<property name="jacoco.version" value="0.7.5.201505241946"/>
- <property name="byteman.version" value="3.0.3"/>
+ <property name="byteman.version" value="4.0.2"/>
+ <property name="jamm.version" value="0.3.2"/>
+ <property name="ecj.version" value="4.6.1"/>
+ <property name="ohc.version" value="0.5.1"/>
+
+ <!--
--- End diff --
nit: let's move this java version section above the dependencies section
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199253798
--- Diff: conf/cassandra-env.sh ---
@@ -86,50 +86,36 @@ calculate_heap_sizes()
fi
}
-# Determine the sort of JVM we'll be running on.
-java_ver_output=`"${JAVA:-java}" -version 2>&1`
-jvmver=`echo "$java_ver_output" | grep '[openjdk|java] version' | awk -F'"' 'NR==1 {print $2}' | cut -d\- -f1`
-JVM_VERSION=${jvmver%_*}
-JVM_PATCH_VERSION=${jvmver#*_}
-
-if [ "$JVM_VERSION" \< "1.8" ] ; then
- echo "Cassandra 3.0 and later require Java 8u40 or later."
- exit 1;
-fi
-
-if [ "$JVM_VERSION" \< "1.8" ] && [ "$JVM_PATCH_VERSION" -lt 40 ] ; then
- echo "Cassandra 3.0 and later require Java 8u40 or later."
- exit 1;
-fi
-
-jvm=`echo "$java_ver_output" | grep -A 1 '[openjdk|java] version' | awk 'NR==2 {print $1}'`
-case "$jvm" in
- OpenJDK)
- JVM_VENDOR=OpenJDK
- # this will be "64-Bit" or "32-Bit"
- JVM_ARCH=`echo "$java_ver_output" | awk 'NR==3 {print $2}'`
- ;;
- "Java(TM)")
- JVM_VENDOR=Oracle
- # this will be "64-Bit" or "32-Bit"
- JVM_ARCH=`echo "$java_ver_output" | awk 'NR==3 {print $3}'`
- ;;
- *)
- # Help fill in other JVM values
- JVM_VENDOR=other
- JVM_ARCH=unknown
- ;;
-esac
-
#GC log path has to be defined here because it needs to access CASSANDRA_HOME
-JVM_OPTS="$JVM_OPTS -Xloggc:${CASSANDRA_HOME}/logs/gc.log"
+if [ $JAVA_VERSION -ge 11 ] ; then
+ # See description of https://bugs.openjdk.java.net/browse/JDK-8046148 for details about the syntax
+ # The following is the equivalent to -XX:+PrintGCDetails -XX:+UseGCLogFileRotation -XX:NumberOfGCLogFiles=10 -XX:GCLogFileSize=10M
+ if ! grep -q "^-[X]log:gc" $CASSANDRA_CONF/jvm.options ; then # [X] to prevent ccm from replacing this line
+ # only add -Xlog:gc if it's not mentioned in jvm.options file
+ mkdir -p ${CASSANDRA_HOME}/logs
+ JVM_OPTS="$JVM_OPTS -Xlog:gc=info,heap*=trace,age*=debug,safepoint=info,promotion*=trace:file=${CASSANDRA_HOME}/logs/gc.log:time,uptime,pid,tid,level:filecount=10,filesize=10240"
--- End diff --
minor nit: `filesize=10240` is the file size in bytes. change to `10485760` if you actually want 10MB files.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440783
--- Diff: src/java/org/apache/cassandra/utils/NativeLibrary.java ---
@@ -78,7 +77,14 @@
static
{
FILE_DESCRIPTOR_FD_FIELD = FBUtilities.getProtectedField(FileDescriptor.class, "fd");
- FILE_CHANNEL_FD_FIELD = FBUtilities.getProtectedField(FileChannelImpl.class, "fd");
+ try
--- End diff --
Because of the `Class.forName` declaring the checked exception.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204382546
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -350,13 +396,38 @@ public static String getRelativePath(String basePath, String path)
public static void clean(ByteBuffer buffer)
{
- if (buffer == null)
+ if (buffer == null || !buffer.isDirect())
return;
- if (isCleanerAvailable && buffer.isDirect())
+
+ // TODO Once we can get rid of Java 8, it's simpler to call sun.misc.Unsafe.invokeCleaner(ByteBuffer),
+ // but need to take care of the attachment handling (i.e. whether 'buf' is a duplicate or slice) - that
+ // is different in sun.misc.Unsafe.invokeCleaner and this implementation.
+
+ try
{
- DirectBuffer db = (DirectBuffer) buffer;
- if (db.cleaner() != null)
- db.cleaner().clean();
+ if (buffer.isDirect())
+ {
+ Object cleaner = mhDirectBufferCleaner.bindTo(buffer).invoke();
--- End diff --
Woops. There shouldn't be any else.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra issue #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on the issue:
https://github.com/apache/cassandra/pull/236
Is javaexec.in.sh needed anymore? Looks like all the java checks are in `cassandra.in.sh` now. The file is not referenced by dtests or ccm, either.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199266510
--- Diff: src/java/org/apache/cassandra/cql3/functions/JavaBasedUDFunction.java ---
@@ -591,14 +612,48 @@ private NameEnvironmentAnswer findType(String className)
String resourceName = className.replace('.', '/') + ".class";
- try (InputStream is = UDFunction.udfClassLoader.getResourceAsStream(resourceName))
+ // up to Java 8:
+ // returns a non-null InputStream for class files
+ // since Java 11:
+ // returns a non-null InputStream for class files for application classes
+ // returns null for class files for system modules (e.g. java.base)
+ try
{
- if (is != null)
+ InputStream is = UDFunction.udfClassLoader.getResourceAsStream(resourceName);
+ try
+ {
+ if (is == null)
+ {
+ // For Java 11 try to see whether the class actually exists and read the
+ // class file data via the class' module. (This is necessary at least
+ // for 9-ea build 123)
--- End diff --
Is this still true as of java11 (ea 19)?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204364994
--- Diff: src/java/org/apache/cassandra/cql3/functions/JavaBasedUDFunction.java ---
@@ -591,14 +592,17 @@ private NameEnvironmentAnswer findType(String className)
String resourceName = className.replace('.', '/') + ".class";
- try (InputStream is = UDFunction.udfClassLoader.getResourceAsStream(resourceName))
+ try
--- End diff --
Not sure why we need a nested `try` block here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199330506
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -67,23 +71,84 @@
public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+ private static MethodHandle mhDirectBufferAddress;
--- End diff --
`mhDirectBufferAddress` isn't used anywhere. can we eliminate it?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204381413
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -64,24 +68,31 @@
public static final long ONE_TB = 1024 * ONE_GB;
private static final DecimalFormat df = new DecimalFormat("#.##");
- public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+
static
{
- boolean canClean = false;
try
{
+ clsDirectBuffer = Class.forName("sun.nio.ch.DirectBuffer");
+ Method mDirectBufferCleaner = clsDirectBuffer.getMethod("cleaner");
+ mhDirectBufferCleaner = MethodHandles.lookup().unreflect(mDirectBufferCleaner);
+ Method mCleanerClean = mDirectBufferCleaner.getReturnType().getMethod("clean");
+ mhCleanerClean = MethodHandles.lookup().unreflect(mCleanerClean);
+
ByteBuffer buf = ByteBuffer.allocateDirect(1);
- ((DirectBuffer) buf).cleaner().clean();
- canClean = true;
+ clean(buf);
}
catch (Throwable t)
{
+ logger.info("Cannot initialize optimized memory deallocator. Some data, both in-memory and on-disk, may live longer due to garbage collection.");
JVMStabilityInspector.inspectThrowable(t);
- logger.info("Cannot initialize un-mmaper. (Are you using a non-Oracle JVM?) Compacted data files will not be removed promptly. Consider using an Oracle JVM or using standard disk access mode");
+ throw new RuntimeException(t);
--- End diff --
Pretty much every JVM should have a `Cleaner` nowadays. Not having would break a lot of assumṕtions IMO.
Changed the log message to ERROR.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204362780
--- Diff: build.xml ---
@@ -1665,7 +1763,7 @@
<echo message="Mem size : ${mem.size}"/>
</target>
- <target name="test" depends="build-test,get-cores,get-mem,stress-build" description="Parallel Test Runner">
+ <target name="test" depends="build-test" description="Parallel Test Runner">
--- End diff --
Why does this not depend on `get-cores,get-mem` anymore? Otherwise, those targets are unused in the build.xml
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440755
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -67,23 +71,84 @@
public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+ private static MethodHandle mhDirectBufferAddress;
+
static
{
boolean canClean = false;
try
{
+ clsDirectBuffer = Class.forName("sun.nio.ch.DirectBuffer");
+ Method mDirectBufferCleaner = clsDirectBuffer.getMethod("cleaner");
+ mhDirectBufferCleaner = MethodHandles.lookup().unreflect(mDirectBufferCleaner);
+ Method mCleanerClean = mDirectBufferCleaner.getReturnType().getMethod("clean");
+ mhCleanerClean = MethodHandles.lookup().unreflect(mCleanerClean);
+ Method mDirectBufferAddress = clsDirectBuffer.getMethod("address");
+ mhDirectBufferAddress = MethodHandles.lookup().unreflect(mDirectBufferAddress);
+
ByteBuffer buf = ByteBuffer.allocateDirect(1);
- ((DirectBuffer) buf).cleaner().clean();
+ cleanerClean(buf);
canClean = true;
}
catch (Throwable t)
{
+ logger.info("Cannot initialize un-mmaper. Compacted data files will not be removed promptly.", t);
--- End diff --
Sounds good
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199330666
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -67,23 +71,84 @@
public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+ private static MethodHandle mhDirectBufferAddress;
+
static
{
boolean canClean = false;
try
{
+ clsDirectBuffer = Class.forName("sun.nio.ch.DirectBuffer");
+ Method mDirectBufferCleaner = clsDirectBuffer.getMethod("cleaner");
+ mhDirectBufferCleaner = MethodHandles.lookup().unreflect(mDirectBufferCleaner);
+ Method mCleanerClean = mDirectBufferCleaner.getReturnType().getMethod("clean");
+ mhCleanerClean = MethodHandles.lookup().unreflect(mCleanerClean);
+ Method mDirectBufferAddress = clsDirectBuffer.getMethod("address");
+ mhDirectBufferAddress = MethodHandles.lookup().unreflect(mDirectBufferAddress);
+
ByteBuffer buf = ByteBuffer.allocateDirect(1);
- ((DirectBuffer) buf).cleaner().clean();
+ cleanerClean(buf);
canClean = true;
}
catch (Throwable t)
{
+ logger.info("Cannot initialize un-mmaper. Compacted data files will not be removed promptly.", t);
--- End diff --
The original log message here was rather awful. I'm not sure sure what actions an operator could take after seeing this, and `Compacted data files will not be removed promptly` is sufficiently vague.
Maybe we could say something like: `Cannot initialize optimized memory deallocator. Some data, both in-memory and on-disk, may live longer due to garbage collection."
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440835
--- Diff: build.xml ---
@@ -462,7 +501,9 @@
<dependency groupId="com.googlecode.concurrent-trees" artifactId="concurrent-trees" version="2.4.0" />
<dependency groupId="com.github.ben-manes.caffeine" artifactId="caffeine" version="2.3.5" />
<dependency groupId="org.jctools" artifactId="jctools-core" version="1.2.1"/>
- <dependency groupId="org.ow2.asm" artifactId="asm" version="5.0.4" />
+ <dependency groupId="org.ow2.asm" artifactId="asm" version="6.2" />
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440825
--- Diff: build.xml ---
@@ -703,6 +744,13 @@
</patternset>
<mapper type="flatten"/>
</unzip>
+
+ <delete>
--- End diff --
Otherwise an older asm (and previously an older netty version as well) landed in `build/lib/jars` via a transitive dependency.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199259098
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -67,23 +71,84 @@
public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+ private static MethodHandle mhDirectBufferAddress;
+
static
{
boolean canClean = false;
try
{
+ clsDirectBuffer = Class.forName("sun.nio.ch.DirectBuffer");
+ Method mDirectBufferCleaner = clsDirectBuffer.getMethod("cleaner");
+ mhDirectBufferCleaner = MethodHandles.lookup().unreflect(mDirectBufferCleaner);
+ Method mCleanerClean = mDirectBufferCleaner.getReturnType().getMethod("clean");
+ mhCleanerClean = MethodHandles.lookup().unreflect(mCleanerClean);
+ Method mDirectBufferAddress = clsDirectBuffer.getMethod("address");
+ mhDirectBufferAddress = MethodHandles.lookup().unreflect(mDirectBufferAddress);
+
ByteBuffer buf = ByteBuffer.allocateDirect(1);
- ((DirectBuffer) buf).cleaner().clean();
+ cleanerClean(buf);
canClean = true;
}
catch (Throwable t)
{
+ logger.info("Cannot initialize un-mmaper. Compacted data files will not be removed promptly.", t);
JVMStabilityInspector.inspectThrowable(t);
- logger.info("Cannot initialize un-mmaper. (Are you using a non-Oracle JVM?) Compacted data files will not be removed promptly. Consider using an Oracle JVM or using standard disk access mode");
}
isCleanerAvailable = canClean;
}
+ public static boolean isDirectBuffer(ByteBuffer buf)
+ {
+ return clsDirectBuffer.isInstance(buf);
--- End diff --
Can't we just call `ByteBuffer.isDirect()` instead?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204380932
--- Diff: src/java/org/apache/cassandra/cql3/functions/JavaBasedUDFunction.java ---
@@ -591,14 +592,17 @@ private NameEnvironmentAnswer findType(String className)
String resourceName = className.replace('.', '/') + ".class";
- try (InputStream is = UDFunction.udfClassLoader.getResourceAsStream(resourceName))
+ try
--- End diff --
Ah - I've changed that part twice. But yea, one try-catch is absolutely sufficient.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199260315
--- Diff: src/java/org/apache/cassandra/security/ThreadAwareSecurityManager.java ---
@@ -208,7 +225,5 @@ public void checkPackageAccess(String pkg)
RuntimePermission perm = new RuntimePermission("accessClassInPackage." + pkg);
throw new AccessControlException("access denied: " + perm, perm);
}
-
- super.checkPackageAccess(pkg);
--- End diff --
Why don't we need to call `super.checkPackageAccess()` anymore?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204211370
--- Diff: conf/jvm11-clients.options ---
@@ -0,0 +1,21 @@
+###########################################################################
+# jvm11-clients.options #
+# #
+# See jvm-clients.options. This file is specific for Java 8 and newer. #
--- End diff --
nit: this should say `for Java 11 and newer`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra issue #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on the issue:
https://github.com/apache/cassandra/pull/236
Committed as sha `6ba2fb9395226491872b41312d978a169f36fcdb`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199260807
--- Diff: src/java/org/apache/cassandra/utils/NativeLibrary.java ---
@@ -78,7 +77,14 @@
static
{
FILE_DESCRIPTOR_FD_FIELD = FBUtilities.getProtectedField(FileDescriptor.class, "fd");
- FILE_CHANNEL_FD_FIELD = FBUtilities.getProtectedField(FileChannelImpl.class, "fd");
+ try
--- End diff --
why is this in a try/catch block now?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440809
--- Diff: conf/cassandra-env.sh ---
@@ -86,50 +86,36 @@ calculate_heap_sizes()
fi
}
-# Determine the sort of JVM we'll be running on.
-java_ver_output=`"${JAVA:-java}" -version 2>&1`
-jvmver=`echo "$java_ver_output" | grep '[openjdk|java] version' | awk -F'"' 'NR==1 {print $2}' | cut -d\- -f1`
-JVM_VERSION=${jvmver%_*}
-JVM_PATCH_VERSION=${jvmver#*_}
-
-if [ "$JVM_VERSION" \< "1.8" ] ; then
- echo "Cassandra 3.0 and later require Java 8u40 or later."
- exit 1;
-fi
-
-if [ "$JVM_VERSION" \< "1.8" ] && [ "$JVM_PATCH_VERSION" -lt 40 ] ; then
- echo "Cassandra 3.0 and later require Java 8u40 or later."
- exit 1;
-fi
-
-jvm=`echo "$java_ver_output" | grep -A 1 '[openjdk|java] version' | awk 'NR==2 {print $1}'`
-case "$jvm" in
- OpenJDK)
- JVM_VENDOR=OpenJDK
- # this will be "64-Bit" or "32-Bit"
- JVM_ARCH=`echo "$java_ver_output" | awk 'NR==3 {print $2}'`
- ;;
- "Java(TM)")
- JVM_VENDOR=Oracle
- # this will be "64-Bit" or "32-Bit"
- JVM_ARCH=`echo "$java_ver_output" | awk 'NR==3 {print $3}'`
- ;;
- *)
- # Help fill in other JVM values
- JVM_VENDOR=other
- JVM_ARCH=unknown
- ;;
-esac
-
#GC log path has to be defined here because it needs to access CASSANDRA_HOME
-JVM_OPTS="$JVM_OPTS -Xloggc:${CASSANDRA_HOME}/logs/gc.log"
+if [ $JAVA_VERSION -ge 11 ] ; then
+ # See description of https://bugs.openjdk.java.net/browse/JDK-8046148 for details about the syntax
+ # The following is the equivalent to -XX:+PrintGCDetails -XX:+UseGCLogFileRotation -XX:NumberOfGCLogFiles=10 -XX:GCLogFileSize=10M
+ if ! grep -q "^-[X]log:gc" $CASSANDRA_CONF/jvm.options ; then # [X] to prevent ccm from replacing this line
+ # only add -Xlog:gc if it's not mentioned in jvm.options file
+ mkdir -p ${CASSANDRA_HOME}/logs
+ JVM_OPTS="$JVM_OPTS -Xlog:gc=info,heap*=trace,age*=debug,safepoint=info,promotion*=trace:file=${CASSANDRA_HOME}/logs/gc.log:time,uptime,pid,tid,level:filecount=10,filesize=10240"
--- End diff --
nice catch!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204355461
--- Diff: build.xml ---
@@ -66,8 +67,14 @@
<property name="doc.dir" value="${basedir}/doc"/>
- <property name="source.version" value="1.8"/>
- <property name="target.version" value="1.8"/>
+ <!--
+ We specific '8' instead of '1.8' in source.version to indicate that that this build requires Java 11 _and_ Java 8.
--- End diff --
petty nit: `We specify`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204380974
--- Diff: build.xml ---
@@ -1665,7 +1763,7 @@
<echo message="Mem size : ${mem.size}"/>
</target>
- <target name="test" depends="build-test,get-cores,get-mem,stress-build" description="Parallel Test Runner">
+ <target name="test" depends="build-test" description="Parallel Test Runner">
--- End diff --
Oops, re-added
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown closed the pull request at:
https://github.com/apache/cassandra/pull/236
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204372219
--- Diff: src/java/org/apache/cassandra/utils/NativeLibrary.java ---
@@ -78,7 +77,14 @@
static
{
FILE_DESCRIPTOR_FD_FIELD = FBUtilities.getProtectedField(FileDescriptor.class, "fd");
- FILE_CHANNEL_FD_FIELD = FBUtilities.getProtectedField(FileChannelImpl.class, "fd");
+ try
--- End diff --
ahh, ok missed that one
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440821
--- Diff: conf/jvm11.options ---
@@ -0,0 +1,89 @@
+###########################################################################
+# jvm11.options #
+# #
+# See jvm.options. This file is specific for Java 11 and newer. #
+###########################################################################
+
+#################
+# GC SETTINGS #
+#################
+
+
+
+### CMS Settings
+#-XX:+UseParNewGC
+#-XX:+UseConcMarkSweepGC
+#-XX:+CMSParallelRemarkEnabled
+#-XX:SurvivorRatio=8
+#-XX:MaxTenuringThreshold=1
+#-XX:CMSInitiatingOccupancyFraction=75
+#-XX:+UseCMSInitiatingOccupancyOnly
+#-XX:CMSWaitDuration=10000
+#-XX:+CMSParallelInitialMarkEnabled
+#-XX:+CMSEdenChunksRecordAlways
+## some JVMs will fill up their heap when accessed via JMX, see CASSANDRA-6541
+#-XX:+CMSClassUnloadingEnabled
+
+
+
+### G1 Settings
+## Use the Hotspot garbage-first collector.
+-XX:+UseG1GC
+-XX:+ParallelRefProcEnabled
+
+#
+## Have the JVM do less remembered set work during STW, instead
+## preferring concurrent GC. Reduces p99.9 latency.
+-XX:G1RSetUpdatingPauseTimePercent=5
+#
+## Main G1GC tunable: lowering the pause target will lower throughput and vise versa.
+## 200ms is the JVM default and lowest viable setting
+## 1000ms increases throughput. Keep it smaller than the timeouts in cassandra.yaml.
+-XX:MaxGCPauseMillis=500
+
+## Optional G1 Settings
+# Save CPU time on large (>= 16GB) heaps by delaying region scanning
+# until the heap is 70% full. The default in Hotspot 8u40 is 40%.
+#-XX:InitiatingHeapOccupancyPercent=70
+
+# For systems with > 8 cores, the default ParallelGCThreads is 5/8 the number of logical cores.
+# Otherwise equal to the number of cores when 8 or less.
+# Machines with > 10 cores should try setting these to <= full cores.
+#-XX:ParallelGCThreads=16
+# By default, ConcGCThreads is 1/4 of ParallelGCThreads.
+# Setting both to the same value can reduce STW durations.
+#-XX:ConcGCThreads=16
+
+
+### JPMS
+
+-Djdk.attach.allowAttachSelf=true
+--add-exports java.base/jdk.internal.misc=ALL-UNNAMED
+--add-opens java.base/jdk.internal.module=ALL-UNNAMED
+--add-exports java.base/jdk.internal.ref=ALL-UNNAMED
+--add-exports java.base/sun.nio.ch=ALL-UNNAMED
+--add-exports java.management.rmi/com.sun.jmx.remote.internal.rmi=ALL-UNNAMED
+--add-exports java.rmi/sun.rmi.registry=ALL-UNNAMED
+--add-exports java.rmi/sun.rmi.server=ALL-UNNAMED
+--add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED
+
+
+### GC logging options -- uncomment to enable
+
+# Java 11 (and newer) GC logging options:
+# See description of https://bugs.openjdk.java.net/browse/JDK-8046148 for details about the syntax
+# The following is the equivalent to -XX:+PrintGCDetails -XX:+UseGCLogFileRotation -XX:NumberOfGCLogFiles=10 -XX:GCLogFileSize=10M
+#-Xlog:gc=info,heap*=trace,age*=debug,safepoint=info,promotion*=trace:file=/var/log/cassandra/gc.log:time,uptime,pid,tid,level:filecount=10,filesize=10240
--- End diff --
nice catch!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204370957
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -350,13 +396,38 @@ public static String getRelativePath(String basePath, String path)
public static void clean(ByteBuffer buffer)
{
- if (buffer == null)
+ if (buffer == null || !buffer.isDirect())
return;
- if (isCleanerAvailable && buffer.isDirect())
+
+ // TODO Once we can get rid of Java 8, it's simpler to call sun.misc.Unsafe.invokeCleaner(ByteBuffer),
+ // but need to take care of the attachment handling (i.e. whether 'buf' is a duplicate or slice) - that
+ // is different in sun.misc.Unsafe.invokeCleaner and this implementation.
+
+ try
{
- DirectBuffer db = (DirectBuffer) buffer;
- if (db.cleaner() != null)
- db.cleaner().clean();
+ if (buffer.isDirect())
+ {
+ Object cleaner = mhDirectBufferCleaner.bindTo(buffer).invoke();
--- End diff --
Can you add some comments here? It's not clear what's going on, but more importantly, why there's two code paths.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440701
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -106,11 +171,57 @@ public static void createHardLink(File from, File to)
}
}
+ private static final File tempDir = new File(System.getProperty("java.io.tmpdir"));
+ private static final AtomicLong tempFileNum = new AtomicLong();
+
+ public static File getTempDir()
+ {
+ return tempDir;
+ }
+
+ /**
+ * Pretty much like {@link File#createTempFile(String, String, File)}, but with
+ * the guarantee that the "random" part of the generated file name between
+ * {@code prefix} and {@code suffix} is a positive, increasing {@code long} value.
+ */
public static File createTempFile(String prefix, String suffix, File directory)
{
try
{
- return File.createTempFile(prefix, suffix, directory);
+ // Do not use java.io.File.createTempFile(), because some tests rely on the
--- End diff --
Right, I think that's reasonable.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440768
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -67,23 +71,84 @@
public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+ private static MethodHandle mhDirectBufferAddress;
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204369509
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -350,13 +396,38 @@ public static String getRelativePath(String basePath, String path)
public static void clean(ByteBuffer buffer)
{
- if (buffer == null)
+ if (buffer == null || !buffer.isDirect())
return;
- if (isCleanerAvailable && buffer.isDirect())
+
+ // TODO Once we can get rid of Java 8, it's simpler to call sun.misc.Unsafe.invokeCleaner(ByteBuffer),
+ // but need to take care of the attachment handling (i.e. whether 'buf' is a duplicate or slice) - that
+ // is different in sun.misc.Unsafe.invokeCleaner and this implementation.
+
+ try
{
- DirectBuffer db = (DirectBuffer) buffer;
- if (db.cleaner() != null)
- db.cleaner().clean();
+ if (buffer.isDirect())
--- End diff --
This check is redundant as you've already performed the `!buffer.isDirect()` earlier in this method (and bailed if it isn't)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440706
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -67,23 +71,84 @@
public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+ private static MethodHandle mhDirectBufferAddress;
+
static
{
boolean canClean = false;
try
{
+ clsDirectBuffer = Class.forName("sun.nio.ch.DirectBuffer");
+ Method mDirectBufferCleaner = clsDirectBuffer.getMethod("cleaner");
+ mhDirectBufferCleaner = MethodHandles.lookup().unreflect(mDirectBufferCleaner);
+ Method mCleanerClean = mDirectBufferCleaner.getReturnType().getMethod("clean");
+ mhCleanerClean = MethodHandles.lookup().unreflect(mCleanerClean);
+ Method mDirectBufferAddress = clsDirectBuffer.getMethod("address");
+ mhDirectBufferAddress = MethodHandles.lookup().unreflect(mDirectBufferAddress);
+
ByteBuffer buf = ByteBuffer.allocateDirect(1);
- ((DirectBuffer) buf).cleaner().clean();
+ cleanerClean(buf);
canClean = true;
}
catch (Throwable t)
{
+ logger.info("Cannot initialize un-mmaper. Compacted data files will not be removed promptly.", t);
JVMStabilityInspector.inspectThrowable(t);
- logger.info("Cannot initialize un-mmaper. (Are you using a non-Oracle JVM?) Compacted data files will not be removed promptly. Consider using an Oracle JVM or using standard disk access mode");
}
isCleanerAvailable = canClean;
}
+ public static boolean isDirectBuffer(ByteBuffer buf)
+ {
+ return clsDirectBuffer.isInstance(buf);
+ }
+
+ public static void cleanerClean(ByteBuffer buf)
+ {
+ cleanerClean(buf, false);
+ }
+
+ public static void cleanerClean(ByteBuffer buf, boolean cleanAttachment)
--- End diff --
yikes - that was wrong. good catch!
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199253772
--- Diff: conf/jvm11.options ---
@@ -0,0 +1,89 @@
+###########################################################################
+# jvm11.options #
+# #
+# See jvm.options. This file is specific for Java 11 and newer. #
+###########################################################################
+
+#################
+# GC SETTINGS #
+#################
+
+
+
+### CMS Settings
+#-XX:+UseParNewGC
+#-XX:+UseConcMarkSweepGC
+#-XX:+CMSParallelRemarkEnabled
+#-XX:SurvivorRatio=8
+#-XX:MaxTenuringThreshold=1
+#-XX:CMSInitiatingOccupancyFraction=75
+#-XX:+UseCMSInitiatingOccupancyOnly
+#-XX:CMSWaitDuration=10000
+#-XX:+CMSParallelInitialMarkEnabled
+#-XX:+CMSEdenChunksRecordAlways
+## some JVMs will fill up their heap when accessed via JMX, see CASSANDRA-6541
+#-XX:+CMSClassUnloadingEnabled
+
+
+
+### G1 Settings
+## Use the Hotspot garbage-first collector.
+-XX:+UseG1GC
+-XX:+ParallelRefProcEnabled
+
+#
+## Have the JVM do less remembered set work during STW, instead
+## preferring concurrent GC. Reduces p99.9 latency.
+-XX:G1RSetUpdatingPauseTimePercent=5
+#
+## Main G1GC tunable: lowering the pause target will lower throughput and vise versa.
+## 200ms is the JVM default and lowest viable setting
+## 1000ms increases throughput. Keep it smaller than the timeouts in cassandra.yaml.
+-XX:MaxGCPauseMillis=500
+
+## Optional G1 Settings
+# Save CPU time on large (>= 16GB) heaps by delaying region scanning
+# until the heap is 70% full. The default in Hotspot 8u40 is 40%.
+#-XX:InitiatingHeapOccupancyPercent=70
+
+# For systems with > 8 cores, the default ParallelGCThreads is 5/8 the number of logical cores.
+# Otherwise equal to the number of cores when 8 or less.
+# Machines with > 10 cores should try setting these to <= full cores.
+#-XX:ParallelGCThreads=16
+# By default, ConcGCThreads is 1/4 of ParallelGCThreads.
+# Setting both to the same value can reduce STW durations.
+#-XX:ConcGCThreads=16
+
+
+### JPMS
+
+-Djdk.attach.allowAttachSelf=true
+--add-exports java.base/jdk.internal.misc=ALL-UNNAMED
+--add-opens java.base/jdk.internal.module=ALL-UNNAMED
+--add-exports java.base/jdk.internal.ref=ALL-UNNAMED
+--add-exports java.base/sun.nio.ch=ALL-UNNAMED
+--add-exports java.management.rmi/com.sun.jmx.remote.internal.rmi=ALL-UNNAMED
+--add-exports java.rmi/sun.rmi.registry=ALL-UNNAMED
+--add-exports java.rmi/sun.rmi.server=ALL-UNNAMED
+--add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED
+
+
+### GC logging options -- uncomment to enable
+
+# Java 11 (and newer) GC logging options:
+# See description of https://bugs.openjdk.java.net/browse/JDK-8046148 for details about the syntax
+# The following is the equivalent to -XX:+PrintGCDetails -XX:+UseGCLogFileRotation -XX:NumberOfGCLogFiles=10 -XX:GCLogFileSize=10M
+#-Xlog:gc=info,heap*=trace,age*=debug,safepoint=info,promotion*=trace:file=/var/log/cassandra/gc.log:time,uptime,pid,tid,level:filecount=10,filesize=10240
--- End diff --
minor nit: `filesize=10240` is the file size in bytes. change to `10485760` if you actually want 10MB files.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440796
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -67,23 +71,84 @@
public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+ private static MethodHandle mhDirectBufferAddress;
+
static
{
boolean canClean = false;
try
{
+ clsDirectBuffer = Class.forName("sun.nio.ch.DirectBuffer");
+ Method mDirectBufferCleaner = clsDirectBuffer.getMethod("cleaner");
+ mhDirectBufferCleaner = MethodHandles.lookup().unreflect(mDirectBufferCleaner);
+ Method mCleanerClean = mDirectBufferCleaner.getReturnType().getMethod("clean");
+ mhCleanerClean = MethodHandles.lookup().unreflect(mCleanerClean);
+ Method mDirectBufferAddress = clsDirectBuffer.getMethod("address");
+ mhDirectBufferAddress = MethodHandles.lookup().unreflect(mDirectBufferAddress);
+
ByteBuffer buf = ByteBuffer.allocateDirect(1);
- ((DirectBuffer) buf).cleaner().clean();
+ cleanerClean(buf);
canClean = true;
}
catch (Throwable t)
{
+ logger.info("Cannot initialize un-mmaper. Compacted data files will not be removed promptly.", t);
JVMStabilityInspector.inspectThrowable(t);
- logger.info("Cannot initialize un-mmaper. (Are you using a non-Oracle JVM?) Compacted data files will not be removed promptly. Consider using an Oracle JVM or using standard disk access mode");
}
isCleanerAvailable = canClean;
}
+ public static boolean isDirectBuffer(ByteBuffer buf)
+ {
+ return clsDirectBuffer.isInstance(buf);
--- End diff --
sure
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440779
--- Diff: src/java/org/apache/cassandra/cql3/functions/JavaBasedUDFunction.java ---
@@ -591,14 +612,48 @@ private NameEnvironmentAnswer findType(String className)
String resourceName = className.replace('.', '/') + ".class";
- try (InputStream is = UDFunction.udfClassLoader.getResourceAsStream(resourceName))
+ // up to Java 8:
+ // returns a non-null InputStream for class files
+ // since Java 11:
+ // returns a non-null InputStream for class files for application classes
+ // returns null for class files for system modules (e.g. java.base)
+ try
{
- if (is != null)
+ InputStream is = UDFunction.udfClassLoader.getResourceAsStream(resourceName);
+ try
+ {
+ if (is == null)
+ {
+ // For Java 11 try to see whether the class actually exists and read the
+ // class file data via the class' module. (This is necessary at least
+ // for 9-ea build 123)
--- End diff --
Hm - seems that was actually a bug in Java and it's been fixed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204368353
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -64,24 +68,31 @@
public static final long ONE_TB = 1024 * ONE_GB;
private static final DecimalFormat df = new DecimalFormat("#.##");
- public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+
static
{
- boolean canClean = false;
try
{
+ clsDirectBuffer = Class.forName("sun.nio.ch.DirectBuffer");
+ Method mDirectBufferCleaner = clsDirectBuffer.getMethod("cleaner");
+ mhDirectBufferCleaner = MethodHandles.lookup().unreflect(mDirectBufferCleaner);
+ Method mCleanerClean = mDirectBufferCleaner.getReturnType().getMethod("clean");
+ mhCleanerClean = MethodHandles.lookup().unreflect(mCleanerClean);
+
ByteBuffer buf = ByteBuffer.allocateDirect(1);
- ((DirectBuffer) buf).cleaner().clean();
- canClean = true;
+ clean(buf);
}
catch (Throwable t)
{
+ logger.info("Cannot initialize optimized memory deallocator. Some data, both in-memory and on-disk, may live longer due to garbage collection.");
JVMStabilityInspector.inspectThrowable(t);
- logger.info("Cannot initialize un-mmaper. (Are you using a non-Oracle JVM?) Compacted data files will not be removed promptly. Consider using an Oracle JVM or using standard disk access mode");
+ throw new RuntimeException(t);
--- End diff --
So, this is a change in semantics (throwing a RuntimeException). Previously, we would just complain if we couldn't get to the buffer cleaner, now we're basically terminating the process. Is that intended? If it is, at minimum the log statement a few lines up should be `FATAL`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r195913415
--- Diff: build.xml ---
@@ -462,7 +501,9 @@
<dependency groupId="com.googlecode.concurrent-trees" artifactId="concurrent-trees" version="2.4.0" />
<dependency groupId="com.github.ben-manes.caffeine" artifactId="caffeine" version="2.3.5" />
<dependency groupId="org.jctools" artifactId="jctools-core" version="1.2.1"/>
- <dependency groupId="org.ow2.asm" artifactId="asm" version="5.0.4" />
+ <dependency groupId="org.ow2.asm" artifactId="asm" version="6.2" />
--- End diff --
nit: maybe introduce a property with the version for `org.ow2.asm`, like what you've done for `ecj.version` and others?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440790
--- Diff: src/java/org/apache/cassandra/security/ThreadAwareSecurityManager.java ---
@@ -208,7 +225,5 @@ public void checkPackageAccess(String pkg)
RuntimePermission perm = new RuntimePermission("accessClassInPackage." + pkg);
throw new AccessControlException("access denied: " + perm, perm);
}
-
- super.checkPackageAccess(pkg);
--- End diff --
Otherwise JavaScript based UDFs stop working due to JPMS, that would raise errors like
`Caused by: java.security.AccessControlException: access denied ("java.lang.RuntimePermission" "accessClassInPackage.jdk.internal.org.objectweb.asm")`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r195913429
--- Diff: build.xml ---
@@ -703,6 +744,13 @@
</patternset>
<mapper type="flatten"/>
</unzip>
+
+ <delete>
--- End diff --
why do you need to delete the netty and asm jars here?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r204369082
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -106,11 +117,46 @@ public static void createHardLink(File from, File to)
}
}
+ private static final File tempDir = new File(System.getProperty("java.io.tmpdir"));
+ private static final AtomicLong tempFileNum = new AtomicLong();
+
+ public static File getTempDir()
+ {
+ return tempDir;
+ }
+
+ /**
+ * Pretty much like {@link File#createTempFile(String, String, File)}, but with
+ * the guarantee that the "random" part of the generated file name between
+ * {@code prefix} and {@code suffix} is a positive, increasing {@code long} value.
+ */
public static File createTempFile(String prefix, String suffix, File directory)
{
try
{
- return File.createTempFile(prefix, suffix, directory);
+ // Do not use java.io.File.createTempFile(), because some tests rely on the
+ // behavior that the "random" part in the temp file name is a positive 'long'.
+ // However, at least since Java 9 the code to generate the "random" part
+ // uses an _unsigned_ random long generated like this:
+ // Long.toUnsignedString(new java.util.Random.nextLong())
+
+ while (true)
+ {
+ // The contract of File.createTempFile() says, that it must not return
+ // the same file name again. We do that here in a very simple way,
+ // that probably doesn't cover all edge cases. Just rely on system
+ // wall clock and return strictly increasing values from that.
+ long num = tempFileNum.getAndIncrement();
+
+ // We have a positive long here, which is safe to use for example
+ // for CommitLogTest.
+ String timePart = Long.toString(num);
--- End diff --
rename this variable, or maybe just move `Long.toString(num)` into the next line.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by snazy <gi...@git.apache.org>.
Github user snazy commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199440838
--- Diff: build.xml ---
@@ -110,16 +118,35 @@
<property name="jacoco.finalexecfile" value="${jacoco.export.dir}/jacoco.exec" />
<property name="jacoco.version" value="0.7.5.201505241946"/>
- <property name="byteman.version" value="3.0.3"/>
+ <property name="byteman.version" value="4.0.2"/>
+ <property name="jamm.version" value="0.3.2"/>
+ <property name="ecj.version" value="4.6.1"/>
+ <property name="ohc.version" value="0.5.1"/>
+
+ <!--
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] cassandra pull request #236: 9608 trunk
Posted by jasobrown <gi...@git.apache.org>.
Github user jasobrown commented on a diff in the pull request:
https://github.com/apache/cassandra/pull/236#discussion_r199330747
--- Diff: src/java/org/apache/cassandra/io/util/FileUtils.java ---
@@ -67,23 +71,84 @@
public static final boolean isCleanerAvailable;
private static final AtomicReference<Optional<FSErrorHandler>> fsErrorHandler = new AtomicReference<>(Optional.empty());
+ private static Class clsDirectBuffer;
+ private static MethodHandle mhDirectBufferCleaner;
+ private static MethodHandle mhCleanerClean;
+ private static MethodHandle mhDirectBufferAddress;
+
static
{
boolean canClean = false;
try
{
+ clsDirectBuffer = Class.forName("sun.nio.ch.DirectBuffer");
+ Method mDirectBufferCleaner = clsDirectBuffer.getMethod("cleaner");
+ mhDirectBufferCleaner = MethodHandles.lookup().unreflect(mDirectBufferCleaner);
+ Method mCleanerClean = mDirectBufferCleaner.getReturnType().getMethod("clean");
+ mhCleanerClean = MethodHandles.lookup().unreflect(mCleanerClean);
+ Method mDirectBufferAddress = clsDirectBuffer.getMethod("address");
+ mhDirectBufferAddress = MethodHandles.lookup().unreflect(mDirectBufferAddress);
+
ByteBuffer buf = ByteBuffer.allocateDirect(1);
- ((DirectBuffer) buf).cleaner().clean();
+ cleanerClean(buf);
canClean = true;
}
catch (Throwable t)
{
+ logger.info("Cannot initialize un-mmaper. Compacted data files will not be removed promptly.", t);
JVMStabilityInspector.inspectThrowable(t);
- logger.info("Cannot initialize un-mmaper. (Are you using a non-Oracle JVM?) Compacted data files will not be removed promptly. Consider using an Oracle JVM or using standard disk access mode");
}
isCleanerAvailable = canClean;
}
+ public static boolean isDirectBuffer(ByteBuffer buf)
+ {
+ return clsDirectBuffer.isInstance(buf);
+ }
+
+ public static void cleanerClean(ByteBuffer buf)
+ {
+ cleanerClean(buf, false);
+ }
+
+ public static void cleanerClean(ByteBuffer buf, boolean cleanAttachment)
--- End diff --
I don't see any code path where `cleanAttachment` can be true. Am I missing something?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org