This is an automated email from the ASF dual-hosted git repository.
bdeggleston pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new 9063cea Fix AbstractBTreePartition locking in java 11
9063cea is described below
commit 9063ceabc9a41825a09db811f74821537dfe726b
Author: Blake Eggleston <[email protected]>
AuthorDate: Tue Mar 12 20:46:37 2019 -0700
Fix AbstractBTreePartition locking in java 11
Patch by Blake Eggleston; Reviewed by Benedict Elliot Smith for
CASSANDRA-14607
---
CHANGES.txt | 1 +
build.xml | 43 +++-----
ide/idea-iml-file.xml | 1 -
.../db/partitions/AtomicBTreePartition.java | 109 +++++++++++----------
.../db/partitions/AtomicBTreePartitionBase.java | 65 ------------
.../db/partitions/AtomicBTreePartitionBase.java | 81 ---------------
6 files changed, 71 insertions(+), 229 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index 1cfbbfc..131bcb2 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.0
+ * Fix AbstractBTreePartition locking in java 11 (CASSANDRA-14607)
* SimpleClient should pass connection properties as options (CASSANDRA-15056)
* Set repaired data tracking flag on range reads if enabled (CASSANDRA-15019)
* Calculate pending ranges for BOOTSTRAP_REPLACE correctly (CASSANDRA-14802)
diff --git a/build.xml b/build.xml
index 8fcb53e..6d79aca 100644
--- a/build.xml
+++ b/build.xml
@@ -35,8 +35,6 @@
<property name="build.src" value="${basedir}/src"/>
<property name="build.src.java" value="${basedir}/src/java"/>
<property name="build.src.antlr" value="${basedir}/src/antlr"/>
- <property name="build.src.java8" value="${basedir}/src/java8"/>
- <property name="build.src.java11" value="${basedir}/src/java11"/>
<property name="build.src.resources" value="${basedir}/src/resources"/>
<property name="build.src.gen-java" value="${basedir}/src/gen-java"/>
<property name="build.lib" value="${basedir}/lib"/>
@@ -811,14 +809,13 @@
depends="maven-ant-tasks-retrieve-build,build-project"
description="Compile Cassandra classes"/>
<target name="codecoverage" depends="jacoco-run,jacoco-report"
description="Create code coverage report"/>
- <target name="_build_java8_only" if="java.version.8">
+ <target name="_build_java8" if="java.version.8">
<echo message="Compiling only for Java 8 ..."/>
<javac fork="true"
debug="true" debuglevel="${debuglevel}" encoding="utf-8"
destdir="${build.classes.main}" includeantruntime="false"
source="8" target="8"
memorymaximumsize="512M">
<src path="${build.src.java}"/>
- <src path="${build.src.java8}"/>
<src path="${build.src.gen-java}"/>
<compilerarg value="-XDignore.symbol.file"/>
<classpath>
@@ -826,9 +823,12 @@
</classpath>
</javac>
</target>
- <target name="_build_multi_java" unless="java.version.8">
+ <target name="_build_java11" unless="java.version.8">
<!-- Note: we cannot use javac's 'release' option, as that does not
allow accessing sun.misc.Unsafe nor
Nashorn's ClassFilter class as any javac modules option is invalid for
relase 8. -->
+ <fail message="JAVA8_HOME env variable must be set when building with
java >= 11">
+ <condition><not><isset
property="env.JAVA8_HOME"/></not></condition>
+ </fail>
<echo message="Compiling for Java 8 (using
${env.JAVA8_HOME}/bin/javac) ..."/>
<javac fork="true"
debug="true" debuglevel="${debuglevel}" encoding="utf-8"
@@ -836,33 +836,19 @@
executable="${env.JAVA8_HOME}/bin/javac"
memorymaximumsize="512M">
<src path="${build.src.java}"/>
- <src path="${build.src.java8}"/>
<src path="${build.src.gen-java}"/>
<compilerarg value="-XDignore.symbol.file"/>
<classpath>
<path refid="cassandra.classpath"/>
</classpath>
</javac>
- <echo message="Compiling for current java version ..."/>
- <javac fork="true"
- debug="true" debuglevel="${debuglevel}" encoding="utf-8"
- destdir="${build.classes.main}/META-INF/versions/11"
includeantruntime="false"
- memorymaximumsize="512M">
- <src path="${build.src.java11}"/>
- <compilerarg value="--release"/>
- <compilerarg value="${release.version}"/>
- <compilerarg value="-XDignore.symbol.file"/>
- <classpath>
- <path refid="cassandra.classpath"/>
- </classpath>
- </javac>
</target>
<target
depends="init,gen-cql3-grammar,generate-cql-html,generate-jflex-java"
name="build-project">
<echo message="${ant.project.name}: ${ant.file}"/>
<!-- Order matters! -->
- <antcall target="_build_java8_only"/>
- <antcall target="_build_multi_java"/>
+ <antcall target="_build_java8"/>
+ <antcall target="_build_java11"/>
<antcall target="createVersionPropFile"/>
<copy todir="${build.classes.main}">
<fileset dir="${build.src.resources}" />
@@ -1032,9 +1018,6 @@
<fileset dir="${build.src.java}" defaultexcludes="yes">
<include name="org/apache/**/*.java"/>
</fileset>
- <fileset dir="${build.src.java11}" defaultexcludes="yes">
- <include name="org/apache/**/*.java"/>
- </fileset>
<fileset dir="${build.src.gen-java}" defaultexcludes="yes">
<include name="org/apache/**/*.java"/>
</fileset>
@@ -1056,9 +1039,6 @@
<fileset dir="${build.src.java}" defaultexcludes="yes">
<include name="org/apache/**/*.java"/>
</fileset>
- <fileset dir="${build.src.java11}" defaultexcludes="yes">
- <include name="org/apache/**/*.java"/>
- </fileset>
<fileset dir="${build.src.gen-java}" defaultexcludes="yes">
<include name="org/apache/**/*.java"/>
</fileset>
@@ -1234,11 +1214,11 @@
</target>
<target name="build-test" depends="_main-jar, stress-build, fqltool-build,
write-poms" description="Compile test classes">
- <antcall target="_build-test_java8_only"/>
- <antcall target="_build-test_multi_java"/>
+ <antcall target="_build-test_java8"/>
+ <antcall target="_build-test_java11"/>
</target>
- <target name="_build-test_java8_only" if="java.version.8">
+ <target name="_build-test_java8" if="java.version.8">
<javac
fork="true"
compiler="modern"
@@ -1266,7 +1246,7 @@
</copy>
</target>
- <target name="_build-test_multi_java" unless="java.version.8">
+ <target name="_build-test_java11" unless="java.version.8">
<javac
fork="true"
compiler="modern"
@@ -2032,7 +2012,6 @@
<echo file=".classpath"><![CDATA[<?xml version="1.0" encoding="UTF-8"?>
<classpath>
<classpathentry kind="src" path="src/java"/>
- <classpathentry kind="src" path="src/java8"/>
<classpathentry kind="src" path="src/resources"/>
<classpathentry kind="src" path="src/gen-java"/>
<classpathentry kind="src" path="conf" including="hotspot_compiler"/>
diff --git a/ide/idea-iml-file.xml b/ide/idea-iml-file.xml
index 0045ae6..17d4c5b 100644
--- a/ide/idea-iml-file.xml
+++ b/ide/idea-iml-file.xml
@@ -24,7 +24,6 @@
<exclude-output />
<content url="file://$MODULE_DIR$">
<sourceFolder url="file://$MODULE_DIR$/src/java"
isTestSource="false" />
- <sourceFolder url="file://$MODULE_DIR$/src/java8"
isTestSource="false" />
<sourceFolder url="file://$MODULE_DIR$/src/gen-java"
isTestSource="false" generated="true" />
<sourceFolder url="file://$MODULE_DIR$/src/resources"
type="java-resource" />
<sourceFolder url="file://$MODULE_DIR$/tools/stress/src"
isTestSource="false" />
diff --git
a/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java
b/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java
index 8c62642..ec73814 100644
--- a/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java
+++ b/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java
@@ -47,7 +47,7 @@ import org.apache.cassandra.utils.memory.MemtableAllocator;
* other thread can see the state where only parts but not all rows have
* been added.
*/
-public final class AtomicBTreePartition extends AtomicBTreePartitionBase
+public final class AtomicBTreePartition extends AbstractBTreePartition
{
public static final long EMPTY_SIZE = ObjectSizes.measure(new
AtomicBTreePartition(null,
DatabaseDescriptor.getPartitioner().decorateKey(ByteBuffer.allocate(1)),
@@ -108,6 +108,50 @@ public final class AtomicBTreePartition extends
AtomicBTreePartitionBase
return true;
}
+ private long[] addAllWithSizeDeltaInternal(RowUpdater updater,
PartitionUpdate update, UpdateTransaction indexer)
+ {
+ Holder current = ref;
+ updater.ref = current;
+ updater.reset();
+
+ if (!update.deletionInfo().getPartitionDeletion().isLive())
+
indexer.onPartitionDeletion(update.deletionInfo().getPartitionDeletion());
+
+ if (update.deletionInfo().hasRanges())
+
update.deletionInfo().rangeIterator(false).forEachRemaining(indexer::onRangeTombstone);
+
+ DeletionInfo deletionInfo;
+ if (update.deletionInfo().mayModify(current.deletionInfo))
+ {
+ if (updater.inputDeletionInfoCopy == null)
+ updater.inputDeletionInfoCopy =
update.deletionInfo().copy(HeapAllocator.instance);
+
+ deletionInfo =
current.deletionInfo.mutableCopy().add(updater.inputDeletionInfoCopy);
+ updater.allocated(deletionInfo.unsharedHeapSize() -
current.deletionInfo.unsharedHeapSize());
+ }
+ else
+ {
+ deletionInfo = current.deletionInfo;
+ }
+
+ RegularAndStaticColumns columns =
update.columns().mergeTo(current.columns);
+ Row newStatic = update.staticRow();
+ Row staticRow = newStatic.isEmpty()
+ ? current.staticRow
+ : (current.staticRow.isEmpty() ?
updater.apply(newStatic) : updater.apply(current.staticRow, newStatic));
+ Object[] tree = BTree.update(current.tree,
update.metadata().comparator, update, update.rowCount(), updater);
+ EncodingStats newStats = current.stats.mergeWith(update.stats());
+
+ if (tree != null && refUpdater.compareAndSet(this, current, new
Holder(columns, tree, deletionInfo, staticRow, newStats)))
+ {
+ updater.finish();
+ return new long[]{ updater.dataSize, updater.colUpdateTimeDelta };
+ }
+ else
+ {
+ return null;
+ }
+ }
/**
* Adds a given update to this in-memtable partition.
*
@@ -117,77 +161,40 @@ public final class AtomicBTreePartition extends
AtomicBTreePartitionBase
public long[] addAllWithSizeDelta(final PartitionUpdate update,
OpOrder.Group writeOp, UpdateTransaction indexer)
{
RowUpdater updater = new RowUpdater(this, allocator, writeOp, indexer);
- DeletionInfo inputDeletionInfoCopy = null;
- boolean monitorOwned = false;
try
{
- if (usePessimisticLocking())
- {
- acquireLock();
- monitorOwned = true;
- }
+ boolean shouldLock = usePessimisticLocking();
indexer.start();
while (true)
{
- Holder current = ref;
- updater.ref = current;
- updater.reset();
-
- if (!update.deletionInfo().getPartitionDeletion().isLive())
-
indexer.onPartitionDeletion(update.deletionInfo().getPartitionDeletion());
-
- if (update.deletionInfo().hasRanges())
-
update.deletionInfo().rangeIterator(false).forEachRemaining(indexer::onRangeTombstone);
-
- DeletionInfo deletionInfo;
- if (update.deletionInfo().mayModify(current.deletionInfo))
+ if (shouldLock)
{
- if (inputDeletionInfoCopy == null)
- inputDeletionInfoCopy =
update.deletionInfo().copy(HeapAllocator.instance);
-
- deletionInfo =
current.deletionInfo.mutableCopy().add(inputDeletionInfoCopy);
- updater.allocated(deletionInfo.unsharedHeapSize() -
current.deletionInfo.unsharedHeapSize());
+ synchronized (this)
+ {
+ long[] result = addAllWithSizeDeltaInternal(updater,
update, indexer);
+ if (result != null)
+ return result;
+ }
}
else
{
- deletionInfo = current.deletionInfo;
- }
+ long[] result = addAllWithSizeDeltaInternal(updater,
update, indexer);
+ if (result != null)
+ return result;
- RegularAndStaticColumns columns =
update.columns().mergeTo(current.columns);
- Row newStatic = update.staticRow();
- Row staticRow = newStatic.isEmpty()
- ? current.staticRow
- : (current.staticRow.isEmpty() ?
updater.apply(newStatic) : updater.apply(current.staticRow, newStatic));
- Object[] tree = BTree.update(current.tree,
update.metadata().comparator, update, update.rowCount(), updater);
- EncodingStats newStats =
current.stats.mergeWith(update.stats());
-
- if (tree != null && refUpdater.compareAndSet(this, current,
new Holder(columns, tree, deletionInfo, staticRow, newStats)))
- {
- updater.finish();
- return new long[]{ updater.dataSize,
updater.colUpdateTimeDelta };
- }
- else if (!monitorOwned)
- {
- boolean shouldLock = usePessimisticLocking();
+ shouldLock = usePessimisticLocking();
if (!shouldLock)
{
shouldLock =
updateWastedAllocationTracker(updater.heapSize);
}
- if (shouldLock)
- {
- acquireLock();
- monitorOwned = true;
- }
}
}
}
finally
{
indexer.commit();
- if (monitorOwned)
- releaseLock();
}
}
@@ -312,6 +319,8 @@ public final class AtomicBTreePartition extends
AtomicBTreePartitionBase
long colUpdateTimeDelta = Long.MAX_VALUE;
List<Row> inserted; // TODO: replace with walk of aborted BTree
+ DeletionInfo inputDeletionInfoCopy = null;
+
private RowUpdater(AtomicBTreePartition updating, MemtableAllocator
allocator, OpOrder.Group writeOp, UpdateTransaction indexer)
{
this.updating = updating;
diff --git
a/src/java11/org/apache/cassandra/db/partitions/AtomicBTreePartitionBase.java
b/src/java11/org/apache/cassandra/db/partitions/AtomicBTreePartitionBase.java
deleted file mode 100644
index 56359a2..0000000
---
a/src/java11/org/apache/cassandra/db/partitions/AtomicBTreePartitionBase.java
+++ /dev/null
@@ -1,65 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.cassandra.db.partitions;
-
-import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
-import java.util.concurrent.locks.ReentrantLock;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import org.apache.cassandra.db.DecoratedKey;
-
-/**
- * Java 11 version for the partition-locks in {@link AtomicBTreePartition}.
- */
-public abstract class AtomicBTreePartitionBase extends AbstractBTreePartition
-{
- private static final Logger logger =
LoggerFactory.getLogger(AtomicBTreePartitionBase.class);
-
- protected AtomicBTreePartitionBase(DecoratedKey partitionKey)
- {
- super(partitionKey);
- }
-
- // Replacement for Unsafe.monitorEnter/monitorExit.
- private volatile ReentrantLock lock;
- private static final AtomicReferenceFieldUpdater<AtomicBTreePartitionBase,
ReentrantLock> lockUpdater =
AtomicReferenceFieldUpdater.newUpdater(AtomicBTreePartitionBase.class,
ReentrantLock.class, "lock");
-
- static
- {
- logger.info("Initializing Java 11 support for AtomicBTreePartition");
-
- if (Runtime.version().version().get(0) < 11)
- throw new RuntimeException("Java 11 required, but found " +
Runtime.version());
- }
-
- protected final void acquireLock()
- {
- if (lock == null)
- lockUpdater.compareAndSet(this, null, new ReentrantLock());
-
- lock.lock();
- }
-
- protected final void releaseLock()
- {
- lock.unlock();
- }
-}
diff --git
a/src/java8/org/apache/cassandra/db/partitions/AtomicBTreePartitionBase.java
b/src/java8/org/apache/cassandra/db/partitions/AtomicBTreePartitionBase.java
deleted file mode 100644
index 32209e9..0000000
--- a/src/java8/org/apache/cassandra/db/partitions/AtomicBTreePartitionBase.java
+++ /dev/null
@@ -1,81 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.apache.cassandra.db.partitions;
-
-import sun.misc.Unsafe;
-
-import java.lang.reflect.Field;
-
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-import org.apache.cassandra.db.DecoratedKey;
-
-/**
- * Java 8 version for the partition-locks in {@link AtomicBTreePartition}.
- */
-public abstract class AtomicBTreePartitionBase extends AbstractBTreePartition
-{
- private static final Logger logger =
LoggerFactory.getLogger(AtomicBTreePartitionBase.class);
- private static final Unsafe unsafe;
-
- static
- {
- logger.info("Initializing Java 8 support for AtomicBTreePartition");
-
- if (!System.getProperty("java.version").startsWith("1.8.0"))
- throw new RuntimeException("Java 8 required, but running " +
System.getProperty("java.version"));
-
- try
- {
- // Safety... in case someone builds only on Java 8 but runs on
Java 11...
- sun.misc.Unsafe.class.getDeclaredMethod("monitorEnter",
Object.class);
- sun.misc.Unsafe.class.getDeclaredMethod("monitorExit",
Object.class);
-
- Field field = sun.misc.Unsafe.class.getDeclaredField("theUnsafe");
- field.setAccessible(true);
- unsafe = (sun.misc.Unsafe) field.get(null);
- }
- catch (NoSuchFieldException | NoSuchMethodException e)
- {
- throw new RuntimeException("This build of Cassandra has no support
for Java 11 as only Java 8 was available during the build. This should never
happen.");
- }
- catch (Exception e)
- {
- throw new AssertionError(e);
- }
- }
-
- protected AtomicBTreePartitionBase(DecoratedKey partitionKey)
- {
- super(partitionKey);
- }
-
- protected final void acquireLock()
- {
- if (unsafe != null)
- unsafe.monitorEnter(this);
- }
-
- protected final void releaseLock()
- {
- if (unsafe != null)
- unsafe.monitorExit(this);
- }
-}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
