-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-28811 Use region server configuration for evicting the cache wh… #6197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
wchevreuil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add UT for the changed behaviour? Something similar to TestSplitWithCache, but start with a given value for evictOnClose, change it for RSes only, trigger a region close, check the cache:
- Set evictOnClose to false:
UTIL.getConfiguration().setBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, false); - Start cluster:
UTIL.startMiniCluster(1); - Change the config for the RS only:
UTIL.getMiniHBaseCluster().getRegionServer(0).getConfiguration().setBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, true); - Disable table:
UTIL.getAdmin().disableTable(); - Check blocks got evicted:
| private void closeRegionAfterUpdatingMeta(MasterProcedureEnv env, RegionStateNode regionNode) { | ||
| CloseRegionProcedure closeProc = | ||
| LOG.debug("Close region: isSplit: {}: evictOnClose: {}", isSplit, | ||
| isSplit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: this log is misleading, we should just pass the evictCache valure for evictOnClose.
...ver/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
Show resolved
Hide resolved
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Added the unit test TestUseRSCacheConfigWhileClosingRegion.java |
wchevreuil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The UT seems correct, but let's refrain from duplicating code whenever possible. Please move these new tests to TestSplitWithCache, and reuse as much of the code from there.
Refactored the unit test and renamed TestSplitWithCache to TestCacheEviction. The new test can be used to test other eviction scenarios in the future. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…ile unassigning a region
87759d0 to
8955e45
Compare
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…ile unassigning a region (#6197) Signed-off-by: Wellington Chevreuil <[email protected]>
…ile unassigning a region (apache#6197) Signed-off-by: Wellington Chevreuil <[email protected]> (cherry picked from commit 888e4dd)
…ile unassigning a region (#6197) (#6217) Signed-off-by: Wellington Chevreuil <[email protected]> (cherry picked from commit 888e4dd)
…evicting the cache while unassigning a region (apache#6197) Signed-off-by: Wellington Chevreuil <[email protected]> (cherry picked from commit 888e4dd)
…evicting the cache while unassigning a region (apache#6197) Signed-off-by: Wellington Chevreuil <[email protected]> (cherry picked from commit 888e4dd)
…ile unassigning a region