diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java index 79eae20..642c131 100644 --- hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java +++ hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java @@ -111,7 +111,7 @@ public class Delete extends Mutation implements Comparable { public Delete(final byte [] rowArray, final int rowOffset, final int rowLength, long ts) { checkRow(rowArray, rowOffset, rowLength); this.row = Bytes.copy(rowArray, rowOffset, rowLength); - this.ts = ts; + setTimestamp(ts); } /** @@ -180,6 +180,9 @@ public class Delete extends Mutation implements Comparable { */ @SuppressWarnings("unchecked") public Delete deleteFamily(byte [] family, long timestamp) { + if (timestamp < 0) { + throw new IllegalArgumentException("Timestamp cannot be negative"); + } List list = familyMap.get(family); if(list == null) { list = new ArrayList(); @@ -215,6 +218,9 @@ public class Delete extends Mutation implements Comparable { */ @SuppressWarnings("unchecked") public Delete deleteColumns(byte [] family, byte [] qualifier, long timestamp) { + if (timestamp < 0) { + throw new IllegalArgumentException("Timestamp cannot be negative"); + } List list = familyMap.get(family); if (list == null) { list = new ArrayList(); @@ -250,6 +256,9 @@ public class Delete extends Mutation implements Comparable { */ @SuppressWarnings("unchecked") public Delete deleteColumn(byte [] family, byte [] qualifier, long timestamp) { + if (timestamp < 0) { + throw new IllegalArgumentException("Timestamp cannot be negative"); + } List list = familyMap.get(family); if(list == null) { list = new ArrayList(); @@ -264,10 +273,13 @@ public class Delete extends Mutation implements Comparable { /** * Set the timestamp of the delete. - * + * * @param timestamp */ public void setTimestamp(long timestamp) { + if (timestamp < 0) { + throw new IllegalArgumentException("Timestamp cannot be negative"); + } this.ts = timestamp; } diff --git hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java index be028c6..1b8a20e 100644 --- hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java +++ hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java @@ -74,6 +74,9 @@ public class Put extends Mutation implements HeapSize, Comparable { checkRow(rowArray, rowOffset, rowLength); this.row = Bytes.copy(rowArray, rowOffset, rowLength); this.ts = ts; + if (ts < 0) { + throw new IllegalArgumentException("Timestamp cannot be negative"); + } } /** @@ -111,6 +114,9 @@ public class Put extends Mutation implements HeapSize, Comparable { */ @SuppressWarnings("unchecked") public Put add(byte [] family, byte [] qualifier, long ts, byte [] value) { + if (ts < 0) { + throw new IllegalArgumentException("Timestamp cannot be negative"); + } List list = getCellList(family); KeyValue kv = createPutKeyValue(family, qualifier, ts, value); ((List)list).add(kv); diff --git hbase-common/src/main/java/org/apache/hadoop/hbase/Cell.java hbase-common/src/main/java/org/apache/hadoop/hbase/Cell.java index 662eef3..cb18d80 100644 --- hbase-common/src/main/java/org/apache/hadoop/hbase/Cell.java +++ hbase-common/src/main/java/org/apache/hadoop/hbase/Cell.java @@ -124,7 +124,7 @@ public interface Cell { /** * @return Long value representing time at which this cell was "Put" into the row. Typically - * represents the time of insertion, but can be any value from Long.MIN_VALUE to Long.MAX_VALUE. + * represents the time of insertion, but can be any value from 0 to Long.MAX_VALUE. */ long getTimestamp(); diff --git hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java index b87ce4f..4910d6a 100644 --- hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java +++ hbase-common/src/main/java/org/apache/hadoop/hbase/io/TimeRange.java @@ -72,6 +72,9 @@ public class TimeRange { */ public TimeRange(long minStamp, long maxStamp) throws IOException { + if (minStamp < 0 || maxStamp < 0) { + throw new IllegalArgumentException("Timestamp cannot be negative"); + } if(maxStamp < minStamp) { throw new IOException("maxStamp is smaller than minStamp"); } diff --git hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java index dbf599a..3dfde86 100644 --- hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java +++ hbase-server/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java @@ -52,8 +52,8 @@ import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.ServerName; -import org.apache.hadoop.hbase.exceptions.TableNotFoundException; import org.apache.hadoop.hbase.client.Delete; +import org.apache.hadoop.hbase.client.Durability; import org.apache.hadoop.hbase.client.Get; import org.apache.hadoop.hbase.client.HBaseAdmin; import org.apache.hadoop.hbase.client.HTable; @@ -63,7 +63,7 @@ import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.ResultScanner; import org.apache.hadoop.hbase.client.Scan; -import org.apache.hadoop.hbase.client.Durability; +import org.apache.hadoop.hbase.exceptions.TableNotFoundException; import org.apache.hadoop.hbase.filter.Filter; import org.apache.hadoop.hbase.filter.ParseFilter; import org.apache.hadoop.hbase.filter.PrefixFilter; @@ -298,7 +298,7 @@ public class ThriftServerRunner implements Runnable { if (implType == ImplType.HS_HA || implType == ImplType.NONBLOCKING || implType == ImplType.THREADED_SELECTOR) { - InetAddress listenAddress = getBindAddress(conf); + InetAddress listenAddress = getBindAddress(conf); TNonblockingServerTransport serverTransport = new TNonblockingServerSocket( new InetSocketAddress(listenAddress, listenPort)); @@ -365,7 +365,7 @@ public class ThriftServerRunner implements Runnable { tserver.getClass().getName()); } - + registerFilters(conf); } @@ -716,7 +716,7 @@ public class ThriftServerRunner implements Runnable { Get get = new Get(getBytes(row)); addAttributes(get, attributes); get.addColumn(family, qualifier); - get.setTimeRange(Long.MIN_VALUE, timestamp); + get.setTimeRange(0, timestamp); get.setMaxVersions(numVersions); Result result = table.get(get); return ThriftUtilities.cellFromHBase(result.raw()); @@ -760,7 +760,7 @@ public class ThriftServerRunner implements Runnable { if (columns == null) { Get get = new Get(getBytes(row)); addAttributes(get, attributes); - get.setTimeRange(Long.MIN_VALUE, timestamp); + get.setTimeRange(0, timestamp); Result result = table.get(get); return ThriftUtilities.rowResultFromHBase(result); } @@ -774,7 +774,7 @@ public class ThriftServerRunner implements Runnable { get.addColumn(famAndQf[0], famAndQf[1]); } } - get.setTimeRange(Long.MIN_VALUE, timestamp); + get.setTimeRange(0, timestamp); Result result = table.get(get); return ThriftUtilities.rowResultFromHBase(result); } catch (IOException e) { @@ -837,7 +837,7 @@ public class ThriftServerRunner implements Runnable { } } } - get.setTimeRange(Long.MIN_VALUE, timestamp); + get.setTimeRange(0, timestamp); gets.add(get); } Result[] result = table.get(gets); @@ -1064,7 +1064,7 @@ public class ThriftServerRunner implements Runnable { table.put(puts); if (!deletes.isEmpty()) table.delete(deletes); - + } catch (IOException e) { LOG.warn(e.getMessage(), e); throw new IOError(e.getMessage()); @@ -1101,6 +1101,7 @@ public class ThriftServerRunner implements Runnable { } } + @Override public void scannerClose(int id) throws IOError, IllegalArgument { LOG.debug("scannerClose: id=" + id); ResultScanner scanner = getScanner(id); @@ -1142,6 +1143,7 @@ public class ThriftServerRunner implements Runnable { return scannerGetList(id,1); } + @Override public int scannerOpenWithScan(ByteBuffer tableName, TScan tScan, Map attributes) throws IOError { @@ -1156,7 +1158,7 @@ public class ThriftServerRunner implements Runnable { scan.setStopRow(tScan.getStopRow()); } if (tScan.isSetTimestamp()) { - scan.setTimeRange(Long.MIN_VALUE, tScan.getTimestamp()); + scan.setTimeRange(0, tScan.getTimestamp()); } if (tScan.isSetCaching()) { scan.setCaching(tScan.getCaching()); @@ -1275,7 +1277,7 @@ public class ThriftServerRunner implements Runnable { HTable table = getTable(tableName); Scan scan = new Scan(getBytes(startRow)); addAttributes(scan, attributes); - scan.setTimeRange(Long.MIN_VALUE, timestamp); + scan.setTimeRange(0, timestamp); if (columns != null && columns.size() != 0) { for (ByteBuffer column : columns) { byte [][] famQf = KeyValue.parseColumn(getBytes(column)); @@ -1302,7 +1304,7 @@ public class ThriftServerRunner implements Runnable { HTable table = getTable(tableName); Scan scan = new Scan(getBytes(startRow), getBytes(stopRow)); addAttributes(scan, attributes); - scan.setTimeRange(Long.MIN_VALUE, timestamp); + scan.setTimeRange(0, timestamp); if (columns != null && columns.size() != 0) { for (ByteBuffer column : columns) { byte [][] famQf = KeyValue.parseColumn(getBytes(column)); @@ -1313,7 +1315,7 @@ public class ThriftServerRunner implements Runnable { } } } - scan.setTimeRange(Long.MIN_VALUE, timestamp); + scan.setTimeRange(0, timestamp); return addScanner(table.getScanner(scan)); } catch (IOException e) { LOG.warn(e.getMessage(), e); diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java index a6dd746..c333550 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java @@ -63,6 +63,8 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.client.metrics.ScanMetrics; import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.hadoop.hbase.coprocessor.MultiRowMutationEndpoint; +import org.apache.hadoop.hbase.exceptions.DoNotRetryIOException; +import org.apache.hadoop.hbase.exceptions.NoSuchColumnFamilyException; import org.apache.hadoop.hbase.filter.BinaryComparator; import org.apache.hadoop.hbase.filter.CompareFilter; import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp; @@ -87,8 +89,6 @@ import org.apache.hadoop.hbase.protobuf.generated.MultiRowMutation.MultiMutateRe import org.apache.hadoop.hbase.protobuf.generated.MultiRowMutation.MultiRowMutationService; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HRegionServer; -import org.apache.hadoop.hbase.exceptions.DoNotRetryIOException; -import org.apache.hadoop.hbase.exceptions.NoSuchColumnFamilyException; import org.apache.hadoop.hbase.regionserver.Store; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; @@ -4950,5 +4950,64 @@ public class TestFromClientSide { assertEquals(1, bar.length); assertEquals(2, bar[0].size()); } + + @Test + public void testNegativeTimestamp() throws IOException { + HTable table = TEST_UTIL.createTable(Bytes.toBytes("testNegativeTimestamp"), FAMILY); + + try { + Put put = new Put(ROW, -1); + put.add(FAMILY, QUALIFIER, VALUE); + table.put(put); + fail("Negative timestamps should not have been allowed"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("negative")); + } + + try { + Put put = new Put(ROW); + put.add(FAMILY, QUALIFIER, -1, VALUE); + table.put(put); + fail("Negative timestamps should not have been allowed"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("negative")); + } + + try { + Delete delete = new Delete(ROW, -1); + table.delete(delete); + fail("Negative timestamps should not have been allowed"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("negative")); + } + + try { + Delete delete = new Delete(ROW); + delete.deleteFamily(FAMILY, -1); + table.delete(delete); + fail("Negative timestamps should not have been allowed"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("negative")); + } + + try { + Scan scan = new Scan(); + scan.setTimeRange(-1, 1); + table.getScanner(scan); + fail("Negative timestamps should not have been allowed"); + } catch (IllegalArgumentException ex) { + assertTrue(ex.getMessage().contains("negative")); + } + + // KeyValue should allow negative timestamps for backwards compat. Otherwise, if the user + // already has negative timestamps in cluster data, HBase won't be able to handle that + try { + new KeyValue(Bytes.toBytes(42), Bytes.toBytes(42), Bytes.toBytes(42), -1, Bytes.toBytes(42)); + } catch (IllegalArgumentException ex) { + fail("KeyValue SHOULD allow negative timestamps"); + } + + table.close(); + } } diff --git hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogFiltering.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogFiltering.java index df5a6a6..c80616f 100644 --- hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogFiltering.java +++ hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestHLogFiltering.java @@ -84,7 +84,7 @@ public class TestHLogFiltering { Delete del = new Delete(row); for (int iCol = 0; iCol < 10; ++iCol) { final byte[] cf = rand.nextBoolean() ? CF1 : CF2; - final long ts = rand.nextInt(); + final long ts = Math.abs(rand.nextInt()); final byte[] qual = Bytes.toBytes("col" + iCol); if (rand.nextBoolean()) { final byte[] value = Bytes.toBytes("value_for_row_" + iRow +