Repository: cassandra Updated Branches: refs/heads/cassandra-3.0 9a2fd8ccf -> 4c56c89a1 refs/heads/cassandra-3.1 918bc0116 -> 5de5d06a4 refs/heads/trunk 306d882fe -> 4a4afa79c
Invalidate prepared statements when indexes are modified Patch by Sam Tunnicliffe; reviewed by Sylvain Lebresne for CASSANDRA-10758a Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/4c56c89a Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/4c56c89a Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/4c56c89a Branch: refs/heads/cassandra-3.0 Commit: 4c56c89a1e47927d6006a6f4294c8a84035fe623 Parents: 9a2fd8c Author: Sam Tunnicliffe <[email protected]> Authored: Fri Nov 27 12:00:04 2015 +0000 Committer: Sam Tunnicliffe <[email protected]> Committed: Mon Nov 30 12:14:17 2015 +0000 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../org/apache/cassandra/config/CFMetaData.java | 14 +++-- .../org/apache/cassandra/config/Schema.java | 8 +-- .../apache/cassandra/cql3/QueryProcessor.java | 4 +- .../cassandra/service/MigrationListener.java | 4 +- .../cassandra/service/MigrationManager.java | 4 +- .../org/apache/cassandra/transport/Server.java | 2 +- .../validation/entities/SecondaryIndexTest.java | 62 +++++++++++++------- .../index/internal/CassandraIndexTest.java | 8 --- 9 files changed, 62 insertions(+), 45 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 0bb05d9..0356045 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0.1 + * Invalidate prepared statements on DROP INDEX (CASSANDRA-10758) * Fix SELECT statement with IN restrictions on partition key, ORDER BY and LIMIT (CASSANDRA-10729) * Improve stress performance over 1k threads (CASSANDRA-7217) http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/src/java/org/apache/cassandra/config/CFMetaData.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/config/CFMetaData.java b/src/java/org/apache/cassandra/config/CFMetaData.java index 4d5a176..62b2369 100644 --- a/src/java/org/apache/cassandra/config/CFMetaData.java +++ b/src/java/org/apache/cassandra/config/CFMetaData.java @@ -723,7 +723,9 @@ public final class CFMetaData /** * Updates CFMetaData in-place to match cfm * - * @return true if any columns were added, removed, or altered; otherwise, false is returned + * @return true if any change was made which impacts queries/updates on the table, + * e.g. any columns or indexes were added, removed, or altered; otherwise, false is returned. + * Used to determine whether prepared statements against this table need to be re-prepared. * @throws ConfigurationException if ks/cf names or cf ids didn't match */ @VisibleForTesting @@ -731,12 +733,12 @@ public final class CFMetaData { logger.debug("applying {} to {}", cfm, this); - validateCompatility(cfm); + validateCompatibility(cfm); partitionKeyColumns = cfm.partitionKeyColumns; clusteringColumns = cfm.clusteringColumns; - boolean hasColumnChange = !partitionColumns.equals(cfm.partitionColumns); + boolean changeAffectsStatements = !partitionColumns.equals(cfm.partitionColumns); partitionColumns = cfm.partitionColumns; rebuild(); @@ -752,14 +754,16 @@ public final class CFMetaData droppedColumns = cfm.droppedColumns; triggers = cfm.triggers; + + changeAffectsStatements |= !indexes.equals(cfm.indexes); indexes = cfm.indexes; logger.debug("application result is {}", this); - return hasColumnChange; + return changeAffectsStatements; } - public void validateCompatility(CFMetaData cfm) throws ConfigurationException + public void validateCompatibility(CFMetaData cfm) throws ConfigurationException { // validate if (!cfm.ksName.equals(ksName)) http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/src/java/org/apache/cassandra/config/Schema.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/config/Schema.java b/src/java/org/apache/cassandra/config/Schema.java index 117c5cd..569c87d 100644 --- a/src/java/org/apache/cassandra/config/Schema.java +++ b/src/java/org/apache/cassandra/config/Schema.java @@ -625,11 +625,11 @@ public class Schema { CFMetaData current = getCFMetaData(table.ksName, table.cfName); assert current != null; - boolean columnsDidChange = current.apply(table); + boolean changeAffectsStatements = current.apply(table); Keyspace keyspace = Keyspace.open(current.ksName); keyspace.getColumnFamilyStore(current.cfName).reload(); - MigrationManager.instance.notifyUpdateColumnFamily(current, columnsDidChange); + MigrationManager.instance.notifyUpdateColumnFamily(current, changeAffectsStatements); } public void dropTable(String ksName, String tableName) @@ -682,12 +682,12 @@ public class Schema public void updateView(ViewDefinition view) { ViewDefinition current = getKSMetaData(view.ksName).views.get(view.viewName).get(); - boolean columnsDidChange = current.metadata.apply(view.metadata); + boolean changeAffectsStatements = current.metadata.apply(view.metadata); Keyspace keyspace = Keyspace.open(current.ksName); keyspace.getColumnFamilyStore(current.viewName).reload(); Keyspace.open(current.ksName).viewManager.update(current.viewName); - MigrationManager.instance.notifyUpdateView(current, columnsDidChange); + MigrationManager.instance.notifyUpdateView(current, changeAffectsStatements); } public void dropView(String ksName, String viewName) http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/src/java/org/apache/cassandra/cql3/QueryProcessor.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/cql3/QueryProcessor.java b/src/java/org/apache/cassandra/cql3/QueryProcessor.java index 96e8387..03659ab 100644 --- a/src/java/org/apache/cassandra/cql3/QueryProcessor.java +++ b/src/java/org/apache/cassandra/cql3/QueryProcessor.java @@ -614,10 +614,10 @@ public class QueryProcessor implements QueryHandler removeAllInvalidPreparedStatementsForFunction(ksName, functionName); } - public void onUpdateColumnFamily(String ksName, String cfName, boolean columnsDidChange) + public void onUpdateColumnFamily(String ksName, String cfName, boolean affectsStatements) { logger.trace("Column definitions for {}.{} changed, invalidating related prepared statements", ksName, cfName); - if (columnsDidChange) + if (affectsStatements) removeInvalidPreparedStatements(ksName, cfName); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/src/java/org/apache/cassandra/service/MigrationListener.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/MigrationListener.java b/src/java/org/apache/cassandra/service/MigrationListener.java index 9e240ea..19d2592 100644 --- a/src/java/org/apache/cassandra/service/MigrationListener.java +++ b/src/java/org/apache/cassandra/service/MigrationListener.java @@ -52,7 +52,9 @@ public abstract class MigrationListener { } - public void onUpdateColumnFamily(String ksName, String cfName, boolean columnsDidChange) + // the boolean flag indicates whether the change that triggered this event may have a substantive + // impact on statements using the column family. + public void onUpdateColumnFamily(String ksName, String cfName, boolean affectsStatements) { } http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/src/java/org/apache/cassandra/service/MigrationManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/service/MigrationManager.java b/src/java/org/apache/cassandra/service/MigrationManager.java index 904665b..a53c3a1 100644 --- a/src/java/org/apache/cassandra/service/MigrationManager.java +++ b/src/java/org/apache/cassandra/service/MigrationManager.java @@ -408,7 +408,7 @@ public class MigrationManager throw new ConfigurationException(String.format("Cannot update non existing table '%s' in keyspace '%s'.", cfm.cfName, cfm.ksName)); KeyspaceMetadata ksm = Schema.instance.getKSMetaData(cfm.ksName); - oldCfm.validateCompatility(cfm); + oldCfm.validateCompatibility(cfm); logger.info(String.format("Update table '%s/%s' From %s To %s", cfm.ksName, cfm.cfName, oldCfm, cfm)); announce(SchemaKeyspace.makeUpdateTableMutation(ksm, oldCfm, cfm, FBUtilities.timestampMicros(), fromThrift), announceLocally); @@ -423,7 +423,7 @@ public class MigrationManager throw new ConfigurationException(String.format("Cannot update non existing materialized view '%s' in keyspace '%s'.", view.viewName, view.ksName)); KeyspaceMetadata ksm = Schema.instance.getKSMetaData(view.ksName); - oldView.metadata.validateCompatility(view.metadata); + oldView.metadata.validateCompatibility(view.metadata); logger.info(String.format("Update view '%s/%s' From %s To %s", view.ksName, view.viewName, oldView, view)); announce(SchemaKeyspace.makeUpdateViewMutation(ksm, oldView, view, FBUtilities.timestampMicros()), announceLocally); http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/src/java/org/apache/cassandra/transport/Server.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/transport/Server.java b/src/java/org/apache/cassandra/transport/Server.java index b786436..04da2d8 100644 --- a/src/java/org/apache/cassandra/transport/Server.java +++ b/src/java/org/apache/cassandra/transport/Server.java @@ -605,7 +605,7 @@ public class Server implements CassandraDaemon.Server send(new Event.SchemaChange(Event.SchemaChange.Change.UPDATED, ksName)); } - public void onUpdateColumnFamily(String ksName, String cfName, boolean columnsDidChange) + public void onUpdateColumnFamily(String ksName, String cfName, boolean affectsStatements) { send(new Event.SchemaChange(Event.SchemaChange.Change.UPDATED, Event.SchemaChange.Target.TABLE, ksName, cfName)); } http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/test/unit/org/apache/cassandra/cql3/validation/entities/SecondaryIndexTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/cql3/validation/entities/SecondaryIndexTest.java b/test/unit/org/apache/cassandra/cql3/validation/entities/SecondaryIndexTest.java index e8bf1fd..38402d9 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/SecondaryIndexTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/SecondaryIndexTest.java @@ -17,28 +17,20 @@ */ package org.apache.cassandra.cql3.validation.entities; -import org.junit.Before; -import org.junit.Test; -import static org.apache.cassandra.Util.throwAssert; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.nio.ByteBuffer; -import java.util.HashMap; -import java.util.Locale; -import java.util.Map; -import java.util.UUID; +import java.util.*; import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; +import org.apache.commons.lang3.StringUtils; +import org.junit.Test; + import org.apache.cassandra.config.CFMetaData; import org.apache.cassandra.config.ColumnDefinition; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.cql3.CQLTester; import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.QueryProcessor; import org.apache.cassandra.cql3.statements.IndexTarget; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.DeletionTime; @@ -52,22 +44,24 @@ import org.apache.cassandra.index.SecondaryIndexManager; import org.apache.cassandra.index.StubIndex; import org.apache.cassandra.index.internal.CustomCassandraIndex; import org.apache.cassandra.schema.IndexMetadata; +import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.transport.messages.ResultMessage; import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.MD5Digest; import org.apache.cassandra.utils.Pair; -import org.apache.commons.lang3.StringUtils; + +import static org.apache.cassandra.Util.throwAssert; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class SecondaryIndexTest extends CQLTester { private static final int TOO_BIG = 1024 * 65; - @Before - public void disablePreparedReuse() throws Throwable - { - // TODO: this shouldn't be needed but is due to #10758. As such, this should be removed on that - // ticket is fixed. - disablePreparedReuseForTest(); - } - @Test public void testCreateAndDropIndex() throws Throwable { @@ -949,6 +943,30 @@ public class SecondaryIndexTest extends CQLTester } } + @Test + public void droppingIndexInvalidatesPreparedStatements() throws Throwable + { + createTable("CREATE TABLE %s (a int, b int, c int, PRIMARY KEY ((a), b))"); + createIndex("CREATE INDEX c_idx ON %s(c)"); + MD5Digest cqlId = prepareStatement("SELECT * FROM %s.%s WHERE c=?", false).statementId; + Integer thriftId = prepareStatement("SELECT * FROM %s.%s WHERE c=?", true).toThriftPreparedResult().getItemId(); + + assertNotNull(QueryProcessor.instance.getPrepared(cqlId)); + assertNotNull(QueryProcessor.instance.getPreparedForThrift(thriftId)); + + dropIndex("DROP INDEX %s.c_idx"); + + assertNull(QueryProcessor.instance.getPrepared(cqlId)); + assertNull(QueryProcessor.instance.getPreparedForThrift(thriftId)); + } + + private ResultMessage.Prepared prepareStatement(String cql, boolean forThrift) + { + return QueryProcessor.prepare(String.format(cql, KEYSPACE, currentTable()), + ClientState.forInternalCalls(), + forThrift); + } + private void validateCell(Cell cell, ColumnDefinition def, ByteBuffer val, long timestamp) { assertNotNull(cell); http://git-wip-us.apache.org/repos/asf/cassandra/blob/4c56c89a/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java b/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java index c72b4ec..c6783cc 100644 --- a/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java +++ b/test/unit/org/apache/cassandra/index/internal/CassandraIndexTest.java @@ -56,14 +56,6 @@ import static org.junit.Assert.fail; */ public class CassandraIndexTest extends CQLTester { - @Before - public void disablePreparedReuse() throws Throwable - { - // TODO: this shouldn't be needed but is due to #10758. As such, this should be removed on that - // ticket is fixed. - disablePreparedReuseForTest(); - } - @Test public void indexOnRegularColumn() throws Throwable {
