From 571e8f8520be236663536500d0e004a2d8b19f2c Mon Sep 17 00:00:00 2001 From: larsh Date: Fri, 11 Nov 2011 23:19:28 +0000 Subject: [PATCH] HBASE-3433 Remove the KV copy of every KV in Scan; introduced by HBASE-3232 git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1201094 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES.txt | 3 ++- .../org/apache/hadoop/hbase/KeyValue.java | 14 ++++---------- .../apache/hadoop/hbase/filter/Filter.java | 19 +++++++++++++++++-- .../hadoop/hbase/filter/FilterBase.java | 12 +++++++++++- .../hadoop/hbase/filter/FilterList.java | 9 +++++++++ .../hadoop/hbase/filter/KeyOnlyFilter.java | 5 ++--- .../hadoop/hbase/filter/SkipFilter.java | 5 +++++ .../hadoop/hbase/filter/WhileMatchFilter.java | 5 +++++ .../hbase/regionserver/ScanQueryMatcher.java | 8 ++++++++ .../hbase/regionserver/StoreScanner.java | 10 +++++----- .../org/apache/hadoop/hbase/TestKeyValue.java | 6 ++---- 11 files changed, 70 insertions(+), 26 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 455b3ab9bd4..5b3434b55c0 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -75,8 +75,9 @@ Release 0.92.0 - Unreleased (Akash Ashok) HBASE-4503 Purge deprecated HBaseClusterTestCase HBASE-4374 Up default regions size from 256M to 1G - HBASE-4648 Bytes.toBigDecimal() doesn't use offset (Brian Keller via Lars H) + HBASE-4648 Bytes.toBigDecimal() doesn't use offset (Bryan Keller via Lars H) HBASE-4715 Remove stale broke .rb scripts from bin dir + HBASE-3433 Remove the KV copy of every KV in Scan; introduced by HBASE-3232 (Lars H) BUG FIXES HBASE-3280 YouAreDeadException being swallowed in HRS getMaster diff --git a/src/main/java/org/apache/hadoop/hbase/KeyValue.java b/src/main/java/org/apache/hadoop/hbase/KeyValue.java index e68e486a11f..be7e2d89455 100644 --- a/src/main/java/org/apache/hadoop/hbase/KeyValue.java +++ b/src/main/java/org/apache/hadoop/hbase/KeyValue.java @@ -1313,15 +1313,11 @@ public class KeyValue implements Writable, HeapSize { } /** - * Converts this KeyValue to only contain the key portion (the value is - * changed to be null). This method does a full copy of the backing byte - * array and does not modify the original byte array of this KeyValue. - *

- * This method is used by KeyOnlyFilter and is an advanced feature of - * KeyValue, proceed with caution. + * Creates a new KeyValue that only contains the key portion (the value is + * set to be null). * @param lenAsVal replace value with the actual value length (false=empty) */ - public void convertToKeyOnly(boolean lenAsVal) { + public KeyValue createKeyOnly(boolean lenAsVal) { // KV format: // Rebuild as: <0:4> int dataLen = lenAsVal? Bytes.SIZEOF_INT : 0; @@ -1332,9 +1328,7 @@ public class KeyValue implements Writable, HeapSize { if (lenAsVal) { Bytes.putInt(newBuffer, newBuffer.length - dataLen, this.getValueLength()); } - this.bytes = newBuffer; - this.offset = 0; - this.length = newBuffer.length; + return new KeyValue(newBuffer); } /** diff --git a/src/main/java/org/apache/hadoop/hbase/filter/Filter.java b/src/main/java/org/apache/hadoop/hbase/filter/Filter.java index e6408b7ab31..02ea5f5c97c 100644 --- a/src/main/java/org/apache/hadoop/hbase/filter/Filter.java +++ b/src/main/java/org/apache/hadoop/hbase/filter/Filter.java @@ -87,7 +87,22 @@ public interface Filter extends Writable { * @return code as described below * @see Filter.ReturnCode */ - public ReturnCode filterKeyValue(KeyValue v); + public ReturnCode filterKeyValue(final KeyValue v); + + /** + * Give the filter a chance to transform the passed KeyValue. + * If the KeyValue is changed a new KeyValue object must be returned. + * @see org.apache.hadoop.hbase.KeyValue#shallowCopy() + * + * The transformed KeyValue is what is eventually returned to the + * client. Most filters will return the passed KeyValue unchanged. + * @see org.apache.hadoop.hbase.filter.KeyOnlyFilter#transform(KeyValue) + * for an example of a transformation. + * + * @param v the KeyValue in question + * @return the changed KeyValue + */ + public KeyValue transform(final KeyValue v); /** * Return codes for filterValue(). @@ -147,5 +162,5 @@ public interface Filter extends Writable { * @return KeyValue which must be next seeked. return null if the filter is * not sure which key to seek to next. */ - public KeyValue getNextKeyHint(KeyValue currentKV); + public KeyValue getNextKeyHint(final KeyValue currentKV); } diff --git a/src/main/java/org/apache/hadoop/hbase/filter/FilterBase.java b/src/main/java/org/apache/hadoop/hbase/filter/FilterBase.java index 2803516a2a0..0d1b12339c1 100644 --- a/src/main/java/org/apache/hadoop/hbase/filter/FilterBase.java +++ b/src/main/java/org/apache/hadoop/hbase/filter/FilterBase.java @@ -77,6 +77,16 @@ public abstract class FilterBase implements Filter { return ReturnCode.INCLUDE; } + /** + * By default no transformation takes place + * + * @inheritDoc + */ + @Override + public KeyValue transform(KeyValue v) { + return v; + } + /** * Filters that never filter by modifying the returned List of KeyValues can * inherit this implementation that does nothing. @@ -128,5 +138,5 @@ public abstract class FilterBase implements Filter { */ public static Filter createFilterFromArguments(ArrayList filterArguments) { throw new IllegalArgumentException("This method has not been implemented"); -} + } } diff --git a/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java b/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java index 6a5bd72296a..216d0dbdc1b 100644 --- a/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java +++ b/src/main/java/org/apache/hadoop/hbase/filter/FilterList.java @@ -180,6 +180,15 @@ public class FilterList implements Filter { return operator == Operator.MUST_PASS_ONE; } + @Override + public KeyValue transform(KeyValue v) { + KeyValue current = v; + for (Filter filter : filters) { + current = filter.transform(current); + } + return current; + } + @Override public ReturnCode filterKeyValue(KeyValue v) { ReturnCode rc = operator == Operator.MUST_PASS_ONE? diff --git a/src/main/java/org/apache/hadoop/hbase/filter/KeyOnlyFilter.java b/src/main/java/org/apache/hadoop/hbase/filter/KeyOnlyFilter.java index 9f99e2cafe1..b2eb3a5fffb 100644 --- a/src/main/java/org/apache/hadoop/hbase/filter/KeyOnlyFilter.java +++ b/src/main/java/org/apache/hadoop/hbase/filter/KeyOnlyFilter.java @@ -43,9 +43,8 @@ public class KeyOnlyFilter extends FilterBase { public KeyOnlyFilter(boolean lenAsVal) { this.lenAsVal = lenAsVal; } @Override - public ReturnCode filterKeyValue(KeyValue kv) { - kv.convertToKeyOnly(this.lenAsVal); - return ReturnCode.INCLUDE; + public KeyValue transform(KeyValue kv) { + return kv.createKeyOnly(this.lenAsVal); } public static Filter createFilterFromArguments(ArrayList filterArguments) { diff --git a/src/main/java/org/apache/hadoop/hbase/filter/SkipFilter.java b/src/main/java/org/apache/hadoop/hbase/filter/SkipFilter.java index 72a91bbf97a..8be40eece55 100644 --- a/src/main/java/org/apache/hadoop/hbase/filter/SkipFilter.java +++ b/src/main/java/org/apache/hadoop/hbase/filter/SkipFilter.java @@ -76,6 +76,11 @@ public class SkipFilter extends FilterBase { return c; } + @Override + public KeyValue transform(KeyValue v) { + return filter.transform(v); + } + public boolean filterRow() { return filterRow; } diff --git a/src/main/java/org/apache/hadoop/hbase/filter/WhileMatchFilter.java b/src/main/java/org/apache/hadoop/hbase/filter/WhileMatchFilter.java index bad3c68ad9b..b9fa92787ff 100644 --- a/src/main/java/org/apache/hadoop/hbase/filter/WhileMatchFilter.java +++ b/src/main/java/org/apache/hadoop/hbase/filter/WhileMatchFilter.java @@ -75,6 +75,11 @@ public class WhileMatchFilter extends FilterBase { return c; } + @Override + public KeyValue transform(KeyValue v) { + return filter.transform(v); + } + public boolean filterRow() { boolean filterRow = this.filter.filterRow(); changeFAR(filterRow); diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java b/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java index a87f19636a7..acea2f2201e 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java @@ -364,6 +364,14 @@ public class ScanQueryMatcher { return this.startKey; } + /** + * + * @return the Filter + */ + Filter getFilter() { + return this.filter; + } + public KeyValue getNextKeyHint(KeyValue kv) { if (filter == null) { return null; diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java index aaa289ed72f..6512a54f78f 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java @@ -26,6 +26,7 @@ import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.filter.Filter; import java.io.IOException; import java.util.ArrayList; @@ -288,22 +289,21 @@ class StoreScanner extends NonLazyKeyValueScanner store != null ? store.getComparator() : null; LOOP: while((kv = this.heap.peek()) != null) { - // kv is no longer immutable due to KeyOnlyFilter! use copy for safety - KeyValue copyKv = kv.shallowCopy(); // Check that the heap gives us KVs in an increasing order. if (prevKV != null && comparator != null && comparator.compare(prevKV, kv) > 0) { throw new IOException("Key " + prevKV + " followed by a " + "smaller key " + kv + " in cf " + store); } - prevKV = copyKv; - ScanQueryMatcher.MatchCode qcode = matcher.match(copyKv); + prevKV = kv; + ScanQueryMatcher.MatchCode qcode = matcher.match(kv); switch(qcode) { case INCLUDE: case INCLUDE_AND_SEEK_NEXT_ROW: case INCLUDE_AND_SEEK_NEXT_COL: - results.add(copyKv); + Filter f = matcher.getFilter(); + results.add(f == null ? kv : f.transform(kv)); if (qcode == ScanQueryMatcher.MatchCode.INCLUDE_AND_SEEK_NEXT_ROW) { if (!matcher.moreRowsMayExistAfter(kv)) { diff --git a/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java b/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java index 2500e3708ff..7af4db47a4f 100644 --- a/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java +++ b/src/test/java/org/apache/hadoop/hbase/TestKeyValue.java @@ -353,7 +353,7 @@ public class TestKeyValue extends TestCase { assertKVLess(c, firstOnRowA, lastOnRowA); } - public void testConvertToKeyOnly() throws Exception { + public void testCreateKeyOnly() throws Exception { long ts = 1; byte [] value = Bytes.toBytes("a real value"); byte [] evalue = new byte[0]; // empty value @@ -361,9 +361,7 @@ public class TestKeyValue extends TestCase { for (byte[] val : new byte[][]{value, evalue}) { for (boolean useLen : new boolean[]{false,true}) { KeyValue kv1 = new KeyValue(rowA, family, qualA, ts, val); - KeyValue kv1ko = kv1.clone(); - assertTrue(kv1.equals(kv1ko)); - kv1ko.convertToKeyOnly(useLen); + KeyValue kv1ko = kv1.createKeyOnly(useLen); // keys are still the same assertTrue(kv1.equals(kv1ko)); // but values are not