Skip to content

Commit 888e4dd

Browse files
ragarkarragarkar
authored
HBASE-28811 Use region server configuration for evicting the cache while unassigning a region (apache#6197)
Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
1 parent b161ad5 commit 888e4dd

4 files changed

Lines changed: 88 additions & 39 deletions

File tree

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/CloseRegionProcedure.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,17 @@ public class CloseRegionProcedure extends RegionRemoteProcedureBase {
4545
// wrong(but do not make it wrong intentionally). The client can handle this error.
4646
private ServerName assignCandidate;
4747

48-
private boolean evictCache;
48+
private boolean isSplit;
4949

5050
public CloseRegionProcedure() {
5151
super();
5252
}
5353

5454
public CloseRegionProcedure(TransitRegionStateProcedure parent, RegionInfo region,
55-
ServerName targetServer, ServerName assignCandidate, boolean evictCache) {
55+
ServerName targetServer, ServerName assignCandidate, boolean isSplit) {
5656
super(parent, region, targetServer);
5757
this.assignCandidate = assignCandidate;
58-
this.evictCache = evictCache;
58+
this.isSplit = isSplit;
5959
}
6060

6161
@Override
@@ -65,7 +65,7 @@ public TableOperationType getTableOperationType() {
6565

6666
@Override
6767
public RemoteOperation newRemoteOperation(MasterProcedureEnv env) {
68-
return new RegionCloseOperation(this, region, getProcId(), assignCandidate, evictCache,
68+
return new RegionCloseOperation(this, region, getProcId(), assignCandidate, isSplit,
6969
env.getMasterServices().getMasterActiveTime());
7070
}
7171

@@ -76,7 +76,7 @@ protected void serializeStateData(ProcedureStateSerializer serializer) throws IO
7676
if (assignCandidate != null) {
7777
builder.setAssignCandidate(ProtobufUtil.toServerName(assignCandidate));
7878
}
79-
builder.setEvictCache(evictCache);
79+
builder.setEvictCache(isSplit);
8080
serializer.serialize(builder.build());
8181
}
8282

@@ -88,7 +88,7 @@ protected void deserializeStateData(ProcedureStateSerializer serializer) throws
8888
if (data.hasAssignCandidate()) {
8989
assignCandidate = ProtobufUtil.toServerName(data.getAssignCandidate());
9090
}
91-
evictCache = data.getEvictCache();
91+
isSplit = data.getEvictCache();
9292
}
9393

9494
@Override

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -372,14 +372,14 @@ private Flow confirmOpened(MasterProcedureEnv env, RegionStateNode regionNode)
372372
}
373373

374374
private void closeRegionAfterUpdatingMeta(MasterProcedureEnv env, RegionStateNode regionNode) {
375-
CloseRegionProcedure closeProc =
376-
isSplit
377-
? new CloseRegionProcedure(this, getRegion(), regionNode.getRegionLocation(),
378-
assignCandidate,
379-
env.getMasterConfiguration().getBoolean(EVICT_BLOCKS_ON_SPLIT_KEY,
380-
DEFAULT_EVICT_ON_SPLIT))
381-
: new CloseRegionProcedure(this, getRegion(), regionNode.getRegionLocation(),
382-
assignCandidate, evictCache);
375+
LOG.debug("Close region: isSplit: {}: evictOnSplit: {}: evictOnClose: {}", isSplit,
376+
env.getMasterConfiguration().getBoolean(EVICT_BLOCKS_ON_SPLIT_KEY, DEFAULT_EVICT_ON_SPLIT),
377+
evictCache);
378+
// Splits/Merges are special cases, rather than deciding on the cache eviction behaviour here at
379+
// Master, we just need to tell this close is for a split/merge and let RSes decide on the
380+
// eviction. See HBASE-28811 for more context.
381+
CloseRegionProcedure closeProc = new CloseRegionProcedure(this, getRegion(),
382+
regionNode.getRegionLocation(), assignCandidate, isSplit);
383383
addChildProcedure(closeProc);
384384
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_CONFIRM_CLOSED);
385385
}

‎hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/UnassignRegionHandler.java‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
*/
1818
package org.apache.hadoop.hbase.regionserver.handler;
1919

