Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 333E717BA7 for ; Wed, 15 Apr 2015 15:23:39 +0000 (UTC) Received: (qmail 39070 invoked by uid 500); 15 Apr 2015 15:23:35 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 39011 invoked by uid 500); 15 Apr 2015 15:23:35 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 38980 invoked by uid 99); 15 Apr 2015 15:23:35 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 15 Apr 2015 15:23:35 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 84BF3E050B; Wed, 15 Apr 2015 15:23:35 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: slebresne@apache.org To: commits@cassandra.apache.org Date: Wed, 15 Apr 2015 15:23:35 -0000 Message-Id: <1bfb1318050c4dcc88af878fe39b8e2e@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/6] cassandra git commit: CQLSSTableWriter causes ArrayIndexOutOfBoundsException Repository: cassandra Updated Branches: refs/heads/cassandra-2.1 730d4d4c4 -> 7b5021324 CQLSSTableWriter causes ArrayIndexOutOfBoundsException patch by carlyeks; reviewed by blerer for CASSANDRA-8978 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/b4c1ef92 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/b4c1ef92 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/b4c1ef92 Branch: refs/heads/cassandra-2.1 Commit: b4c1ef92e103d58dcddbae9a9d3885e870676844 Parents: 84b2846 Author: Carl Yeksigian Authored: Wed Apr 15 10:05:32 2015 -0500 Committer: Sylvain Lebresne Committed: Wed Apr 15 10:08:12 2015 -0500 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../io/sstable/AbstractSSTableSimpleWriter.java | 9 +++++++ .../cassandra/io/sstable/CQLSSTableWriter.java | 26 +++++++++++++++++++- .../io/sstable/SSTableSimpleUnsortedWriter.java | 10 ++++++-- .../io/sstable/CQLSSTableWriterTest.java | 12 +++++---- 5 files changed, 50 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/b4c1ef92/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 460b07c..a492c74 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.0.15: + * Fix ArrayIndexOutOfBoundsException in CQLSSTableWriter (CASSANDRA-8978) * Add shutdown gossip state to prevent timeouts during rolling restarts (CASSANDRA-8336) * Fix running with java.net.preferIPv6Addresses=true (CASSANDRA-9137) * Fix failed bootstrap/replace attempts being persisted in system.peers (CASSANDRA-9180) http://git-wip-us.apache.org/repos/asf/cassandra/blob/b4c1ef92/src/java/org/apache/cassandra/io/sstable/AbstractSSTableSimpleWriter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/sstable/AbstractSSTableSimpleWriter.java b/src/java/org/apache/cassandra/io/sstable/AbstractSSTableSimpleWriter.java index af1c43c..d089770 100644 --- a/src/java/org/apache/cassandra/io/sstable/AbstractSSTableSimpleWriter.java +++ b/src/java/org/apache/cassandra/io/sstable/AbstractSSTableSimpleWriter.java @@ -187,6 +187,15 @@ public abstract class AbstractSSTableSimpleWriter implements Closeable return currentKey; } + /** + * Package protected for use by AbstractCQLSSTableWriter. + * Not meant to be exposed publicly. + */ + boolean shouldStartNewRow() throws IOException + { + return currentKey == null; + } + protected abstract void writeRow(DecoratedKey key, ColumnFamily columnFamily) throws IOException; protected abstract ColumnFamily getColumnFamily() throws IOException; http://git-wip-us.apache.org/repos/asf/cassandra/blob/b4c1ef92/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java b/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java index fb4c186..84b2863 100644 --- a/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java +++ b/src/java/org/apache/cassandra/io/sstable/CQLSSTableWriter.java @@ -218,7 +218,7 @@ public class CQLSSTableWriter implements Closeable { for (ByteBuffer key: keys) { - if (writer.currentKey() == null || !key.equals(writer.currentKey().key)) + if (writer.shouldStartNewRow() || !key.equals(writer.currentKey().key)) writer.newRow(key); insert.addUpdateForKey(writer.currentColumnFamily(), key, clusteringPrefix, params, false); } @@ -532,6 +532,8 @@ public class CQLSSTableWriter implements Closeable */ private static class BufferedWriter extends SSTableSimpleUnsortedWriter { + private boolean needsSync = false; + public BufferedWriter(File directory, CFMetaData metadata, IPartitioner partitioner, long bufferSizeInMB) { super(directory, metadata, partitioner, bufferSizeInMB); @@ -560,6 +562,28 @@ public class CQLSSTableWriter implements Closeable }; } + @Override + protected void replaceColumnFamily() throws IOException + { + needsSync = true; + } + + @Override + /** + * If we have marked that the column family is being replaced, when we start the next row, + * we should sync out the previous partition and create a new row based on the current value. + */ + boolean shouldStartNewRow() throws IOException + { + if (needsSync) + { + needsSync = false; + super.sync(); + return true; + } + return super.shouldStartNewRow(); + } + protected void addColumn(Column column) throws IOException { throw new UnsupportedOperationException(); http://git-wip-us.apache.org/repos/asf/cassandra/blob/b4c1ef92/src/java/org/apache/cassandra/io/sstable/SSTableSimpleUnsortedWriter.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/sstable/SSTableSimpleUnsortedWriter.java b/src/java/org/apache/cassandra/io/sstable/SSTableSimpleUnsortedWriter.java index 5bddea3..955214d 100644 --- a/src/java/org/apache/cassandra/io/sstable/SSTableSimpleUnsortedWriter.java +++ b/src/java/org/apache/cassandra/io/sstable/SSTableSimpleUnsortedWriter.java @@ -119,7 +119,7 @@ public class SSTableSimpleUnsortedWriter extends AbstractSSTableSimpleWriter // We don't want to sync in writeRow() only as this might blow up the bufferSize for wide rows. if (currentSize > bufferSize) - sync(); + replaceColumnFamily(); } protected ColumnFamily getColumnFamily() throws IOException @@ -159,7 +159,13 @@ public class SSTableSimpleUnsortedWriter extends AbstractSSTableSimpleWriter } } - private void sync() throws IOException + // This is overridden by CQLSSTableWriter to hold off replacing column family until the next iteration through + protected void replaceColumnFamily() throws IOException + { + sync(); + } + + protected void sync() throws IOException { if (buffer.isEmpty()) return; http://git-wip-us.apache.org/repos/asf/cassandra/blob/b4c1ef92/test/unit/org/apache/cassandra/io/sstable/CQLSSTableWriterTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/io/sstable/CQLSSTableWriterTest.java b/test/unit/org/apache/cassandra/io/sstable/CQLSSTableWriterTest.java index 0922502..94fe309 100644 --- a/test/unit/org/apache/cassandra/io/sstable/CQLSSTableWriterTest.java +++ b/test/unit/org/apache/cassandra/io/sstable/CQLSSTableWriterTest.java @@ -138,10 +138,12 @@ public class CQLSSTableWriterTest // > 1MB and validate that this created more than 1 sstable. File tempdir = Files.createTempDir(); String schema = "CREATE TABLE ks.test (" - + " k int PRIMARY KEY," - + " v blob" + + " k int," + + " c int," + + " v blob," + + " PRIMARY KEY (k,c)" + ")"; - String insert = "INSERT INTO ks.test (k, v) VALUES (?, ?)"; + String insert = "INSERT INTO ks.test (k, c, v) VALUES (?, ?, ?)"; CQLSSTableWriter writer = CQLSSTableWriter.builder() .inDirectory(tempdir) .forTable(schema) @@ -152,8 +154,8 @@ public class CQLSSTableWriterTest ByteBuffer val = ByteBuffer.allocate(1024 * 1050); - writer.addRow(0, val); - writer.addRow(1, val); + writer.addRow(0, 0, val); + writer.addRow(0, 1, val); writer.close(); FilenameFilter filterDataFiles = new FilenameFilter()