From fb347bd3d16e35c50df5fca14ed3c0611e80985b Mon Sep 17 00:00:00 2001 From: Jonathan Ellis Date: Thu, 7 May 2009 16:03:32 +0000 Subject: [PATCH] fix CF.digest NPE. patch by jbellis; reviewed by Eric Evans for CASSANDRA-149 git-svn-id: https://svn.apache.org/repos/asf/incubator/cassandra/trunk@772690 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/cassandra/db/ColumnFamily.java | 7 +- .../org/apache/cassandra/db/ReadResponse.java | 4 +- .../apache/cassandra/db/ReadVerbHandler.java | 2 +- src/java/org/apache/cassandra/db/Row.java | 6 +- .../apache/cassandra/utils/FBUtilities.java | 6 +- .../apache/cassandra/db/ColumnFamilyTest.java | 92 +++++++++++-------- 6 files changed, 66 insertions(+), 51 deletions(-) diff --git a/src/java/org/apache/cassandra/db/ColumnFamily.java b/src/java/org/apache/cassandra/db/ColumnFamily.java index 9b1dd0ac4f..d5312f937c 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamily.java +++ b/src/java/org/apache/cassandra/db/ColumnFamily.java @@ -32,6 +32,7 @@ import java.util.SortedSet; import java.util.concurrent.atomic.AtomicInteger; import org.apache.commons.lang.StringUtils; +import org.apache.commons.lang.ArrayUtils; import org.apache.log4j.Logger; import org.apache.cassandra.config.DatabaseDescriptor; @@ -399,10 +400,10 @@ public final class ColumnFamily public byte[] digest() { Set columns = columns_.getSortedColumns(); - byte[] xorHash = null; - for(IColumn column : columns) + byte[] xorHash = ArrayUtils.EMPTY_BYTE_ARRAY; + for(IColumn column : columns) { - if(xorHash == null) + if(xorHash.length == 0) { xorHash = column.digest(); } diff --git a/src/java/org/apache/cassandra/db/ReadResponse.java b/src/java/org/apache/cassandra/db/ReadResponse.java index d3d8d566c1..e6e069f29a 100644 --- a/src/java/org/apache/cassandra/db/ReadResponse.java +++ b/src/java/org/apache/cassandra/db/ReadResponse.java @@ -27,6 +27,7 @@ import org.apache.cassandra.io.ICompactSerializer; import org.apache.cassandra.net.Message; import org.apache.cassandra.net.MessagingService; import org.apache.cassandra.service.StorageService; +import org.apache.commons.lang.ArrayUtils; /* @@ -60,11 +61,12 @@ private static ICompactSerializer serializer_; private String table_; private Row row_; - private byte[] digest_ = new byte[0]; + private byte[] digest_ = ArrayUtils.EMPTY_BYTE_ARRAY; private boolean isDigestQuery_ = false; public ReadResponse(String table, byte[] digest ) { + assert digest != null; table_ = table; digest_= digest; } diff --git a/src/java/org/apache/cassandra/db/ReadVerbHandler.java b/src/java/org/apache/cassandra/db/ReadVerbHandler.java index f551d699fe..5ec85905b0 100644 --- a/src/java/org/apache/cassandra/db/ReadVerbHandler.java +++ b/src/java/org/apache/cassandra/db/ReadVerbHandler.java @@ -77,7 +77,7 @@ public class ReadVerbHandler implements IVerbHandler Row row = null; row = readCommand.getRow(table); ReadResponse readResponse = null; - if(readCommand.isDigestQuery()) + if (readCommand.isDigestQuery()) { readResponse = new ReadResponse(table.getTableName(), row.digest()); } diff --git a/src/java/org/apache/cassandra/db/Row.java b/src/java/org/apache/cassandra/db/Row.java index 0f3e023c71..44f6bcf5fd 100644 --- a/src/java/org/apache/cassandra/db/Row.java +++ b/src/java/org/apache/cassandra/db/Row.java @@ -178,12 +178,10 @@ public class Row } else { - byte[] tmpHash = columnFamilies_.get(cFamily).digest(); - xorHash = FBUtilities.xor(xorHash, tmpHash); + xorHash = FBUtilities.xor(xorHash, columnFamilies_.get(cFamily).digest()); } } - logger_.info("DIGEST TIME: " + (System.currentTimeMillis() - start) - + " ms."); + logger_.info("DIGEST TIME: " + (System.currentTimeMillis() - start) + " ms."); return xorHash; } diff --git a/src/java/org/apache/cassandra/utils/FBUtilities.java b/src/java/org/apache/cassandra/utils/FBUtilities.java index b6b6802f91..e7a6325fa4 100644 --- a/src/java/org/apache/cassandra/utils/FBUtilities.java +++ b/src/java/org/apache/cassandra/utils/FBUtilities.java @@ -331,8 +331,10 @@ public class FBUtilities public static byte[] xor(byte[] b1, byte[] b2) { - byte[] bLess = null; - byte[] bMore = null; + assert b1 != null; + assert b2 != null; + byte[] bLess; + byte[] bMore; if(b1.length > b2.length) { diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java index c57b97bdd8..1647efb335 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyTest.java @@ -15,7 +15,8 @@ public class ColumnFamilyTest // TODO test SuperColumns @Test - public void testSingleColumn() throws IOException { + public void testSingleColumn() throws IOException + { Random random = new Random(); byte[] bytes = new byte[1024]; random.nextBytes(bytes); @@ -35,11 +36,12 @@ public class ColumnFamilyTest } @Test - public void testManyColumns() throws IOException { + public void testManyColumns() throws IOException + { ColumnFamily cf; - TreeMap map = new TreeMap(); - for ( int i = 100; i < 1000; ++i ) + TreeMap map = new TreeMap(); + for (int i = 100; i < 1000; ++i) { map.put(Integer.toString(i), ("Avinash Lakshman is a good man: " + i).getBytes()); } @@ -47,7 +49,7 @@ public class ColumnFamilyTest // write cf = new ColumnFamily("Standard1", "Standard"); DataOutputBuffer bufOut = new DataOutputBuffer(); - for (String cName: map.navigableKeySet()) + for (String cName : map.navigableKeySet()) { cf.addColumn(cName, map.get(cName), 314); } @@ -57,60 +59,70 @@ public class ColumnFamilyTest DataInputBuffer bufIn = new DataInputBuffer(); bufIn.reset(bufOut.getData(), bufOut.getLength()); cf = ColumnFamily.serializer().deserialize(bufIn); - for (String cName: map.navigableKeySet()) + for (String cName : map.navigableKeySet()) { assert Arrays.equals(cf.getColumn(cName).value(), map.get(cName)); } assert new HashSet(cf.getColumns().keySet()).equals(map.keySet()); } - - @Test - public void testGetColumnCount() { - ColumnFamily cf = new ColumnFamily("Standard1", "Standard"); - byte val[] = "sample value".getBytes(); - - cf.addColumn("col1", val, 1); - cf.addColumn("col2", val, 2); - cf.addColumn("col1", val, 3); - assert 2 == cf.getColumnCount(); - assert 2 == cf.getAllColumns().size(); - } - @Test - public void testTimestamp() { - ColumnFamily cf = new ColumnFamily("Standard1", "Standard"); - byte val1[] = "sample 1".getBytes(); + public void testGetColumnCount() + { + ColumnFamily cf = new ColumnFamily("Standard1", "Standard"); + byte val[] = "sample value".getBytes(); + + cf.addColumn("col1", val, 1); + cf.addColumn("col2", val, 2); + cf.addColumn("col1", val, 3); + + assert 2 == cf.getColumnCount(); + assert 2 == cf.getAllColumns().size(); + } + + @Test + public void testTimestamp() + { + ColumnFamily cf = new ColumnFamily("Standard1", "Standard"); + byte val1[] = "sample 1".getBytes(); byte val2[] = "sample 2".getBytes(); byte val3[] = "sample 3".getBytes(); - cf.addColumn("col1", val1, 2); + cf.addColumn("col1", val1, 2); cf.addColumn("col1", val2, 2); // same timestamp, new value cf.addColumn("col1", val3, 1); // older timestamp -- should be ignored assert Arrays.equals(val2, cf.getColumn("col1").value()); } - + @Test - public void testMergeAndAdd(){ - ColumnFamily cf_new = new ColumnFamily("Standard1", "Standard"); - ColumnFamily cf_old = new ColumnFamily("Standard1", "Standard"); - ColumnFamily cf_result = new ColumnFamily("Standard1", "Standard"); - byte val[] = "sample value".getBytes(); - byte val2[] = "x value ".getBytes(); - - cf_new.addColumn("col1", val, 3); - cf_new.addColumn("col2", val, 4); + public void testMergeAndAdd() + { + ColumnFamily cf_new = new ColumnFamily("Standard1", "Standard"); + ColumnFamily cf_old = new ColumnFamily("Standard1", "Standard"); + ColumnFamily cf_result = new ColumnFamily("Standard1", "Standard"); + byte val[] = "sample value".getBytes(); + byte val2[] = "x value ".getBytes(); - cf_old.addColumn("col2", val2, 1); - cf_old.addColumn("col3", val2, 2); + cf_new.addColumn("col1", val, 3); + cf_new.addColumn("col2", val, 4); - cf_result.addColumns(cf_new); - cf_result.addColumns(cf_old); - - assert 3 == cf_result.getColumnCount() : "Count is " + cf_new.getColumnCount(); - //addcolumns will only add if timestamp >= old timestamp + cf_old.addColumn("col2", val2, 1); + cf_old.addColumn("col3", val2, 2); + + cf_result.addColumns(cf_new); + cf_result.addColumns(cf_old); + + assert 3 == cf_result.getColumnCount() : "Count is " + cf_new.getColumnCount(); + //addcolumns will only add if timestamp >= old timestamp assert Arrays.equals(val, cf_result.getColumn("col2").value()); } + + @Test + public void testEmptyDigest() + { + ColumnFamily cf = new ColumnFamily("Standard1", "Standard"); + assert cf.digest().length == 0; + } }