Skip to content

Commit

Permalink
HBASE-14278 Fix NPE that is showing up since HBASE-14274 went in
Browse files Browse the repository at this point in the history
  • Loading branch information
@elliottneilclark
elliottneilclark committed Sep 17, 2015
1 parent 7b08f4c commit c1ac4bb
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 40 deletions.
18 changes: 15 additions & 3 deletions .../src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionAggregateSourceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.hbase.classification.InterfaceAudience;
import org.apache.hadoop.hbase.metrics.BaseSourceImpl;
import org.apache.hadoop.metrics2.MetricsCollector;
Expand All @@ -35,6 +37,8 @@
public class MetricsRegionAggregateSourceImpl extends BaseSourceImpl
implements MetricsRegionAggregateSource {

private static final Log LOG = LogFactory.getLog(MetricsRegionAggregateSourceImpl.class);

private final MetricsExecutorImpl executor = new MetricsExecutorImpl();

private final Set<MetricsRegionSource> regionSources =
Expand All @@ -54,7 +58,7 @@ public MetricsRegionAggregateSourceImpl(String metricsName,
// Every few mins clean the JMX cache.
executor.getExecutor().scheduleWithFixedDelay(new Runnable() {
public void run() {
JmxCacheBuster.clearJmxCache(true);
JmxCacheBuster.clearJmxCache();
}
}, 5, 5, TimeUnit.MINUTES);
}
Expand All @@ -67,12 +71,20 @@ public void register(MetricsRegionSource source) {

@Override
public void deregister(MetricsRegionSource toRemove) {
regionSources.remove(toRemove);
try {
regionSources.remove(toRemove);
} catch (Exception e) {
// Ignored. If this errors out it means that someone is double
// closing the region source and the region is already nulled out.
LOG.info(
"Error trying to remove " + toRemove + " from " + this.getClass().getSimpleName(),
e);
}
clearCache();
}

private synchronized void clearCache() {
JmxCacheBuster.clearJmxCache(true);
JmxCacheBuster.clearJmxCache();
}

/**
Expand Down
12 changes: 6 additions & 6 deletions ...pat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSourceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,28 +68,28 @@ public MetricsRegionServerSourceImpl(String metricsName,
this.rsWrap = rsWrap;

putHisto = getMetricsRegistry().newHistogram(MUTATE_KEY);
slowPut = getMetricsRegistry().newCounter(SLOW_MUTATE_KEY, SLOW_MUTATE_DESC, 0l);
slowPut = getMetricsRegistry().newCounter(SLOW_MUTATE_KEY, SLOW_MUTATE_DESC, 0L);

deleteHisto = getMetricsRegistry().newHistogram(DELETE_KEY);
slowDelete = getMetricsRegistry().newCounter(SLOW_DELETE_KEY, SLOW_DELETE_DESC, 0l);
slowDelete = getMetricsRegistry().newCounter(SLOW_DELETE_KEY, SLOW_DELETE_DESC, 0L);

getHisto = getMetricsRegistry().newHistogram(GET_KEY);
slowGet = getMetricsRegistry().newCounter(SLOW_GET_KEY, SLOW_GET_DESC, 0l);
slowGet = getMetricsRegistry().newCounter(SLOW_GET_KEY, SLOW_GET_DESC, 0L);

incrementHisto = getMetricsRegistry().newHistogram(INCREMENT_KEY);
slowIncrement = getMetricsRegistry().newCounter(SLOW_INCREMENT_KEY, SLOW_INCREMENT_DESC, 0L);

appendHisto = getMetricsRegistry().newHistogram(APPEND_KEY);
slowAppend = getMetricsRegistry().newCounter(SLOW_APPEND_KEY, SLOW_APPEND_DESC, 0l);
slowAppend = getMetricsRegistry().newCounter(SLOW_APPEND_KEY, SLOW_APPEND_DESC, 0L);

replayHisto = getMetricsRegistry().newHistogram(REPLAY_KEY);
scanNextHisto = getMetricsRegistry().newHistogram(SCAN_NEXT_KEY);

splitTimeHisto = getMetricsRegistry().newHistogram(SPLIT_KEY);
flushTimeHisto = getMetricsRegistry().newHistogram(FLUSH_KEY);

splitRequest = getMetricsRegistry().newCounter(SPLIT_REQUEST_KEY, SPLIT_REQUEST_DESC, 0l);
splitSuccess = getMetricsRegistry().newCounter(SPLIT_SUCCESS_KEY, SPLIT_SUCCESS_DESC, 0l);
splitRequest = getMetricsRegistry().newCounter(SPLIT_REQUEST_KEY, SPLIT_REQUEST_DESC, 0L);
splitSuccess = getMetricsRegistry().newCounter(SPLIT_SUCCESS_KEY, SPLIT_SUCCESS_DESC, 0L);
}

@Override
Expand Down
50 changes: 31 additions & 19 deletions ...p2-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionSourceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,12 @@

import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReadWriteLock;
import java.util.concurrent.locks.ReentrantReadWriteLock;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.commons.math.stat.descriptive.DescriptiveStatistics;
import org.apache.hadoop.hbase.classification.InterfaceAudience;
import org.apache.hadoop.metrics2.MetricsRecordBuilder;
import org.apache.hadoop.metrics2.impl.JmxCacheBuster;
import org.apache.hadoop.metrics2.lib.DynamicMetricsRegistry;
import org.apache.hadoop.metrics2.lib.Interns;
import org.apache.hadoop.metrics2.lib.MutableCounterLong;
Expand Down Expand Up @@ -65,6 +61,7 @@ public class MetricsRegionSourceImpl implements MetricsRegionSource {
private final MutableCounterLong regionAppend;
private final MutableHistogram regionGet;
private final MutableHistogram regionScanNext;
private final int hashCode;

public MetricsRegionSourceImpl(MetricsRegionWrapper regionWrapper,
MetricsRegionAggregateSourceImpl aggregate) {
Expand Down Expand Up @@ -101,6 +98,8 @@ public MetricsRegionSourceImpl(MetricsRegionWrapper regionWrapper,

regionScanNextKey = regionNamePrefix + MetricsRegionServerSource.SCAN_NEXT_KEY;
regionScanNext = registry.newHistogram(regionScanNextKey);

hashCode = regionWrapper.getRegionHashCode();
}

@Override
Expand Down Expand Up @@ -173,12 +172,16 @@ public MetricsRegionAggregateSource getAggregateSource() {

@Override
public int compareTo(MetricsRegionSource source) {
if (!(source instanceof MetricsRegionSourceImpl))
if (!(source instanceof MetricsRegionSourceImpl)) {
return -1;
}

MetricsRegionSourceImpl impl = (MetricsRegionSourceImpl) source;
return this.regionWrapper.getRegionName()
.compareTo(impl.regionWrapper.getRegionName());
if (impl == null) {
return -1;
}

return Long.compare(hashCode, impl.hashCode);
}

void snapshot(MetricsRecordBuilder mrb, boolean ignored) {
Expand All @@ -203,31 +206,40 @@ void snapshot(MetricsRecordBuilder mrb, boolean ignored) {
}

mrb.addGauge(
Interns.info(regionNamePrefix + MetricsRegionServerSource.STORE_COUNT,
Interns.info(
regionNamePrefix + MetricsRegionServerSource.STORE_COUNT,
MetricsRegionServerSource.STORE_COUNT_DESC),
this.regionWrapper.getNumStores());
mrb.addGauge(Interns.info(regionNamePrefix + MetricsRegionServerSource.STOREFILE_COUNT,
mrb.addGauge(Interns.info(
regionNamePrefix + MetricsRegionServerSource.STOREFILE_COUNT,
MetricsRegionServerSource.STOREFILE_COUNT_DESC),
this.regionWrapper.getNumStoreFiles());
mrb.addGauge(Interns.info(regionNamePrefix + MetricsRegionServerSource.MEMSTORE_SIZE,
mrb.addGauge(Interns.info(
regionNamePrefix + MetricsRegionServerSource.MEMSTORE_SIZE,
MetricsRegionServerSource.MEMSTORE_SIZE_DESC),
this.regionWrapper.getMemstoreSize());
mrb.addGauge(Interns.info(regionNamePrefix + MetricsRegionServerSource.STOREFILE_SIZE,
mrb.addGauge(Interns.info(
regionNamePrefix + MetricsRegionServerSource.STOREFILE_SIZE,
MetricsRegionServerSource.STOREFILE_SIZE_DESC),
this.regionWrapper.getStoreFileSize());
mrb.addCounter(Interns.info(regionNamePrefix + MetricsRegionSource.COMPACTIONS_COMPLETED_COUNT,
mrb.addCounter(Interns.info(
regionNamePrefix + MetricsRegionSource.COMPACTIONS_COMPLETED_COUNT,
MetricsRegionSource.COMPACTIONS_COMPLETED_DESC),
this.regionWrapper.getNumCompactionsCompleted());
mrb.addCounter(Interns.info(regionNamePrefix + MetricsRegionSource.NUM_BYTES_COMPACTED_COUNT,
mrb.addCounter(Interns.info(
regionNamePrefix + MetricsRegionSource.NUM_BYTES_COMPACTED_COUNT,
MetricsRegionSource.NUM_BYTES_COMPACTED_DESC),
this.regionWrapper.getNumBytesCompacted());
mrb.addCounter(Interns.info(regionNamePrefix + MetricsRegionSource.NUM_FILES_COMPACTED_COUNT,
mrb.addCounter(Interns.info(
regionNamePrefix + MetricsRegionSource.NUM_FILES_COMPACTED_COUNT,
MetricsRegionSource.NUM_FILES_COMPACTED_DESC),
this.regionWrapper.getNumFilesCompacted());
mrb.addCounter(Interns.info(regionNamePrefix + MetricsRegionServerSource.READ_REQUEST_COUNT,
mrb.addCounter(Interns.info(
regionNamePrefix + MetricsRegionServerSource.READ_REQUEST_COUNT,
MetricsRegionServerSource.READ_REQUEST_COUNT_DESC),
this.regionWrapper.getReadRequestCount());
mrb.addCounter(Interns.info(regionNamePrefix + MetricsRegionServerSource.WRITE_REQUEST_COUNT,
mrb.addCounter(Interns.info(
regionNamePrefix + MetricsRegionServerSource.WRITE_REQUEST_COUNT,
MetricsRegionServerSource.WRITE_REQUEST_COUNT_DESC),
this.regionWrapper.getWriteRequestCount());

Expand Down Expand Up @@ -265,12 +277,12 @@ void snapshot(MetricsRecordBuilder mrb, boolean ignored) {

@Override
public int hashCode() {
return regionWrapper.getRegionHashCode();
return hashCode;
}

@Override
public boolean equals(Object obj) {
if (obj == this) return true;
return obj instanceof MetricsRegionSourceImpl && compareTo((MetricsRegionSourceImpl) obj) == 0;
return obj == this ||
(obj instanceof MetricsRegionSourceImpl && compareTo((MetricsRegionSourceImpl) obj) == 0);
}
}
13 changes: 5 additions & 8 deletions hbase-hadoop2-compat/src/main/java/org/apache/hadoop/metrics2/impl/JmxCacheBuster.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,18 @@ private JmxCacheBuster() {
/**
* For JMX to forget about all previously exported metrics.
*/

public static void clearJmxCache() {
clearJmxCache(false);
}

public static synchronized void clearJmxCache(boolean force) {
//If there are more then 100 ms before the executor will run then everything should be merged.
ScheduledFuture future = fut.get();
if (!force &&
(future == null || (!future.isDone() && future.getDelay(TimeUnit.MILLISECONDS) > 100))) {
if ((future == null || (!future.isDone() && future.getDelay(TimeUnit.MILLISECONDS) > 100))) {
// BAIL OUT
return;
}
future = executor.getExecutor().schedule(new JmxCacheBusterRunnable(), 5, TimeUnit.SECONDS);
fut.set(future);
}

static class JmxCacheBusterRunnable implements Runnable {
final static class JmxCacheBusterRunnable implements Runnable {
@Override
public void run() {
if (LOG.isTraceEnabled()) {
Expand All @@ -78,6 +72,9 @@ public void run() {
try {
if (DefaultMetricsSystem.instance() != null) {
DefaultMetricsSystem.instance().stop();
// Sleep some time so that the rest of the hadoop metrics
// system knows that things are done
Thread.sleep(500);
DefaultMetricsSystem.instance().start();
}
} catch (Exception exception) {
Expand Down
8 changes: 4 additions & 4 deletions ...ompat/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionSourceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ public void testCompareToHashCodeEquals() throws Exception {
assertEquals(one.hashCode(), oneClone.hashCode());
assertNotEquals(one, two);

assertTrue( one.compareTo(two) < 0);
assertTrue( two.compareTo(one) > 0);
assertTrue( one.compareTo(two) != 0);
assertTrue( two.compareTo(one) != 0);
assertTrue( two.compareTo(one) != one.compareTo(two));
assertTrue( two.compareTo(two) == 0);
}


Expand All @@ -59,8 +61,6 @@ static class RegionWrapperStub implements MetricsRegionWrapper {
private String regionName;

public RegionWrapperStub(String regionName) {


this.regionName = regionName;
}

Expand Down

0 comments on commit c1ac4bb

Please sign in to comment.