Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,8 @@ default String getNameAsString() {
int getStoreRefCount();

/**
* @return the max reference count for any store file among all stores files
* @return the max reference count for any store file among all compacted stores files
* of this region
*/
int getMaxStoreFileRefCount();
int getMaxCompactedStoreFileRefCount();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because RegionMetrics is a public interface, could you please deprecate getMaxStoreFileRefCount and add getMaxCompactedStoreFileRefCount as a new method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okk, yes ideally we would do that but in this case, this change int getMaxStoreFileRefCount(); has not yet landed to any release so far and is available to 2.3.0 and 1.6.0 only, no other release branch. Hence, the plan is to update this method now only before 2.3.0 or 1.6.0 makes it to releases.
Thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok. This seems like a minor gap in our documentation on how we handle deprecations. Would be good to have a common sense and a documentation for it. @saintstack Any thoughts?

Copy link
Contributor Author

@virajjasani virajjasani Dec 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with the suggestion.
This new IA method was introduced as part of Jira HBASE-22460 and HBASE-23213 which are not part of any active release so far. Hence IMO, before 2.3.0 or 1.6.0 goes live, ideally we should be good to make this change, but yes agree to follow standard best practices reg such cases if we have common practice available.
It could also be argued that we can change any Public IA method signatures as long as they are not yet present in any active release.

cc: @apurtell @anoopsjohn

}
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public static RegionMetrics toRegionMetrics(ClusterStatusProtos.RegionLoad regio
.setStoreCount(regionLoadPB.getStores())
.setStoreFileCount(regionLoadPB.getStorefiles())
.setStoreRefCount(regionLoadPB.getStoreRefCount())
.setMaxStoreFileRefCount(regionLoadPB.getMaxStoreFileRefCount())
.setMaxCompactedStoreFileRefCount(regionLoadPB.getMaxCompactedStoreFileRefCount())
.setStoreFileSize(new Size(regionLoadPB.getStorefileSizeMB(), Size.Unit.MEGABYTE))
.setStoreSequenceIds(regionLoadPB.getStoreCompleteSequenceIdList().stream()
.collect(Collectors.toMap(
Expand Down Expand Up @@ -114,7 +114,7 @@ public static ClusterStatusProtos.RegionLoad toRegionLoad(RegionMetrics regionMe
.setStores(regionMetrics.getStoreCount())
.setStorefiles(regionMetrics.getStoreFileCount())
.setStoreRefCount(regionMetrics.getStoreRefCount())
.setMaxStoreFileRefCount(regionMetrics.getMaxStoreFileRefCount())
.setMaxCompactedStoreFileRefCount(regionMetrics.getMaxCompactedStoreFileRefCount())
.setStorefileSizeMB((int) regionMetrics.getStoreFileSize().get(Size.Unit.MEGABYTE))
.addAllStoreCompleteSequenceId(toStoreSequenceId(regionMetrics.getStoreSequenceId()))
.setStoreUncompressedSizeMB(
Expand All @@ -130,7 +130,7 @@ public static RegionMetricsBuilder newBuilder(byte[] name) {
private int storeCount;
private int storeFileCount;
private int storeRefCount;
private int maxStoreFileRefCount;
private int maxCompactedStoreFileRefCount;
private long compactingCellCount;
private long compactedCellCount;
private Size storeFileSize = Size.ZERO;
Expand Down Expand Up @@ -164,8 +164,8 @@ public RegionMetricsBuilder setStoreRefCount(int value) {
this.storeRefCount = value;
return this;
}
public RegionMetricsBuilder setMaxStoreFileRefCount(int value) {
this.maxStoreFileRefCount = value;
public RegionMetricsBuilder setMaxCompactedStoreFileRefCount(int value) {
this.maxCompactedStoreFileRefCount = value;
return this;
}
public RegionMetricsBuilder setCompactingCellCount(long value) {
Expand Down Expand Up @@ -242,7 +242,7 @@ public RegionMetrics build() {
storeCount,
storeFileCount,
storeRefCount,
maxStoreFileRefCount,
maxCompactedStoreFileRefCount,
compactingCellCount,
compactedCellCount,
storeFileSize,
Expand All @@ -267,7 +267,7 @@ private static class RegionMetricsImpl implements RegionMetrics {
private final int storeCount;
private final int storeFileCount;
private final int storeRefCount;
private final int maxStoreFileRefCount;
private final int maxCompactedStoreFileRefCount;
private final long compactingCellCount;
private final long compactedCellCount;
private final Size storeFileSize;
Expand All @@ -289,7 +289,7 @@ private static class RegionMetricsImpl implements RegionMetrics {
int storeCount,
int storeFileCount,
int storeRefCount,
int maxStoreFileRefCount,
int maxCompactedStoreFileRefCount,
final long compactingCellCount,
long compactedCellCount,
Size storeFileSize,
Expand All @@ -311,7 +311,7 @@ private static class RegionMetricsImpl implements RegionMetrics {
this.storeCount = storeCount;
this.storeFileCount = storeFileCount;
this.storeRefCount = storeRefCount;
this.maxStoreFileRefCount = maxStoreFileRefCount;
this.maxCompactedStoreFileRefCount = maxCompactedStoreFileRefCount;
this.compactingCellCount = compactingCellCount;
this.compactedCellCount = compactedCellCount;
this.storeFileSize = Preconditions.checkNotNull(storeFileSize);
Expand Down Expand Up @@ -352,8 +352,8 @@ public int getStoreRefCount() {
}

@Override
public int getMaxStoreFileRefCount() {
return maxStoreFileRefCount;
public int getMaxCompactedStoreFileRefCount() {
return maxCompactedStoreFileRefCount;
}

@Override
Expand Down Expand Up @@ -449,8 +449,8 @@ public String toString() {
this.getStoreFileCount());
Strings.appendKeyValue(sb, "storeRefCount",
this.getStoreRefCount());
Strings.appendKeyValue(sb, "maxStoreFileRefCount",
this.getMaxStoreFileRefCount());
Strings.appendKeyValue(sb, "maxCompactedStoreFileRefCount",
this.getMaxCompactedStoreFileRefCount());
Strings.appendKeyValue(sb, "uncompressedStoreFileSize",
this.getUncompressedStoreFileSize());
Strings.appendKeyValue(sb, "lastMajorCompactionTimestamp",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ public String toString() {
int storeCount = 0;
int storeFileCount = 0;
int storeRefCount = 0;
int maxStoreFileRefCount = 0;
int maxCompactedStoreFileRefCount = 0;
long uncompressedStoreFileSizeMB = 0;
long storeFileSizeMB = 0;
long memStoreSizeMB = 0;
Expand All @@ -377,8 +377,9 @@ public String toString() {
storeCount += r.getStoreCount();
storeFileCount += r.getStoreFileCount();
storeRefCount += r.getStoreRefCount();
int currentMaxStoreFileRefCount = r.getMaxStoreFileRefCount();
maxStoreFileRefCount = Math.max(maxStoreFileRefCount, currentMaxStoreFileRefCount);
int currentMaxCompactedStoreFileRefCount = r.getMaxCompactedStoreFileRefCount();
maxCompactedStoreFileRefCount = Math.max(maxCompactedStoreFileRefCount,
currentMaxCompactedStoreFileRefCount);
uncompressedStoreFileSizeMB += r.getUncompressedStoreFileSize().get(Size.Unit.MEGABYTE);
storeFileSizeMB += r.getStoreFileSize().get(Size.Unit.MEGABYTE);
memStoreSizeMB += r.getMemStoreSize().get(Size.Unit.MEGABYTE);
Expand All @@ -401,7 +402,8 @@ public String toString() {
Strings.appendKeyValue(sb, "numberOfStores", storeCount);
Strings.appendKeyValue(sb, "numberOfStorefiles", storeFileCount);
Strings.appendKeyValue(sb, "storeRefCount", storeRefCount);
Strings.appendKeyValue(sb, "maxStoreFileRefCount", maxStoreFileRefCount);
Strings.appendKeyValue(sb, "maxCompactedStoreFileRefCount",
maxCompactedStoreFileRefCount);
Strings.appendKeyValue(sb, "storefileUncompressedSizeMB", uncompressedStoreFileSizeMB);
Strings.appendKeyValue(sb, "storefileSizeMB", storeFileSizeMB);
if (uncompressedStoreFileSizeMB != 0) {
Expand Down
6 changes: 3 additions & 3 deletions hbase-common/src/main/resources/hbase-default.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1915,15 +1915,15 @@ possible configurations would overwhelm and obscure the important.
<name>hbase.regions.recovery.store.file.ref.count</name>
<value>-1</value>
<description>
Very large ref count on a file indicates
Very large ref count on a compacted store file indicates
that it is a ref leak on that object. Such files
can not be removed even after it is invalidated
can not be removed after it is invalidated
via compaction. Only way to recover in such
scenario is to reopen the region which can
release all resources, like the refcount, leases, etc.
This config represents Store files Ref Count threshold
value considered for reopening regions.
Any region with store files ref count > this value
Any region with compacted store files ref count > this value
would be eligible for reopening by master.
Default value -1 indicates this feature is turned off.
Only positive integer value should be provided to enable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ public interface MetricsRegionServerSource extends BaseSource, JvmPauseMonitorSo
String STOREFILE_COUNT_DESC = "Number of Store Files";
String STORE_REF_COUNT = "storeRefCount";
String STORE_REF_COUNT_DESC = "Store reference count";
String MAX_STORE_FILE_REF_COUNT = "maxStoreFileRefCount";
String MAX_COMPACTED_STORE_FILE_REF_COUNT = "maxCompactedStoreFileRefCount";
String MEMSTORE_SIZE = "memStoreSize";
String MEMSTORE_SIZE_DESC = "Size of the memstore";
String STOREFILE_SIZE = "storeFileSize";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ public interface MetricsRegionWrapper {

/**
* @return the max number of references active on any store file among
* all store files that belong to this region
* all compacted store files that belong to this region
*/
long getMaxStoreFileRefCount();
long getMaxCompactedStoreFileRefCount();
}
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,10 @@ void snapshot(MetricsRecordBuilder mrb, boolean ignored) {
MetricsRegionServerSource.STORE_REF_COUNT),
this.regionWrapper.getStoreRefCount());
mrb.addGauge(Interns.info(
regionNamePrefix + MetricsRegionServerSource.MAX_STORE_FILE_REF_COUNT,
MetricsRegionServerSource.MAX_STORE_FILE_REF_COUNT),
this.regionWrapper.getMaxStoreFileRefCount());
regionNamePrefix + MetricsRegionServerSource.MAX_COMPACTED_STORE_FILE_REF_COUNT,
MetricsRegionServerSource.MAX_COMPACTED_STORE_FILE_REF_COUNT),
this.regionWrapper.getMaxCompactedStoreFileRefCount()
);
mrb.addGauge(Interns.info(
regionNamePrefix + MetricsRegionServerSource.MEMSTORE_SIZE,
MetricsRegionServerSource.MEMSTORE_SIZE_DESC),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public long getStoreRefCount() {
}

@Override
public long getMaxStoreFileRefCount() {
public long getMaxCompactedStoreFileRefCount() {
return 0;
}

Expand Down
4 changes: 2 additions & 2 deletions hbase-protocol-shaded/src/main/protobuf/ClusterStatus.proto
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ message RegionLoad {
optional int32 store_ref_count = 21 [default = 0];

/**
* The max number of references active on single store file among all store files
* The max number of references active on single store file among all compacted store files
* that belong to given region
*/
optional int32 max_store_file_ref_count = 22 [default = 0];
optional int32 max_compacted_store_file_ref_count = 22 [default = 0];
}

message UserLoad {
Expand Down
4 changes: 2 additions & 2 deletions hbase-protocol/src/main/protobuf/ClusterStatus.proto
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,10 @@ message RegionLoad {
optional int32 store_ref_count = 21 [default = 0];

/**
* The max number of references active on single store file among all store files
* The max number of references active on single store file among all compacted store files
* that belong to given region
*/
optional int32 max_store_file_ref_count = 22 [default = 0];
optional int32 max_compacted_store_file_ref_count = 22 [default = 0];
}

message UserLoad {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,17 +130,18 @@ private Map<TableName, List<byte[]>> getTableToRegionsByRefCount(
for (ServerMetrics serverMetrics : serverMetricsMap.values()) {
Map<byte[], RegionMetrics> regionMetricsMap = serverMetrics.getRegionMetrics();
for (RegionMetrics regionMetrics : regionMetricsMap.values()) {
// For each region, each store file can have different ref counts
// We need to find maximum of all such ref counts and if that max count
// is beyond a threshold value, we should reopen the region.
// Here, we take max ref count of all store files and not the cumulative
// count of all store files
final int maxStoreFileRefCount = regionMetrics.getMaxStoreFileRefCount();

if (maxStoreFileRefCount > storeFileRefCountThreshold) {
// For each region, each compacted store file can have different ref counts
// We need to find maximum of all such ref counts and if that max count of compacted
// store files is beyond a threshold value, we should reopen the region.
// Here, we take max ref count of all compacted store files and not the cumulative
// count of all compacted store files
final int maxCompactedStoreFileRefCount = regionMetrics
.getMaxCompactedStoreFileRefCount();

if (maxCompactedStoreFileRefCount > storeFileRefCountThreshold) {
final byte[] regionName = regionMetrics.getRegionName();
prepareTableToReopenRegionsMap(tableToReopenRegionsMap, regionName,
maxStoreFileRefCount);
maxCompactedStoreFileRefCount);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1638,7 +1638,7 @@ RegionLoad createRegionLoad(final HRegion r, RegionLoad.Builder regionLoadBldr,
int stores = 0;
int storefiles = 0;
int storeRefCount = 0;
int maxStoreFileRefCount = 0;
int maxCompactedStoreFileRefCount = 0;
int storeUncompressedSizeMB = 0;
int storefileSizeMB = 0;
int memstoreSizeMB = (int) (r.getMemStoreDataSize() / 1024 / 1024);
Expand All @@ -1654,8 +1654,9 @@ RegionLoad createRegionLoad(final HRegion r, RegionLoad.Builder regionLoadBldr,
storefiles += store.getStorefilesCount();
int currentStoreRefCount = store.getStoreRefCount();
storeRefCount += currentStoreRefCount;
int currentMaxStoreFileRefCount = store.getMaxStoreFileRefCount();
maxStoreFileRefCount = Math.max(maxStoreFileRefCount, currentMaxStoreFileRefCount);
int currentMaxCompactedStoreFileRefCount = store.getMaxCompactedStoreFileRefCount();
maxCompactedStoreFileRefCount = Math.max(maxCompactedStoreFileRefCount,
currentMaxCompactedStoreFileRefCount);
storeUncompressedSizeMB += (int) (store.getStoreSizeUncompressed() / 1024 / 1024);
storefileSizeMB += (int) (store.getStorefilesSize() / 1024 / 1024);
//TODO: storefileIndexSizeKB is same with rootLevelIndexSizeKB?
Expand Down Expand Up @@ -1684,7 +1685,7 @@ RegionLoad createRegionLoad(final HRegion r, RegionLoad.Builder regionLoadBldr,
.setStores(stores)
.setStorefiles(storefiles)
.setStoreRefCount(storeRefCount)
.setMaxStoreFileRefCount(maxStoreFileRefCount)
.setMaxCompactedStoreFileRefCount(maxCompactedStoreFileRefCount)
.setStoreUncompressedSizeMB(storeUncompressedSizeMB)
.setStorefileSizeMB(storefileSizeMB)
.setMemStoreSizeMB(memstoreSizeMB)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2806,18 +2806,19 @@ public int getStoreRefCount() {
}

/**
* @return get maximum ref count of storeFile among all HStore Files
* @return get maximum ref count of storeFile among all compacted HStore Files
* for the HStore
*/
public int getMaxStoreFileRefCount() {
OptionalInt maxStoreFileRefCount = this.storeEngine.getStoreFileManager()
.getStorefiles()
public int getMaxCompactedStoreFileRefCount() {
OptionalInt maxCompactedStoreFileRefCount = this.storeEngine.getStoreFileManager()
.getCompactedfiles()
.stream()
.filter(sf -> sf.getReader() != null)
.filter(HStoreFile::isHFile)
.mapToInt(HStoreFile::getRefCount)
.max();
return maxStoreFileRefCount.isPresent() ? maxStoreFileRefCount.getAsInt() : 0;
return maxCompactedStoreFileRefCount.isPresent()
? maxCompactedStoreFileRefCount.getAsInt() : 0;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public class MetricsRegionWrapperImpl implements MetricsRegionWrapper, Closeable
private Runnable runnable;
private long numStoreFiles;
private long storeRefCount;
private long maxStoreFileRefCount;
private long maxCompactedStoreFileRefCount;
private long memstoreSize;
private long storeFileSize;
private long maxStoreFileAge;
Expand Down Expand Up @@ -127,8 +127,8 @@ public long getStoreRefCount() {
}

@Override
public long getMaxStoreFileRefCount() {
return maxStoreFileRefCount;
public long getMaxCompactedStoreFileRefCount() {
return maxCompactedStoreFileRefCount;
}

@Override
Expand Down Expand Up @@ -239,7 +239,7 @@ public class HRegionMetricsWrapperRunnable implements Runnable {
public void run() {
long tempNumStoreFiles = 0;
int tempStoreRefCount = 0;
int tempMaxStoreFileRefCount = 0;
int tempMaxCompactedStoreFileRefCount = 0;
long tempMemstoreSize = 0;
long tempStoreFileSize = 0;
long tempMaxStoreFileAge = 0;
Expand All @@ -254,9 +254,9 @@ public void run() {
tempNumStoreFiles += store.getStorefilesCount();
int currentStoreRefCount = store.getStoreRefCount();
tempStoreRefCount += currentStoreRefCount;
int currentMaxStoreFileRefCount = store.getMaxStoreFileRefCount();
tempMaxStoreFileRefCount = Math.max(tempMaxStoreFileRefCount,
currentMaxStoreFileRefCount);
int currentMaxCompactedStoreFileRefCount = store.getMaxCompactedStoreFileRefCount();
tempMaxCompactedStoreFileRefCount = Math.max(tempMaxCompactedStoreFileRefCount,
currentMaxCompactedStoreFileRefCount);
tempMemstoreSize += store.getMemStoreSize().getDataSize();
tempStoreFileSize += store.getStorefilesSize();
OptionalLong storeMaxStoreFileAge = store.getMaxStoreFileAge();
Expand Down Expand Up @@ -284,7 +284,7 @@ public void run() {

numStoreFiles = tempNumStoreFiles;
storeRefCount = tempStoreRefCount;
maxStoreFileRefCount = tempMaxStoreFileRefCount;
maxCompactedStoreFileRefCount = tempMaxCompactedStoreFileRefCount;
memstoreSize = tempMemstoreSize;
storeFileSize = tempStoreFileSize;
maxStoreFileAge = tempMaxStoreFileAge;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ public void testRegionReopensWithoutStoreRefConfig() throws Exception {
Mockito.verify(hMaster, Mockito.times(0)).reopenRegions(Mockito.any(), Mockito.anyList(),
Mockito.anyLong(), Mockito.anyLong());

// default maxStoreFileRefCount is -1 (no regions to be reopened using AM)
// default maxCompactedStoreFileRefCount is -1 (no regions to be reopened using AM)
Mockito.verify(hMaster, Mockito.times(0)).getAssignmentManager();
Mockito.verify(assignmentManager, Mockito.times(0))
.getRegionInfo(Mockito.any());
Expand Down Expand Up @@ -380,7 +380,7 @@ public long getLastReportTimestamp() {
return serverMetrics;
}

private static RegionMetrics getRegionMetrics(byte[] regionName, int storeRefCount) {
private static RegionMetrics getRegionMetrics(byte[] regionName, int compactedStoreRefCount) {
RegionMetrics regionMetrics = new RegionMetrics() {

@Override
Expand Down Expand Up @@ -485,12 +485,12 @@ public long getLastMajorCompactionTimestamp() {

@Override
public int getStoreRefCount() {
return storeRefCount;
return compactedStoreRefCount;
}

@Override
public int getMaxStoreFileRefCount() {
return storeRefCount;
public int getMaxCompactedStoreFileRefCount() {
return compactedStoreRefCount;
}

};
Expand Down Expand Up @@ -615,4 +615,4 @@ public boolean isStopped() {

}

}
}
Loading