Return-Path: Delivered-To: apmail-hadoop-hbase-commits-archive@minotaur.apache.org Received: (qmail 42768 invoked from network); 17 May 2009 19:18:27 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 17 May 2009 19:18:27 -0000 Received: (qmail 86279 invoked by uid 500); 17 May 2009 19:18:27 -0000 Delivered-To: apmail-hadoop-hbase-commits-archive@hadoop.apache.org Received: (qmail 86233 invoked by uid 500); 17 May 2009 19:18:27 -0000 Mailing-List: contact hbase-commits-help@hadoop.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: hbase-dev@hadoop.apache.org Delivered-To: mailing list hbase-commits@hadoop.apache.org Received: (qmail 86224 invoked by uid 99); 17 May 2009 19:18:26 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 17 May 2009 19:18:26 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Sun, 17 May 2009 19:18:22 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 3995523888A6; Sun, 17 May 2009 19:18:01 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r775739 - in /hadoop/hbase/trunk: CHANGES.txt src/java/org/apache/hadoop/hbase/regionserver/HRegion.java src/test/org/apache/hadoop/hbase/client/TestHTable.java Date: Sun, 17 May 2009 19:18:01 -0000 To: hbase-commits@hadoop.apache.org From: apurtell@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20090517191801.3995523888A6@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: apurtell Date: Sun May 17 19:18:00 2009 New Revision: 775739 URL: http://svn.apache.org/viewvc?rev=775739&view=rev Log: HBASE-1431 NPE in HTable.checkAndSave when row doesn't exist Modified: hadoop/hbase/trunk/CHANGES.txt hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/client/TestHTable.java Modified: hadoop/hbase/trunk/CHANGES.txt URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/CHANGES.txt?rev=775739&r1=775738&r2=775739&view=diff ============================================================================== --- hadoop/hbase/trunk/CHANGES.txt (original) +++ hadoop/hbase/trunk/CHANGES.txt Sun May 17 19:18:00 2009 @@ -131,6 +131,8 @@ HBASE-1323 hbase-1234 broke TestThriftServer; fix and reenable HBASE-1425 ColumnValueFilter and WhileMatchFilter fixes on trunk (Clint Morgan via Stack) + HBASE-1431 NPE in HTable.checkAndSave when row doesn't exist (Guilherme + Mauro Germoglio Barbosa via Andrew Purtell) IMPROVEMENTS HBASE-1089 Add count of regions on filesystem to master UI; add percentage Modified: hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java?rev=775739&r1=775738&r2=775739&view=diff ============================================================================== --- hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java (original) +++ hadoop/hbase/trunk/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java Sun May 17 19:18:00 2009 @@ -1396,13 +1396,17 @@ Map actualValues = getFull(row, keySet, HConstants.LATEST_TIMESTAMP, 1,lid); for (byte[] key : keySet) { - // If test fails exit - if(!Bytes.equals(actualValues.get(key).getValue(), - expectedValues.get(key))) { - success = false; - break; - } - } + // If test fails exit + Cell cell = actualValues.get(key); + byte[] actualValue = new byte[] {}; + if (cell != null) + actualValue = cell.getValue(); + if(!Bytes.equals(actualValue, + expectedValues.get(key))) { + success = false; + break; + } + } if (success) { long commitTime = (b.getTimestamp() == LATEST_TIMESTAMP)? now: b.getTimestamp(); Modified: hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/client/TestHTable.java URL: http://svn.apache.org/viewvc/hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/client/TestHTable.java?rev=775739&r1=775738&r2=775739&view=diff ============================================================================== --- hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/client/TestHTable.java (original) +++ hadoop/hbase/trunk/src/test/org/apache/hadoop/hbase/client/TestHTable.java Sun May 17 19:18:00 2009 @@ -235,12 +235,24 @@ BatchUpdate batchUpdate = new BatchUpdate(row); BatchUpdate batchUpdate2 = new BatchUpdate(row); BatchUpdate batchUpdate3 = new BatchUpdate(row); + + // this row doesn't exist when checkAndSave is invoked + byte [] row1 = Bytes.toBytes("row1"); + BatchUpdate batchUpdate4 = new BatchUpdate(row1); + // to be used for a checkAndSave for expected empty columns + BatchUpdate batchUpdate5 = new BatchUpdate(row); + HbaseMapWritable expectedValues = new HbaseMapWritable(); HbaseMapWritable badExpectedValues = new HbaseMapWritable(); - + HbaseMapWritable expectedNoValues = + new HbaseMapWritable(); + // the columns used here must not be updated on batchupate + HbaseMapWritable expectedNoValues1 = + new HbaseMapWritable(); + for(int i = 0; i < 5; i++) { // This batchupdate is our initial batch update, // As such we also set our expected values to the same values @@ -250,7 +262,12 @@ badExpectedValues.put(Bytes.toBytes(COLUMN_FAMILY_STR+i), Bytes.toBytes(500)); - + + expectedNoValues.put(Bytes.toBytes(COLUMN_FAMILY_STR+i), new byte[] {}); + // the columns used here must not be updated on batchupate + expectedNoValues1.put(Bytes.toBytes(COLUMN_FAMILY_STR+i+","+i), new byte[] {}); + + // This is our second batchupdate that we will use to update the initial // batchupdate batchUpdate2.put(COLUMN_FAMILY_STR+i, Bytes.toBytes(i+1)); @@ -258,6 +275,14 @@ // This final batch update is to check that our expected values (which // are now wrong) batchUpdate3.put(COLUMN_FAMILY_STR+i, Bytes.toBytes(i+2)); + + // Batch update that will not happen because it is to happen with some + // expected values, but the row doesn't exist + batchUpdate4.put(COLUMN_FAMILY_STR+i, Bytes.toBytes(i)); + + // Batch update will happen: the row exists, but the expected columns don't, + // just as the condition + batchUpdate5.put(COLUMN_FAMILY_STR+i, Bytes.toBytes(i+3)); } // Initialize rows @@ -279,6 +304,58 @@ // make sure that the old expected values fail assertFalse(table.checkAndSave(batchUpdate3, expectedValues,null)); + + // row doesn't exist, so doesn't matter the expected + // values (unless they are empty) + assertFalse(table.checkAndSave(batchUpdate4, badExpectedValues, null)); + + assertTrue(table.checkAndSave(batchUpdate4, expectedNoValues, null)); + // make sure check and save saves the data when expected values were empty and the row + // didn't exist + r = table.getRow(row1); + columns = batchUpdate4.getColumns(); + for(int i = 0; i < columns.length;i++) { + assertTrue(Bytes.equals(r.get(columns[i]).getValue(),batchUpdate4.get(columns[i]))); + } + + // since the row isn't empty anymore, those expected (empty) values + // are not valid anymore, so check and save method doesn't save. + assertFalse(table.checkAndSave(batchUpdate4, expectedNoValues, null)); + + // the row exists, but the columns don't. since the expected values are + // for columns without value, checkAndSave must be successful. + assertTrue(table.checkAndSave(batchUpdate5, expectedNoValues1, null)); + // make sure checkAndSave saved values for batchUpdate5. + r = table.getRow(row); + columns = batchUpdate5.getColumns(); + for(int i = 0; i < columns.length;i++) { + assertTrue(Bytes.equals(r.get(columns[i]).getValue(),batchUpdate5.get(columns[i]))); + } + + // since the condition wasn't changed, the following checkAndSave + // must also be successful. + assertTrue(table.checkAndSave(batchUpdate, expectedNoValues1, null)); + // make sure checkAndSave saved values for batchUpdate1 + r = table.getRow(row); + columns = batchUpdate.getColumns(); + for(int i = 0; i < columns.length;i++) { + assertTrue(Bytes.equals(r.get(columns[i]).getValue(),batchUpdate.get(columns[i]))); + } + + // one failing condition must make the following checkAndSave fail + // the failing condition is a column to be empty, however, it has a value. + HbaseMapWritable expectedValues1 = + new HbaseMapWritable(); + expectedValues1.put(Bytes.toBytes(COLUMN_FAMILY_STR+0), new byte[] {}); + expectedValues1.put(Bytes.toBytes(COLUMN_FAMILY_STR+"EMPTY+ROW"), new byte[] {}); + assertFalse(table.checkAndSave(batchUpdate5, expectedValues1, null)); + + // assure the values on the row remain the same + r = table.getRow(row); + columns = batchUpdate.getColumns(); + for(int i = 0; i < columns.length;i++) { + assertTrue(Bytes.equals(r.get(columns[i]).getValue(),batchUpdate.get(columns[i]))); + } } /** @@ -373,5 +450,5 @@ fail("Should have thrown a TableNotFoundException instead of a " + e.getClass()); } - } + } }