You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by st...@apache.org on 2016/06/20 11:27:47 UTC
cassandra git commit: SSTable tools mishandling LocalPartitioner
Repository: cassandra
Updated Branches:
refs/heads/trunk 471552e04 -> 0e9d6bfe1
SSTable tools mishandling LocalPartitioner
patch by Chris Lohfink; reviewed by Stefania Alborghetti for CASSANDRA-12002
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/0e9d6bfe
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/0e9d6bfe
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/0e9d6bfe
Branch: refs/heads/trunk
Commit: 0e9d6bfe1a182b5ce12e7772ade3433b440aa2c6
Parents: 471552e
Author: Chris Lohfink <cl...@gmail.com>
Authored: Thu Jun 16 09:16:13 2016 +0200
Committer: Stefania Alborghetti <st...@datastax.com>
Committed: Mon Jun 20 13:25:02 2016 +0200
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../apache/cassandra/tools/SSTableExport.java | 8 +--
.../cassandra/tools/SSTableMetadataViewer.java | 2 +-
.../org/apache/cassandra/utils/FBUtilities.java | 35 ++++++++++++
.../apache/cassandra/utils/FBUtilitiesTest.java | 58 +++++++++++++++++++-
5 files changed, 94 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0e9d6bfe/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 3d8d511..a0f7c56 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.8
+ * SSTable tools mishandling LocalPartitioner (CASSANDRA-12002)
* When SEPWorker assigned work, set thread name to match pool (CASSANDRA-11966)
* Add cross-DC latency metrics (CASSANDRA-11596)
* Allow terms in selection clause (CASSANDRA-10783)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0e9d6bfe/src/java/org/apache/cassandra/tools/SSTableExport.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/tools/SSTableExport.java b/src/java/org/apache/cassandra/tools/SSTableExport.java
index 09dbbed..cc6b84b 100644
--- a/src/java/org/apache/cassandra/tools/SSTableExport.java
+++ b/src/java/org/apache/cassandra/tools/SSTableExport.java
@@ -98,14 +98,10 @@ public class SSTableExport
if (!desc.version.storeRows())
throw new IOException("pre-3.0 SSTable is not supported.");
- EnumSet<MetadataType> types = EnumSet.of(MetadataType.VALIDATION, MetadataType.STATS, MetadataType.HEADER);
+ EnumSet<MetadataType> types = EnumSet.of(MetadataType.STATS, MetadataType.HEADER);
Map<MetadataType, MetadataComponent> sstableMetadata = desc.getMetadataSerializer().deserialize(desc, types);
- ValidationMetadata validationMetadata = (ValidationMetadata) sstableMetadata.get(MetadataType.VALIDATION);
SerializationHeader.Component header = (SerializationHeader.Component) sstableMetadata.get(MetadataType.HEADER);
-
- IPartitioner partitioner = SecondaryIndexManager.isIndexColumnFamily(desc.cfname)
- ? new LocalPartitioner(header.getKeyType())
- : FBUtilities.newPartitioner(validationMetadata.partitioner);
+ IPartitioner partitioner = FBUtilities.newPartitioner(desc);
CFMetaData.Builder builder = CFMetaData.Builder.create("keyspace", "table").withPartitioner(partitioner);
header.getStaticColumns().entrySet().stream()
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0e9d6bfe/src/java/org/apache/cassandra/tools/SSTableMetadataViewer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/tools/SSTableMetadataViewer.java b/src/java/org/apache/cassandra/tools/SSTableMetadataViewer.java
index ef53087..3c8ba64 100644
--- a/src/java/org/apache/cassandra/tools/SSTableMetadataViewer.java
+++ b/src/java/org/apache/cassandra/tools/SSTableMetadataViewer.java
@@ -92,7 +92,7 @@ public class SSTableMetadataViewer
out.printf("TTL max: %s%n", stats.maxTTL);
if (validation != null && header != null)
- printMinMaxToken(descriptor, FBUtilities.newPartitioner(validation.partitioner), header.getKeyType(), out);
+ printMinMaxToken(descriptor, FBUtilities.newPartitioner(descriptor), header.getKeyType(), out);
if (header != null && header.getClusteringTypes().size() == stats.minClusteringValues.size())
{
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0e9d6bfe/src/java/org/apache/cassandra/utils/FBUtilities.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/utils/FBUtilities.java b/src/java/org/apache/cassandra/utils/FBUtilities.java
index 76178ad..af2cb1b 100644
--- a/src/java/org/apache/cassandra/utils/FBUtilities.java
+++ b/src/java/org/apache/cassandra/utils/FBUtilities.java
@@ -32,6 +32,7 @@ import java.util.zip.Checksum;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Joiner;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
@@ -42,11 +43,18 @@ import org.apache.cassandra.auth.IAuthorizer;
import org.apache.cassandra.auth.IRoleManager;
import org.apache.cassandra.config.DatabaseDescriptor;
import org.apache.cassandra.db.DecoratedKey;
+import org.apache.cassandra.db.SerializationHeader;
+import org.apache.cassandra.db.marshal.AbstractType;
import org.apache.cassandra.dht.IPartitioner;
+import org.apache.cassandra.dht.LocalPartitioner;
import org.apache.cassandra.dht.Range;
import org.apache.cassandra.dht.Token;
import org.apache.cassandra.exceptions.ConfigurationException;
import org.apache.cassandra.io.IVersionedSerializer;
+import org.apache.cassandra.io.sstable.Descriptor;
+import org.apache.cassandra.io.sstable.metadata.MetadataComponent;
+import org.apache.cassandra.io.sstable.metadata.MetadataType;
+import org.apache.cassandra.io.sstable.metadata.ValidationMetadata;
import org.apache.cassandra.schema.CompressionParams;
import org.apache.cassandra.io.util.DataOutputBuffer;
import org.apache.cassandra.io.util.DataOutputBufferFixed;
@@ -395,10 +403,37 @@ public class FBUtilities
result.get(ms, TimeUnit.MILLISECONDS);
}
+ /**
+ * Create a new instance of a partitioner defined in an SSTable Descriptor
+ * @param desc Descriptor of an sstable
+ * @return a new IPartitioner instance
+ * @throws IOException
+ */
+ public static IPartitioner newPartitioner(Descriptor desc) throws IOException
+ {
+ EnumSet<MetadataType> types = EnumSet.of(MetadataType.VALIDATION, MetadataType.HEADER);
+ Map<MetadataType, MetadataComponent> sstableMetadata = desc.getMetadataSerializer().deserialize(desc, types);
+ ValidationMetadata validationMetadata = (ValidationMetadata) sstableMetadata.get(MetadataType.VALIDATION);
+ SerializationHeader.Component header = (SerializationHeader.Component) sstableMetadata.get(MetadataType.HEADER);
+ return newPartitioner(validationMetadata.partitioner, Optional.of(header.getKeyType()));
+ }
+
public static IPartitioner newPartitioner(String partitionerClassName) throws ConfigurationException
{
+ return newPartitioner(partitionerClassName, Optional.empty());
+ }
+
+ @VisibleForTesting
+ static IPartitioner newPartitioner(String partitionerClassName, Optional<AbstractType<?>> comparator) throws ConfigurationException
+ {
if (!partitionerClassName.contains("."))
partitionerClassName = "org.apache.cassandra.dht." + partitionerClassName;
+
+ if (partitionerClassName.equals("org.apache.cassandra.dht.LocalPartitioner"))
+ {
+ assert comparator.isPresent() : "Expected a comparator for local partitioner";
+ return new LocalPartitioner(comparator.get());
+ }
return FBUtilities.instanceOrConstruct(partitionerClassName, "partitioner");
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/0e9d6bfe/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java
index 90c5f05..3c1ea74 100644
--- a/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java
+++ b/test/unit/org/apache/cassandra/utils/FBUtilitiesTest.java
@@ -1,4 +1,4 @@
-/**
+/*
* 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
@@ -22,12 +22,17 @@ import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.CharacterCodingException;
import java.nio.charset.StandardCharsets;
+import java.util.Map;
+import java.util.Optional;
+import java.util.TreeMap;
import com.google.common.primitives.Ints;
+
+import org.junit.Assert;
import org.junit.Test;
-import java.util.Map;
-import java.util.TreeMap;
+import org.apache.cassandra.db.marshal.*;
+import org.apache.cassandra.dht.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
@@ -95,4 +100,51 @@ public class FBUtilitiesTest
ByteBuffer bytes = ByteBuffer.wrap(new byte[]{(byte)0xff, (byte)0xfe});
ByteBufferUtil.string(bytes, StandardCharsets.UTF_8);
}
+
+ private static void assertPartitioner(String name, Class expected)
+ {
+ Assert.assertTrue(String.format("%s != %s", name, expected.toString()),
+ expected.isInstance(FBUtilities.newPartitioner(name)));
+ }
+
+ /**
+ * Check that given a name, the correct partitioner instance is created.
+ *
+ * If the assertions in this test start failing, it likely means the sstabledump/sstablemetadata tools will
+ * also fail to read existing sstables.
+ */
+ @Test
+ public void testNewPartitionerNoArgConstructors()
+ {
+ assertPartitioner("ByteOrderedPartitioner", ByteOrderedPartitioner.class);
+ assertPartitioner("LengthPartitioner", LengthPartitioner.class);
+ assertPartitioner("Murmur3Partitioner", Murmur3Partitioner.class);
+ assertPartitioner("OrderPreservingPartitioner", OrderPreservingPartitioner.class);
+ assertPartitioner("RandomPartitioner", RandomPartitioner.class);
+ assertPartitioner("org.apache.cassandra.dht.ByteOrderedPartitioner", ByteOrderedPartitioner.class);
+ assertPartitioner("org.apache.cassandra.dht.LengthPartitioner", LengthPartitioner.class);
+ assertPartitioner("org.apache.cassandra.dht.Murmur3Partitioner", Murmur3Partitioner.class);
+ assertPartitioner("org.apache.cassandra.dht.OrderPreservingPartitioner", OrderPreservingPartitioner.class);
+ assertPartitioner("org.apache.cassandra.dht.RandomPartitioner", RandomPartitioner.class);
+ }
+
+ /**
+ * Check that we can instantiate local partitioner correctly and that we can pass the correct type
+ * to it as a constructor argument.
+ *
+ * If the assertions in this test start failing, it likely means the sstabledump/sstablemetadata tools will
+ * also fail to read existing sstables.
+ */
+ @Test
+ public void testNewPartitionerLocalPartitioner()
+ {
+ for (String name : new String[] {"LocalPartitioner", "org.apache.cassandra.dht.LocalPartitioner"})
+ for (AbstractType<?> type : new AbstractType<?>[] {UUIDType.instance, ListType.getInstance(Int32Type.instance, true)})
+ {
+ IPartitioner partitioner = FBUtilities.newPartitioner(name, Optional.of(type));
+ Assert.assertTrue(String.format("%s != LocalPartitioner", partitioner.toString()),
+ LocalPartitioner.class.isInstance(partitioner));
+ Assert.assertEquals(partitioner.partitionOrdering(), type);
+ }
+ }
}