diff --git a/CHANGES.txt b/CHANGES.txt index 426aaf9a2ae..e9fab9c195c 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -99,6 +99,8 @@ Release 0.20.0 - Unreleased (Evgeny Ryabitskiy via Stack) HBASE-1322 hbase-1234 broke TestAtomicIncrement; fix and reenable (Evgeny Ryabitskiy and Ryan Rawson via Stack) + HBASE-1347 HTable.incrementColumnValue does not take negative 'amount' + (Evgeny Ryabitskiy via Stack) IMPROVEMENTS HBASE-1089 Add count of regions on filesystem to master UI; add percentage diff --git a/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java index f3df39b63ea..0529ad56bd2 100644 --- a/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/src/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -2701,7 +2701,8 @@ public class HRegion implements HConstants { "/"+ Bytes.toString(column)); value = Bytes.toBytes(amount); } else { - value = incrementBytes(value, amount); + if (amount == 0) return Bytes.toLong(value); + value = Bytes.incrementBytes(value, amount); } BatchUpdate b = new BatchUpdate(row, ts); @@ -2713,34 +2714,4 @@ public class HRegion implements HConstants { releaseRowLock(lid); } } - - private byte [] incrementBytes(byte[] value, long amount) throws IOException { - // Hopefully this doesn't happen too often. - if (value.length < Bytes.SIZEOF_LONG) { - byte [] newvalue = new byte[Bytes.SIZEOF_LONG]; - System.arraycopy(value, 0, newvalue, newvalue.length - value.length, - value.length); - value = newvalue; - } else if (value.length > Bytes.SIZEOF_LONG) { - throw new DoNotRetryIOException("Increment Bytes - value too big: " + - value.length); - } - return binaryIncrement(value, amount); - } - - private byte [] binaryIncrement(byte [] value, long amount) { - for(int i=0;i> (8 * i)) % 256; - int val = value[value.length-i-1] & 0xff; - int total = cur + val; - if(total > 255) { - amount += ((long)256 << (8 * i)); - total %= 256; - } - value[value.length-i-1] = (byte)total; - amount = (amount >> (8 * (i + 1))) << (8 * (i + 1)); - if(amount == 0) return value; - } - return value; - } -} \ No newline at end of file +} diff --git a/src/java/org/apache/hadoop/hbase/util/Bytes.java b/src/java/org/apache/hadoop/hbase/util/Bytes.java index ccc4716c84d..6d59dafb824 100644 --- a/src/java/org/apache/hadoop/hbase/util/Bytes.java +++ b/src/java/org/apache/hadoop/hbase/util/Bytes.java @@ -27,6 +27,7 @@ import java.nio.ByteBuffer; import java.util.Comparator; import java.math.BigInteger; +import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.io.ImmutableBytesWritable; import org.apache.hadoop.io.RawComparator; @@ -909,5 +910,90 @@ public class Bytes { return mid; } return - (low+1); - } -} \ No newline at end of file + } + + /** + * Bytewise binary increment/deincrement of long contained in byte array + * on given amount. + * + * @param value - array of bytes containing long (length <= SIZEOF_LONG) + * @param amount value will be incremented on (deincremented if negative) + * @return array of bytes containing incremented long (length == SIZEOF_LONG) + * @throws IOException - if value.length > SIZEOF_LONG + */ + public static byte [] incrementBytes(byte[] value, long amount) + throws IOException { + byte[] val = value; + if (val.length < SIZEOF_LONG) { + // Hopefully this doesn't happen too often. + byte [] newvalue; + if (val[0] < 0) { + byte [] negativeValue = {-1, -1, -1, -1, -1, -1, -1, -1}; + newvalue = negativeValue; + } else { + newvalue = new byte[SIZEOF_LONG]; + } + System.arraycopy(val, 0, newvalue, newvalue.length - val.length, + val.length); + val = newvalue; + } else if (val.length > SIZEOF_LONG) { + throw new DoNotRetryIOException("Increment Bytes - value too big: " + + val.length); + } + if(amount == 0) return val; + if(val[0] < 0){ + return binaryIncrementNeg(val, amount); + } + return binaryIncrementPos(val, amount); + } + + /* increment/deincrement for positive value */ + private static byte [] binaryIncrementPos(byte [] value, long amount) { + long amo = amount; + int sign = 1; + if (amount < 0) { + amo = -amount; + sign = -1; + } + for(int i=0;i> 8); + int val = value[value.length-i-1] & 0x0ff; + int total = val + cur; + if(total > 255) { + amo += sign; + total %= 256; + } else if (total < 0) { + amo -= sign; + } + value[value.length-i-1] = (byte)total; + if (amo == 0) return value; + } + return value; + } + + /* increment/deincrement for negative value */ + private static byte [] binaryIncrementNeg(byte [] value, long amount) { + long amo = amount; + int sign = 1; + if (amount < 0) { + amo = -amount; + sign = -1; + } + for(int i=0;i> 8); + int val = ((~value[value.length-i-1]) & 0x0ff) + 1; + int total = cur - val; + if(total >= 0) { + amo += sign; + } else if (total < -256) { + amo -= sign; + total %= 256; + } + value[value.length-i-1] = (byte)total; + if (amo == 0) return value; + } + return value; + } +} diff --git a/src/test/org/apache/hadoop/hbase/regionserver/TestAtomicIncrement.java b/src/test/org/apache/hadoop/hbase/regionserver/TestAtomicIncrement.java index 895062babda..651daf10e53 100644 --- a/src/test/org/apache/hadoop/hbase/regionserver/TestAtomicIncrement.java +++ b/src/test/org/apache/hadoop/hbase/regionserver/TestAtomicIncrement.java @@ -88,6 +88,9 @@ public class TestAtomicIncrement extends HBaseClusterTestCase { assertEquals(3L, table.incrementColumnValue(row, column, 1)); assertEquals(-2L, table.incrementColumnValue(row, column, -5)); + assertEquals(-502L, table.incrementColumnValue(row, column, -500)); + assertEquals(1500L, table.incrementColumnValue(row, column, 2002)); + assertEquals(1501L, table.incrementColumnValue(row, column, 1)); row = Bytes.toBytes("foo3"); byte[] value2 = {1,2,3,4,5,6,7,8,9}; diff --git a/src/test/org/apache/hadoop/hbase/util/TestBytes.java b/src/test/org/apache/hadoop/hbase/util/TestBytes.java index 4976d3dbe7e..e919d5f2a50 100644 --- a/src/test/org/apache/hadoop/hbase/util/TestBytes.java +++ b/src/test/org/apache/hadoop/hbase/util/TestBytes.java @@ -19,6 +19,7 @@ */ package org.apache.hadoop.hbase.util; +import java.io.IOException; import java.util.Arrays; import junit.framework.TestCase; @@ -113,4 +114,40 @@ public class TestBytes extends TestCase { assertEquals(5, Bytes.binarySearch(arr, key3, 1, 1, Bytes.BYTES_RAWCOMPARATOR)); } -} \ No newline at end of file + + public void testIncrementBytes() throws IOException { + + assertTrue(checkTestIncrementBytes(10, 1)); + assertTrue(checkTestIncrementBytes(12, 123435445)); + assertTrue(checkTestIncrementBytes(124634654, 1)); + assertTrue(checkTestIncrementBytes(10005460, 5005645)); + assertTrue(checkTestIncrementBytes(1, -1)); + assertTrue(checkTestIncrementBytes(10, -1)); + assertTrue(checkTestIncrementBytes(10, -5)); + assertTrue(checkTestIncrementBytes(1005435000, -5)); + assertTrue(checkTestIncrementBytes(10, -43657655)); + assertTrue(checkTestIncrementBytes(-1, 1)); + assertTrue(checkTestIncrementBytes(-26, 5034520)); + assertTrue(checkTestIncrementBytes(-10657200, 5)); + assertTrue(checkTestIncrementBytes(-12343250, 45376475)); + assertTrue(checkTestIncrementBytes(-10, -5)); + assertTrue(checkTestIncrementBytes(-12343250, -5)); + assertTrue(checkTestIncrementBytes(-12, -34565445)); + assertTrue(checkTestIncrementBytes(-1546543452, -34565445)); + } + + private static boolean checkTestIncrementBytes(long val, long amount) + throws IOException { + byte[] value = Bytes.toBytes(val); + byte [] testValue = {-1, -1, -1, -1, -1, -1, -1, -1}; + if (value[0] > 0) { + testValue = new byte[Bytes.SIZEOF_LONG]; + } + System.arraycopy(value, 0, testValue, testValue.length - value.length, + value.length); + + long incrementResult = Bytes.toLong(Bytes.incrementBytes(value, amount)); + + return (Bytes.toLong(testValue) + amount) == incrementResult; + } +}