petersomogyi commented on code in PR #8128:
URL: https://github.com/apache/hbase/pull/8128#discussion_r3148111622
##########
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionWrapper.java:
##########
@@ -80,6 +80,11 @@ public interface MetricsRegionWrapper {
*/
float getCurrentRegionCacheRatio();
+ /**
+ * Gets the current cold date % ratio for this region.
Review Comment:
```suggestion
* Gets the current cold data % ratio for this region.
```
##########
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServerSource.java:
##########
@@ -654,6 +654,11 @@ public interface MetricsRegionServerSource extends
BaseSource, JvmPauseMonitorSo
String CURRENT_REGION_CACHE_RATIO = "currentRegionCacheRatio";
String CURRENT_REGION_CACHE_RATIO_DESC = "The percentage of caching
completed for this region.";
+ String CURRENT_REGION_COLD_DATA_RATIO = "currentRegionColdDataRatio";
+
+ String CURRENT_REGION_COLD_DATA_RATIO_DESC = "The percentage of data in this
region that "
+ + "is marked as cold by the configured time based priority logic. ";
Review Comment:
```suggestion
+ "is marked as cold by the configured time based priority logic.";
```
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java:
##########
@@ -2490,7 +2490,17 @@ public Optional<Boolean> shouldCacheFile(HFileInfo
hFileInfo, Configuration conf
String fileName = hFileInfo.getHFileContext().getHFileName();
DataTieringManager dataTieringManager = DataTieringManager.getInstance();
if (dataTieringManager != null && !dataTieringManager.isHotData(hFileInfo,
conf)) {
- LOG.debug("Data tiering is enabled for file: '{}' and it is not hot
data", fileName);
+ LOG.debug("Custom tiering is enabled for file: '{}' and it is not hot
data", fileName);
+ // If custom tiering has been just enabled for a file that was cached,
we now need
+ // to evict it.
+ Set<BlockCacheKey> keySet =
+ getAllCacheKeysForFile(hFileInfo.getHFileContext().getHFileName(), 0,
Long.MAX_VALUE);
+ int evictedBlocks = evictBlockSet(keySet);
Review Comment:
The method name `shouldCacheFile` suggests this just a check but it actually
evicts the blocks.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DataTieringManager.java:
##########
@@ -347,4 +387,29 @@ private static boolean
isDataTieringFeatureEnabled(Configuration conf) {
public static void resetForTestingOnly() {
instance = null;
}
+
+ public Map<String, Pair<List<String>, Long>> getRegionColdDataSize() {
+ return regionColdDataSize;
+ }
+
+ /**
+ * Updates regionColdData size for the region containing the passed
compactedFiles.
+ */
+ public void updateRegionColdDataSize(String encodedRegionName,
+ Collection<HStoreFile> compactedFiles, Collection<HStoreFile> newFiles) {
+ regionColdDataSize.computeIfPresent(encodedRegionName, (k, v) -> {
+ for (HStoreFile file : compactedFiles) {
+ if (v.getFirst().contains(file.getPath().getName())) {
+ v.getFirst().remove(file.getPath().getName());
+ v.setSecond(v.getSecond() -
Bytes.toLong(file.getMetadataValue(FILE_SIZE)));
+ }
+ }
+ for (HStoreFile file : newFiles) {
+ // call isHotData to account for the new file size in
regionColdDataSize, if the new file is
+ // considered cold data as per data-tiering logic.
+ isHotData(file.getFileInfo().getHFileInfo(),
file.getFileInfo().getConf());
Review Comment:
Can this cause a deadlock? This part is inside
`regionColdDataSize.computeIfPresent` block and the `isHotData` also runs
`regionColdDataSize.compute` on the same ConcurrentMap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
