Extend ColumnIdentifier.internedInstances key to include the type that generated the byte buffer
patch by Stefania Alborghetti; reviewed by Aleksey Yeschenko for CASSANDRA-12516 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/4bc3aa93 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/4bc3aa93 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/4bc3aa93 Branch: refs/heads/trunk Commit: 4bc3aa93337474a29bc4ad43ff4438755c9c7ab8 Parents: 85ed48a Author: Stefania Alborghetti <stefania.alborghe...@datastax.com> Authored: Fri Sep 2 16:28:35 2016 +0800 Committer: Stefania Alborghetti <stefania.alborghe...@datastax.com> Committed: Tue Sep 20 09:26:38 2016 +0800 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/cql3/ColumnIdentifier.java | 46 +++++++++++++++++--- .../apache/cassandra/schema/SchemaKeyspace.java | 6 ++- .../utils/NativeSSTableLoaderClient.java | 6 ++- .../cassandra/cql3/ColumnIdentifierTest.java | 24 ++++++++++ 5 files changed, 72 insertions(+), 11 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/4bc3aa93/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 5fd8e5e..b61c76c 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.10 + * Extend ColumnIdentifier.internedInstances key to include the type that generated the byte buffer (CASSANDRA-12516) * Backport CASSANDRA-10756 (race condition in NativeTransportService shutdown) (CASSANDRA-12472) http://git-wip-us.apache.org/repos/asf/cassandra/blob/4bc3aa93/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java b/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java index afb65e1..1e25b23 100644 --- a/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java +++ b/src/java/org/apache/cassandra/cql3/ColumnIdentifier.java @@ -60,7 +60,38 @@ public class ColumnIdentifier extends Selectable implements IMeasurableMemory, C private static final long EMPTY_SIZE = ObjectSizes.measure(new ColumnIdentifier(ByteBufferUtil.EMPTY_BYTE_BUFFER, "", false)); - private static final ConcurrentMap<ByteBuffer, ColumnIdentifier> internedInstances = new MapMaker().weakValues().makeMap(); + private static final ConcurrentMap<InternedKey, ColumnIdentifier> internedInstances = new MapMaker().weakValues().makeMap(); + + private static final class InternedKey + { + private final AbstractType<?> type; + private final ByteBuffer bytes; + + InternedKey(AbstractType<?> type, ByteBuffer bytes) + { + this.type = type; + this.bytes = bytes; + } + + @Override + public boolean equals(Object o) + { + if (this == o) + return true; + + if (o == null || getClass() != o.getClass()) + return false; + + InternedKey that = (InternedKey) o; + return bytes.equals(that.bytes) && type.equals(that.type); + } + + @Override + public int hashCode() + { + return bytes.hashCode() + 31 * type.hashCode(); + } + } private static long prefixComparison(ByteBuffer bytes) { @@ -103,24 +134,25 @@ public class ColumnIdentifier extends Selectable implements IMeasurableMemory, C public static ColumnIdentifier getInterned(ByteBuffer bytes, AbstractType<?> type) { - return getInterned(bytes, type.getString(bytes)); + return getInterned(type, bytes, type.getString(bytes)); } public static ColumnIdentifier getInterned(String rawText, boolean keepCase) { String text = keepCase ? rawText : rawText.toLowerCase(Locale.US); ByteBuffer bytes = ByteBufferUtil.bytes(text); - return getInterned(bytes, text); + return getInterned(UTF8Type.instance, bytes, text); } - public static ColumnIdentifier getInterned(ByteBuffer bytes, String text) + public static ColumnIdentifier getInterned(AbstractType<?> type, ByteBuffer bytes, String text) { - ColumnIdentifier id = internedInstances.get(bytes); + InternedKey key = new InternedKey(type, bytes); + ColumnIdentifier id = internedInstances.get(key); if (id != null) return id; ColumnIdentifier created = new ColumnIdentifier(bytes, text, true); - ColumnIdentifier previous = internedInstances.putIfAbsent(bytes, created); + ColumnIdentifier previous = internedInstances.putIfAbsent(key, created); return previous == null ? created : previous; } @@ -246,7 +278,7 @@ public class ColumnIdentifier extends Selectable implements IMeasurableMemory, C if (def.name.bytes.equals(bufferName)) return def.name; } - return getInterned(thriftColumnNameType.fromString(rawText), text); + return getInterned(thriftColumnNameType, thriftColumnNameType.fromString(rawText), text); } public boolean processesSelection() http://git-wip-us.apache.org/repos/asf/cassandra/blob/4bc3aa93/src/java/org/apache/cassandra/schema/SchemaKeyspace.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java index e3756ec..84a5e13 100644 --- a/src/java/org/apache/cassandra/schema/SchemaKeyspace.java +++ b/src/java/org/apache/cassandra/schema/SchemaKeyspace.java @@ -1001,8 +1001,6 @@ public final class SchemaKeyspace String keyspace = row.getString("keyspace_name"); String table = row.getString("table_name"); - ColumnIdentifier name = ColumnIdentifier.getInterned(row.getBytes("column_name_bytes"), row.getString("column_name")); - ColumnDefinition.Kind kind = ColumnDefinition.Kind.valueOf(row.getString("kind").toUpperCase()); int position = row.getInt("position"); @@ -1012,6 +1010,10 @@ public final class SchemaKeyspace if (order == ClusteringOrder.DESC) type = ReversedType.getInstance(type); + ColumnIdentifier name = ColumnIdentifier.getInterned(type, + row.getBytes("column_name_bytes"), + row.getString("column_name")); + return new ColumnDefinition(keyspace, table, name, type, position, kind); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/4bc3aa93/src/java/org/apache/cassandra/utils/NativeSSTableLoaderClient.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/utils/NativeSSTableLoaderClient.java b/src/java/org/apache/cassandra/utils/NativeSSTableLoaderClient.java index 5bcbcf7..4c6b12e 100644 --- a/src/java/org/apache/cassandra/utils/NativeSSTableLoaderClient.java +++ b/src/java/org/apache/cassandra/utils/NativeSSTableLoaderClient.java @@ -198,13 +198,15 @@ public class NativeSSTableLoaderClient extends SSTableLoader.Client private static ColumnDefinition createDefinitionFromRow(Row row, String keyspace, String table, Types types) { - ColumnIdentifier name = ColumnIdentifier.getInterned(row.getBytes("column_name_bytes"), row.getString("column_name")); - ClusteringOrder order = ClusteringOrder.valueOf(row.getString("clustering_order").toUpperCase()); AbstractType<?> type = CQLTypeParser.parse(keyspace, row.getString("type"), types); if (order == ClusteringOrder.DESC) type = ReversedType.getInstance(type); + ColumnIdentifier name = ColumnIdentifier.getInterned(type, + row.getBytes("column_name_bytes"), + row.getString("column_name")); + int position = row.getInt("position"); ColumnDefinition.Kind kind = ColumnDefinition.Kind.valueOf(row.getString("kind").toUpperCase()); return new ColumnDefinition(keyspace, table, name, type, position, kind); http://git-wip-us.apache.org/repos/asf/cassandra/blob/4bc3aa93/test/unit/org/apache/cassandra/cql3/ColumnIdentifierTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/ColumnIdentifierTest.java b/test/unit/org/apache/cassandra/cql3/ColumnIdentifierTest.java index c287883..c4b43b8 100644 --- a/test/unit/org/apache/cassandra/cql3/ColumnIdentifierTest.java +++ b/test/unit/org/apache/cassandra/cql3/ColumnIdentifierTest.java @@ -24,7 +24,9 @@ import java.util.concurrent.ThreadLocalRandom; import org.junit.Test; import junit.framework.Assert; +import org.apache.cassandra.db.marshal.AbstractType; import org.apache.cassandra.db.marshal.BytesType; +import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.utils.ByteBufferUtil; public class ColumnIdentifierTest @@ -58,4 +60,26 @@ public class ColumnIdentifierTest return v < 0 ? -1 : v > 0 ? 1 : 0; } + @Test + public void testInternedCache() + { + AbstractType<?> utf8Type = UTF8Type.instance; + AbstractType<?> bytesType = BytesType.instance; + + byte[] bytes = new byte [] { 0x63, (byte) 0x32 }; + String text = "c2"; // the UTF-8 encoding of this string is the same as bytes, 0x630x32 + + ColumnIdentifier c1 = ColumnIdentifier.getInterned(ByteBuffer.wrap(bytes), bytesType); + ColumnIdentifier c2 = ColumnIdentifier.getInterned(utf8Type, utf8Type.fromString(text), text); + ColumnIdentifier c3 = ColumnIdentifier.getInterned(text, true); + + Assert.assertTrue(c1.isInterned()); + Assert.assertTrue(c2.isInterned()); + Assert.assertTrue(c3.isInterned()); + + Assert.assertEquals("6332", c1.toString()); + Assert.assertEquals(text, c2.toString()); + Assert.assertEquals(text, c3.toString()); + } + }