20+
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.DEFAULT_EVICT_ON_CLOSE;
21+
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.DEFAULT_EVICT_ON_SPLIT;
22+
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.EVICT_BLOCKS_ON_CLOSE_KEY;
23+
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.EVICT_BLOCKS_ON_SPLIT_KEY;
24+
2025
import edu.umd.cs.findbugs.annotations.Nullable;
2126
import java.io.IOException;
2227
import java.util.concurrent.TimeUnit;
@@ -61,21 +66,21 @@ public class UnassignRegionHandler extends EventHandler {
6166

6267
private final RetryCounter retryCounter;
6368

64-
private boolean evictCache;
69+
private boolean isSplit;
6570

6671
// active time of the master that sent this unassign request, used for fencing
6772
private final long initiatingMasterActiveTime;
6873

6974
public UnassignRegionHandler(HRegionServer server, String encodedName, long closeProcId,
7075
boolean abort, @Nullable ServerName destination, EventType eventType,
71-
long initiatingMasterActiveTime, boolean evictCache) {
76+
long initiatingMasterActiveTime, boolean isSplit) {
7277
super(server, eventType);
7378
this.encodedName = encodedName;
7479
this.closeProcId = closeProcId;
7580
this.abort = abort;
7681
this.destination = destination;
7782
this.retryCounter = HandlerUtil.getRetryCounter();
78-
this.evictCache = evictCache;
83+
this.isSplit = isSplit;
7984
this.initiatingMasterActiveTime = initiatingMasterActiveTime;
8085
}
8186

@@ -125,7 +130,11 @@ public void process() throws IOException {
125130
}
126131
// This should be true only in the case of splits/merges closing the parent regions, as
127132
// there's no point on keep blocks for those region files.
128-
region.getStores().forEach(s -> s.getCacheConfig().setEvictOnClose(evictCache));
133+
final boolean evictCacheOnClose = isSplit
134+
? server.getConfiguration().getBoolean(EVICT_BLOCKS_ON_SPLIT_KEY, DEFAULT_EVICT_ON_SPLIT)
135+
: server.getConfiguration().getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE);
136+
LOG.debug("Unassign region: split region: {}: evictCache: {}", isSplit, evictCacheOnClose);
137+
region.getStores().forEach(s -> s.getCacheConfig().setEvictOnClose(evictCacheOnClose));
129138

