Author: liyin Date: Wed Apr 2 20:49:00 2014 New Revision: 1584159 URL: http://svn.apache.org/r1584159 Log: [HBASE-7099][89-fb] Fixed compaction hook metrics and detected a problem
Author: adela Summary: CompactionHook.transform(..) is assuming the client doesn't change the passed RestrictedKeyValue and because of that metric for transformed keyvalues didn't work. Fixed that and ran on one machine on tsh58: https://fburl.com/16574871 Test Plan: ran on tsh58 and it works: https://fburl.com/16574871 Reviewers: aaiyer, rshroff, liyintang Reviewed By: liyintang CC: hbase-eng@ Differential Revision: https://phabricator.fb.com/D1219342 Task ID: 2095744 Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RestrictedKeyValue.java hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/compactionhook/LowerToUpperCompactionHook.java Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RestrictedKeyValue.java URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RestrictedKeyValue.java?rev=1584159&r1=1584158&r2=1584159&view=diff ============================================================================== --- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RestrictedKeyValue.java (original) +++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/RestrictedKeyValue.java Wed Apr 2 20:49:00 2014 @@ -133,6 +133,28 @@ public class RestrictedKeyValue { return true; } + /** + * Compares RestrictedKeyValue with the passed {@link KeyValue} + * @param kv + * @return + */ + public boolean equals(KeyValue kv) { + if (keyValue == null) { + return kv == null; + } else if (kv == null) { + return false; + } else if (!keyValue.equals(kv)) { + //this compares just the key part + return false; + } else if (Bytes.BYTES_RAWCOMPARATOR.compare(keyValue.getBuffer(), + keyValue.getValueOffset(), keyValue.getValueLength(), + kv.getBuffer(), kv.getValueOffset(), + kv.getValueLength()) != 0) { + return false; + } + return true; + } + @Override public String toString() { return "RestrictedKeyValue [keyValue=" + keyValue + "/value=" + Bytes.toString(keyValue.getValue()) + "]"; Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java?rev=1584159&r1=1584158&r2=1584159&view=diff ============================================================================== --- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java (original) +++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java Wed Apr 2 20:49:00 2014 @@ -1427,28 +1427,20 @@ public class Store extends SchemaConfigu } if (compactHook != null && kv.isPut()) { try { - RestrictedKeyValue restrictedKv = new RestrictedKeyValue(kv); RestrictedKeyValue modifiedKv = compactHook - .transform(restrictedKv); + .transform(new RestrictedKeyValue(kv)); if (modifiedKv != null) { writer.append(modifiedKv.getKeyValue(), kvContext); bytesSaved += modifiedKv.differenceInBytes(kv); + if (!modifiedKv.equals(kv)) { + kvsConverted++; + } } else { if (kv != null) { - // TODO: adela check if we are too spamy with this logging - LOG.info("Skipping keyvalue during compaction, due to compaction hook decision: " - + kv); bytesSaved += kv.getLength(); + kvsConverted++; } } - if (!restrictedKv.equals(modifiedKv)) { - kvsConverted++; - } else { - // TODO: adela check if we are too spamy with this logging - LOG.info("Keyvalue is not modified by compaction hook!" - + " modified: " + modifiedKv + "original: " - + restrictedKv); - } } catch (Exception e) { // if exception happened just write unmodified keyvalue writer.append(kv, kvContext); Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/compactionhook/LowerToUpperCompactionHook.java URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/compactionhook/LowerToUpperCompactionHook.java?rev=1584159&r1=1584158&r2=1584159&view=diff ============================================================================== --- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/compactionhook/LowerToUpperCompactionHook.java (original) +++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/compactionhook/LowerToUpperCompactionHook.java Wed Apr 2 20:49:00 2014 @@ -30,9 +30,8 @@ import org.apache.hadoop.hbase.util.Byte public class LowerToUpperCompactionHook implements CompactionHook { @Override - public RestrictedKeyValue transform (RestrictedKeyValue kv) { - RestrictedKeyValue kvModified = new RestrictedKeyValue(kv); - String currentValue = Bytes.toString(kv.getValue()); + public RestrictedKeyValue transform (RestrictedKeyValue kvModified) { + String currentValue = Bytes.toString(kvModified.getValue()); if (currentValue.equals("abc")) { return null; }
