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 <[email protected]> Authored: Thu Jun 16 09:16:13 2016 +0200 Committer: Stefania Alborghetti <[email protected]> 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); + } + } }