130139
if (region.close(abort) == null) {
131140
// XXX: Is this still possible? The old comment says about split, but now split is done at

hbase-server/src/test/java/org/apache/hadoop/hbase/TestSplitWithCache.java renamed to hbase-server/src/test/java/org/apache/hadoop/hbase/TestCacheEviction.java

Lines changed: 61 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020
import static org.apache.hadoop.hbase.HConstants.BUCKET_CACHE_IOENGINE_KEY;
2121
import static org.apache.hadoop.hbase.HConstants.BUCKET_CACHE_SIZE_KEY;
2222
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY;
23+
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.EVICT_BLOCKS_ON_CLOSE_KEY;
2324
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.EVICT_BLOCKS_ON_SPLIT_KEY;
2425
import static org.apache.hadoop.hbase.io.hfile.CacheConfig.PREFETCH_BLOCKS_ON_OPEN_KEY;
2526
import static org.junit.Assert.assertTrue;
2627

28+
import java.io.IOException;
2729
import java.util.ArrayList;
2830
import java.util.Collection;
2931
import java.util.List;
@@ -47,13 +49,13 @@
4749
import org.slf4j.LoggerFactory;
4850

4951
@Category({ MiscTests.class, MediumTests.class })
50-
public class TestSplitWithCache {
52+
public class TestCacheEviction {
5153

5254
@ClassRule
5355
public static final HBaseClassTestRule CLASS_RULE =
54-
HBaseClassTestRule.forClass(TestSplitWithCache.class);
56+
HBaseClassTestRule.forClass(TestCacheEviction.class);
5557

56-
private static final Logger LOG = LoggerFactory.getLogger(TestSplitWithCache.class);
58+
private static final Logger LOG = LoggerFactory.getLogger(TestCacheEviction.class);
5759

5860
private static final HBaseTestingUtil UTIL = new HBaseTestingUtil();
5961

@@ -69,42 +71,44 @@ public static void setUp() throws Exception {
6971

7072
@Test
7173
public void testEvictOnSplit() throws Exception {
72-
doTest("testEvictOnSplit", true,
74+
doTestEvictOnSplit("testEvictOnSplit", true,
7375
(f, m) -> Waiter.waitFor(UTIL.getConfiguration(), 1000, () -> m.get(f) != null),
7476
(f, m) -> Waiter.waitFor(UTIL.getConfiguration(), 1000, () -> m.get(f) == null));
7577
}
7678

7779
@Test
7880
public void testDoesntEvictOnSplit() throws Exception {
79-
doTest("testDoesntEvictOnSplit", false,
81+
doTestEvictOnSplit("testDoesntEvictOnSplit", false,
8082
(f, m) -> Waiter.waitFor(UTIL.getConfiguration(), 1000, () -> m.get(f) != null),
8183
(f, m) -> Waiter.waitFor(UTIL.getConfiguration(), 1000, () -> m.get(f) != null));
8284
}
8385

84-
private void doTest(String table, boolean evictOnSplit,
86+
@Test
87+
public void testEvictOnClose() throws Exception {
88+
doTestEvictOnClose("testEvictOnClose", true,
89+
(f, m) -> Waiter.waitFor(UTIL.getConfiguration(), 1000, () -> m.get(f) != null),
90+
(f, m) -> Waiter.waitFor(UTIL.getConfiguration(), 1000, () -> m.get(f) == null));
91+
}
92+
93+
@Test
94+
public void testDoesntEvictOnClose() throws Exception {
95+
doTestEvictOnClose("testDoesntEvictOnClose", false,
96+
(f, m) -> Waiter.waitFor(UTIL.getConfiguration(), 1000, () -> m.get(f) != null),
97+
(f, m) -> Waiter.waitFor(UTIL.getConfiguration(), 1000, () -> m.get(f) != null));
98+
}
99+
100+
private void doTestEvictOnSplit(String table, boolean evictOnSplit,
85101
BiConsumer<String, Map<String, Pair<String, Long>>> predicateBeforeSplit,
86102
BiConsumer<String, Map<String, Pair<String, Long>>> predicateAfterSplit) throws Exception {
87-
UTIL.getConfiguration().setBoolean(EVICT_BLOCKS_ON_SPLIT_KEY, evictOnSplit);
88103
UTIL.startMiniCluster(1);
89104
try {
90105
TableName tableName = TableName.valueOf(table);
91-
byte[] family = Bytes.toBytes("CF");
92-
TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName)
93-
.setColumnFamily(ColumnFamilyDescriptorBuilder.of(family)).build();
94-
UTIL.getAdmin().createTable(td);
95-
UTIL.waitTableAvailable(tableName);
96-
Table tbl = UTIL.getConnection().getTable(tableName);
97-
List<Put> puts = new ArrayList<>();
98-
for (int i = 0; i < 1000; i++) {
99-
Put p = new Put(Bytes.toBytes("row-" + i));
100-
p.addColumn(family, Bytes.toBytes(1), Bytes.toBytes("val-" + i));
101-
puts.add(p);
102-
}
103-
tbl.put(puts);
104-
UTIL.getAdmin().flush(tableName);
106+
createAndCacheTable(tableName);
105107
Collection<HStoreFile> files =
106108
UTIL.getMiniHBaseCluster().getRegions(tableName).get(0).getStores().get(0).getStorefiles();
107109
checkCacheForBlocks(tableName, files, predicateBeforeSplit);
110+
UTIL.getMiniHBaseCluster().getRegionServer(0).getConfiguration()
111+
.setBoolean(EVICT_BLOCKS_ON_SPLIT_KEY, evictOnSplit);
108112
UTIL.getAdmin().split(tableName, Bytes.toBytes("row-500"));
109113
Waiter.waitFor(UTIL.getConfiguration(), 30000,
110114
() -> UTIL.getMiniHBaseCluster().getRegions(tableName).size() == 2);
@@ -113,7 +117,43 @@ private void doTest(String table, boolean evictOnSplit,
113117
} finally {
114118
UTIL.shutdownMiniCluster();
115119
}
120+
}
116121

122+
private void doTestEvictOnClose(String table, boolean evictOnClose,
123+
BiConsumer<String, Map<String, Pair<String, Long>>> predicateBeforeClose,
124+
BiConsumer<String, Map<String, Pair<String, Long>>> predicateAfterClose) throws Exception {
125+
UTIL.startMiniCluster(1);
126+
try {
127+
TableName tableName = TableName.valueOf(table);
128+
createAndCacheTable(tableName);
129+
Collection<HStoreFile> files =
130+
UTIL.getMiniHBaseCluster().getRegions(tableName).get(0).getStores().get(0).getStorefiles();
131+
checkCacheForBlocks(tableName, files, predicateBeforeClose);
132+
UTIL.getMiniHBaseCluster().getRegionServer(0).getConfiguration()
133+
.setBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, evictOnClose);
134+
UTIL.getAdmin().disableTable(tableName);
135+
UTIL.waitUntilNoRegionsInTransition();
136+
checkCacheForBlocks(tableName, files, predicateAfterClose);
137+
} finally {
138+
UTIL.shutdownMiniCluster();
139+
}
140+
}
141+
142+
private void createAndCacheTable(TableName tableName) throws IOException, InterruptedException {
143+
byte[] family = Bytes.toBytes("CF");
144+
TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName)
145+
.setColumnFamily(ColumnFamilyDescriptorBuilder.of(family)).build();
146+
UTIL.getAdmin().createTable(td);
147+
UTIL.waitTableAvailable(tableName);
148+
Table tbl = UTIL.getConnection().getTable(tableName);
149+
List<Put> puts = new ArrayList<>();
150+
for (int i = 0; i < 1000; i++) {
151+
Put p = new Put(Bytes.toBytes("row-" + i));
152+
p.addColumn(family, Bytes.toBytes(1), Bytes.toBytes("val-" + i));
153+
puts.add(p);
154+
}
155+
tbl.put(puts);
156+
UTIL.getAdmin().flush(tableName);
117157
}
118158

119159
private void checkCacheForBlocks(TableName tableName, Collection<HStoreFile> files,

0 commit comments

Comments
 (0)