Only include modified column data in indexing delta Patch by Sam Tunnicliffe; reviewed by Ariel Weisberg for CASSANDRA-10438
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/4f4918f6 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/4f4918f6 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/4f4918f6 Branch: refs/heads/trunk Commit: 4f4918f6ff02ab3c232a71ebd80073f407afcb58 Parents: 6b25c58 Author: Sam Tunnicliffe <[email protected]> Authored: Fri Oct 2 16:43:20 2015 +0100 Committer: Sam Tunnicliffe <[email protected]> Committed: Thu Oct 8 10:34:59 2015 +0100 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../cassandra/index/SecondaryIndexManager.java | 6 +- .../validation/entities/SecondaryIndexTest.java | 80 ++++++++++++++++++++ .../org/apache/cassandra/index/StubIndex.java | 1 + 4 files changed, 85 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f4918f6/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 18d84de..6eb56ec 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 3.0 + * Only include modified cell data in indexing deltas (CASSANDRA-10438) * Do not load keyspace when creating sstable writer (CASSANDRA-10443) * If node is not yet gossiping write all MV updates to batchlog only (CASSANDRA-10413) * Re-populate token metadata after commit log recovery (CASSANDRA-10293) http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f4918f6/src/java/org/apache/cassandra/index/SecondaryIndexManager.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java index 9b15a3e..87b47d9 100644 --- a/src/java/org/apache/cassandra/index/SecondaryIndexManager.java +++ b/src/java/org/apache/cassandra/index/SecondaryIndexManager.java @@ -776,15 +776,15 @@ public class SecondaryIndexManager implements IndexRegistry { final Row.Builder toRemove = BTreeRow.sortedBuilder(); toRemove.newRow(existing.clustering()); + toRemove.addPrimaryKeyLivenessInfo(existing.primaryKeyLivenessInfo()); final Row.Builder toInsert = BTreeRow.sortedBuilder(); toInsert.newRow(updated.clustering()); + toInsert.addPrimaryKeyLivenessInfo(updated.primaryKeyLivenessInfo()); // diff listener collates the columns to be added & removed from the indexes RowDiffListener diffListener = new RowDiffListener() { public void onPrimaryKeyLivenessInfo(int i, Clustering clustering, LivenessInfo merged, LivenessInfo original) { - if (merged != null && merged != original) - toInsert.addPrimaryKeyLivenessInfo(merged); } public void onDeletion(int i, Clustering clustering, Row.Deletion merged, Row.Deletion original) @@ -797,7 +797,7 @@ public class SecondaryIndexManager implements IndexRegistry public void onCell(int i, Clustering clustering, Cell merged, Cell original) { - if (merged != null && merged != original) + if (merged != null && !merged.equals(original)) toInsert.addCell(merged); if (merged == null || (original != null && shouldCleanupOldValue(original, merged))) http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f4918f6/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 48d3a85..472149d 100644 --- a/test/unit/org/apache/cassandra/cql3/validation/entities/SecondaryIndexTest.java +++ b/test/unit/org/apache/cassandra/cql3/validation/entities/SecondaryIndexTest.java @@ -30,16 +30,19 @@ import org.apache.cassandra.cql3.ColumnIdentifier; import org.apache.cassandra.cql3.statements.IndexTarget; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.marshal.AbstractType; +import org.apache.cassandra.db.rows.Cell; import org.apache.cassandra.db.rows.Row; import org.apache.cassandra.exceptions.ConfigurationException; import org.apache.cassandra.exceptions.SyntaxException; import org.apache.cassandra.index.SecondaryIndexManager; import org.apache.cassandra.index.StubIndex; import org.apache.cassandra.schema.IndexMetadata; +import org.apache.cassandra.utils.ByteBufferUtil; 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; @@ -709,6 +712,83 @@ public class SecondaryIndexTest extends CQLTester assertColumnValue(1, "c", index2.rowsUpdated.get(0).right, cfm); } + @Test + public void testUpdatesToMemtableData() throws Throwable + { + // verify the contract specified by Index.Indexer::updateRow(oldRowData, newRowData), + // when a row in the memtable is updated, the indexer should be informed of: + // * new columns + // * removed columns + // * columns whose value, timestamp or ttl have been modified. + // Any columns which are unchanged by the update are not passed to the Indexer + // Note that for simplicity this test resets the index between each scenario + createTable("CREATE TABLE %s (k int, c int, v1 int, v2 int, PRIMARY KEY (k,c))"); + createIndex(String.format("CREATE CUSTOM INDEX test_index ON %%s() USING '%s'", StubIndex.class.getName())); + execute("INSERT INTO %s (k, c, v1, v2) VALUES (0, 0, 0, 0) USING TIMESTAMP 0"); + + ColumnDefinition v1 = getCurrentColumnFamilyStore().metadata.getColumnDefinition(new ColumnIdentifier("v1", true)); + ColumnDefinition v2 = getCurrentColumnFamilyStore().metadata.getColumnDefinition(new ColumnIdentifier("v2", true)); + + StubIndex index = (StubIndex)getCurrentColumnFamilyStore().indexManager.getIndexByName("test_index"); + assertEquals(1, index.rowsInserted.size()); + + // Overwrite a single value, leaving the other untouched + execute("UPDATE %s USING TIMESTAMP 1 SET v1=1 WHERE k=0 AND c=0"); + assertEquals(1, index.rowsUpdated.size()); + Row oldRow = index.rowsUpdated.get(0).left; + assertEquals(1, oldRow.size()); + validateCell(oldRow.getCell(v1), v1, ByteBufferUtil.bytes(0), 0); + Row newRow = index.rowsUpdated.get(0).right; + assertEquals(1, newRow.size()); + validateCell(newRow.getCell(v1), v1, ByteBufferUtil.bytes(1), 1); + index.reset(); + + // Overwrite both values + execute("UPDATE %s USING TIMESTAMP 2 SET v1=2, v2=2 WHERE k=0 AND c=0"); + assertEquals(1, index.rowsUpdated.size()); + oldRow = index.rowsUpdated.get(0).left; + assertEquals(2, oldRow.size()); + validateCell(oldRow.getCell(v1), v1, ByteBufferUtil.bytes(1), 1); + validateCell(oldRow.getCell(v2), v2, ByteBufferUtil.bytes(0), 0); + newRow = index.rowsUpdated.get(0).right; + assertEquals(2, newRow.size()); + validateCell(newRow.getCell(v1), v1, ByteBufferUtil.bytes(2), 2); + validateCell(newRow.getCell(v2), v2, ByteBufferUtil.bytes(2), 2); + index.reset(); + + // Delete one value + execute("DELETE v1 FROM %s USING TIMESTAMP 3 WHERE k=0 AND c=0"); + assertEquals(1, index.rowsUpdated.size()); + oldRow = index.rowsUpdated.get(0).left; + assertEquals(1, oldRow.size()); + validateCell(oldRow.getCell(v1), v1, ByteBufferUtil.bytes(2), 2); + newRow = index.rowsUpdated.get(0).right; + assertEquals(1, newRow.size()); + Cell newCell = newRow.getCell(v1); + assertTrue(newCell.isTombstone()); + assertEquals(3, newCell.timestamp()); + index.reset(); + + // Modify the liveness of the primary key, the delta rows should contain + // no cell data as only the pk was altered, but it should illustrate the + // change to the liveness info + execute("INSERT INTO %s(k, c) VALUES (0, 0) USING TIMESTAMP 4"); + assertEquals(1, index.rowsUpdated.size()); + oldRow = index.rowsUpdated.get(0).left; + assertEquals(0, oldRow.size()); + assertEquals(0, oldRow.primaryKeyLivenessInfo().timestamp()); + newRow = index.rowsUpdated.get(0).right; + assertEquals(0, newRow.size()); + assertEquals(4, newRow.primaryKeyLivenessInfo().timestamp()); + } + + private void validateCell(Cell cell, ColumnDefinition def, ByteBuffer val, long timestamp) + { + assertNotNull(cell); + assertEquals(0, def.type.compare(cell.value(), val)); + assertEquals(timestamp, cell.timestamp()); + } + private static void assertColumnValue(int expected, String name, Row row, CFMetaData cfm) { ColumnDefinition col = cfm.getColumnDefinition(new ColumnIdentifier(name, true)); http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f4918f6/test/unit/org/apache/cassandra/index/StubIndex.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/index/StubIndex.java b/test/unit/org/apache/cassandra/index/StubIndex.java index c8a3241..05c860a 100644 --- a/test/unit/org/apache/cassandra/index/StubIndex.java +++ b/test/unit/org/apache/cassandra/index/StubIndex.java @@ -58,6 +58,7 @@ public class StubIndex implements Index { rowsInserted.clear(); rowsDeleted.clear(); + rowsUpdated.clear(); partitionDeletions.clear(); rangeTombstones.clear(); }
