Push the validation of secondary index values to the secondary index manager
Patch by Alex Liu, Reviewed by tjake for CASSANDRA-4240 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/bce2d6c3 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/bce2d6c3 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/bce2d6c3 Branch: refs/heads/cassandra-1.1 Commit: bce2d6c35753929c62a557159f944ace82896869 Parents: 787e0e6 Author: T Jake Luciani <jak...@gmail.com> Authored: Thu Jul 26 13:37:24 2012 -0400 Committer: T Jake Luciani <jak...@gmail.com> Committed: Thu Jul 26 13:37:24 2012 -0400 ---------------------------------------------------------------------- CHANGES.txt | 2 +- .../db/index/PerColumnSecondaryIndex.java | 8 + .../cassandra/db/index/PerRowSecondaryIndex.java | 7 + .../apache/cassandra/db/index/SecondaryIndex.java | 3 + .../cassandra/db/index/SecondaryIndexManager.java | 7 + .../apache/cassandra/thrift/ThriftValidation.java | 14 +- .../cassandra/db/SecondaryIndexColumSizeTest.java | 216 +++++++++++++++ 7 files changed, 249 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/bce2d6c3/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 66a7b9e..79278aa 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -11,7 +11,7 @@ * Set gc_grace on index CF to 0 (CASSANDRA-4314) * fix 1.0.x node join to mixed version cluster, other nodes >= 1.1 (CASSANDRA-4195) * Fix LCS splitting sstable base on uncompressed size (CASSANDRA-4419) - + * Push the validation of secondary index values to the SecondaryIndexManager (CASSANDRA-4240) 1.0.10 * fix maxTimestamp to include row tombstones (CASSANDRA-4116) http://git-wip-us.apache.org/repos/asf/cassandra/blob/bce2d6c3/src/java/org/apache/cassandra/db/index/PerColumnSecondaryIndex.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/index/PerColumnSecondaryIndex.java b/src/java/org/apache/cassandra/db/index/PerColumnSecondaryIndex.java index c137ead..a7bd3fb 100644 --- a/src/java/org/apache/cassandra/db/index/PerColumnSecondaryIndex.java +++ b/src/java/org/apache/cassandra/db/index/PerColumnSecondaryIndex.java @@ -22,6 +22,8 @@ import java.nio.ByteBuffer; import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.IColumn; +import org.apache.cassandra.thrift.Column; +import org.apache.cassandra.utils.FBUtilities; /** * Base class for Secondary indexes that implement a unique index per column @@ -60,4 +62,10 @@ public abstract class PerColumnSecondaryIndex extends SecondaryIndex { return getIndexName(); } + + @Override + public boolean validate(Column column) + { + return column.value.remaining() < FBUtilities.MAX_UNSIGNED_SHORT; + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/bce2d6c3/src/java/org/apache/cassandra/db/index/PerRowSecondaryIndex.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/index/PerRowSecondaryIndex.java b/src/java/org/apache/cassandra/db/index/PerRowSecondaryIndex.java index f23980b..a14fead 100644 --- a/src/java/org/apache/cassandra/db/index/PerRowSecondaryIndex.java +++ b/src/java/org/apache/cassandra/db/index/PerRowSecondaryIndex.java @@ -26,6 +26,7 @@ import java.util.SortedSet; import org.apache.cassandra.db.ColumnFamily; import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.IColumn; +import org.apache.cassandra.thrift.Column; import org.apache.cassandra.utils.ByteBufferUtil; /** @@ -70,4 +71,10 @@ public abstract class PerRowSecondaryIndex extends SecondaryIndex throw new RuntimeException(e); } } + + @Override + public boolean validate(Column column) + { + return true; + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/bce2d6c3/src/java/org/apache/cassandra/db/index/SecondaryIndex.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/index/SecondaryIndex.java b/src/java/org/apache/cassandra/db/index/SecondaryIndex.java index 5006217..3e38f28 100644 --- a/src/java/org/apache/cassandra/db/index/SecondaryIndex.java +++ b/src/java/org/apache/cassandra/db/index/SecondaryIndex.java @@ -31,6 +31,7 @@ import org.apache.cassandra.db.compaction.CompactionManager; import org.apache.cassandra.db.index.keys.KeysIndex; import org.apache.cassandra.io.sstable.ReducingKeyIterator; import org.apache.cassandra.io.sstable.SSTableReader; +import org.apache.cassandra.thrift.Column; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; @@ -321,4 +322,6 @@ public abstract class SecondaryIndex return index; } + + public abstract boolean validate(Column column); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/bce2d6c3/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java b/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java index 77bf954..ecdf8b1 100644 --- a/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java +++ b/src/java/org/apache/cassandra/db/index/SecondaryIndexManager.java @@ -31,6 +31,7 @@ import org.apache.cassandra.dht.AbstractBounds; import org.apache.cassandra.dht.LocalToken; import org.apache.cassandra.io.sstable.ReducingKeyIterator; import org.apache.cassandra.io.sstable.SSTableReader; +import org.apache.cassandra.thrift.Column; import org.apache.cassandra.thrift.IndexClause; import org.apache.cassandra.thrift.IndexExpression; import org.apache.cassandra.utils.ByteBufferUtil; @@ -559,4 +560,10 @@ public class SecondaryIndexManager return indexSearchers.get(0).search(clause, range, dataFilter); } + + public boolean validate(Column column) + { + SecondaryIndex index = getIndexForColumn(column.name); + return index != null ? index.validate(column) : true; + } } http://git-wip-us.apache.org/repos/asf/cassandra/blob/bce2d6c3/src/java/org/apache/cassandra/thrift/ThriftValidation.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/thrift/ThriftValidation.java b/src/java/org/apache/cassandra/thrift/ThriftValidation.java index ee1f662..480a9be 100644 --- a/src/java/org/apache/cassandra/thrift/ThriftValidation.java +++ b/src/java/org/apache/cassandra/thrift/ThriftValidation.java @@ -428,13 +428,13 @@ public class ThriftValidation (isSubColumn ? metadata.subcolumnComparator : metadata.comparator).getString(column.name))); } - // Indexed column values cannot be larger than 64K. See CASSANDRA-3057 for more details - if (columnDef != null && columnDef.getIndexType() != null && column.value.remaining() > FBUtilities.MAX_UNSIGNED_SHORT) - throw new InvalidRequestException(String.format("Can't index column value of size %d for index %s in CF %s of KS %s", - column.value.remaining(), - columnDef.getIndexName(), - metadata.cfName, - metadata.ksName)); + // Indexed column values cannot be larger than 64K. See CASSANDRA-3057/4240 for more details + if (!Table.open(metadata.ksName).getColumnFamilyStore(metadata.cfName).indexManager.validate(column)) + throw new InvalidRequestException(String.format("Can't index column value of size %d for index %s in CF %s of KS %s", + column.value.remaining(), + columnDef.getIndexName(), + metadata.cfName, + metadata.ksName)); } /** http://git-wip-us.apache.org/repos/asf/cassandra/blob/bce2d6c3/test/unit/org/apache/cassandra/db/SecondaryIndexColumSizeTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/SecondaryIndexColumSizeTest.java b/test/unit/org/apache/cassandra/db/SecondaryIndexColumSizeTest.java new file mode 100644 index 0000000..bf35c30 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/SecondaryIndexColumSizeTest.java @@ -0,0 +1,216 @@ +/* +* 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; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.List; +import java.util.Set; +import java.util.SortedSet; + +import org.apache.cassandra.config.ConfigurationException; +import org.apache.cassandra.db.index.PerColumnSecondaryIndex; +import org.apache.cassandra.db.index.PerRowSecondaryIndex; +import org.apache.cassandra.db.index.SecondaryIndexSearcher; +import org.apache.cassandra.thrift.Column; +import org.apache.cassandra.utils.ByteBufferUtil; +import org.junit.Test; + +public class SecondaryIndexColumSizeTest +{ + + @Test + public void test64kColumn() + { + Column column = new Column(); + column.name = ByteBufferUtil.bytes("test"); + + // a byte buffer more than 64k + ByteBuffer buffer = ByteBuffer.allocate(1024 * 65); + buffer.clear(); + + //read more than 64k + for (int i=0; i<1024*64/4 + 1; i++) + buffer.putInt(0); + + // for read + buffer.flip(); + column.value = buffer; + + SolrIndex solrIndex = new SolrIndex(); + ColumnIndex perColumnIndex = new ColumnIndex(); + + assertTrue(solrIndex.validate(column)); + assertFalse(perColumnIndex.validate(column)); + + // test less than 64k value + buffer.flip(); + buffer.clear(); + buffer.putInt(20); + buffer.flip(); + + assertTrue(solrIndex.validate(column)); + assertTrue(perColumnIndex.validate(column)); + } + + public class SolrIndex extends PerRowSecondaryIndex + { + + @Override + public void applyIndexUpdates(ByteBuffer rowKey, ColumnFamily cf, SortedSet<ByteBuffer> mutatedIndexedColumns, ColumnFamily oldIndexedColumns) throws IOException + { + } + + @Override + public void init() + { + } + + @Override + public void validateOptions() throws ConfigurationException + { + } + + @Override + public String getIndexName() + { + return null; + } + + @Override + protected SecondaryIndexSearcher createSecondaryIndexSearcher(Set<ByteBuffer> columns) + { + return null; + } + + @Override + public void forceBlockingFlush() throws IOException + { + } + + @Override + public long getLiveSize() + { + return 0; + } + + @Override + public ColumnFamilyStore getIndexCfs() + { + return null; + } + + @Override + public void removeIndex(ByteBuffer columnName) throws IOException + { + } + + @Override + public void invalidate() + { + } + + @Override + public void truncate(long truncatedAt) + { + } + + @Override + public void deleteFromIndex(DecoratedKey<?> key, List<IColumn> indexedColumnsInRow) + { + } + + } + + + public class ColumnIndex extends PerColumnSecondaryIndex + { + + @Override + public void init() + { + } + + @Override + public void validateOptions() throws ConfigurationException + { + } + + @Override + public String getIndexName() + { + return null; + } + + @Override + protected SecondaryIndexSearcher createSecondaryIndexSearcher(Set<ByteBuffer> columns) + { + return null; + } + + @Override + public void forceBlockingFlush() throws IOException + { + } + + @Override + public long getLiveSize() + { + return 0; + } + + @Override + public ColumnFamilyStore getIndexCfs() + { + return null; + } + + @Override + public void removeIndex(ByteBuffer columnName) throws IOException + { + } + + @Override + public void invalidate() + { + } + + @Override + public void truncate(long truncatedAt) + { + } + + @Override + public void deleteColumn(DecoratedKey valueKey, ByteBuffer rowKey, IColumn col) throws IOException + { + } + + @Override + public void insertColumn(DecoratedKey valueKey, ByteBuffer rowKey, IColumn col) throws IOException + { + } + + @Override + public void updateColumn(DecoratedKey valueKey, ByteBuffer rowKey, IColumn col) throws IOException + { + } + } +}