Support store block meta to RocksBlockStore, store location to heap #15238
Conversation
|
very interested in performance comparison before and after this change. Also, is there any HEAP memory estimation about the location info? |
|
@kevincai We had a test through stressmasterbench these days, it shows that this PR can improve the performance obviously.
|
Of course using heap will be faster than rocks. I'd be more interested to see the performance difference compared to the memory usage. After all we are comparing the pros and cons and we need to see the cons. |
@maobaolong Pls see comments above and let's discuss on this PR in the next community meeting. Thx :) |
maobaolong
force-pushed
the
supportRocksBlockStoreMetaOnly
branch
from
5fc4b5a
to
cdbcf8b
Compare
|
@jiacheliu3 Is there any further suggestion about this PR? |
There was a problem hiding this comment.
@maobaolong Thanks for the improvements they make sense to me. Could you please add below stats?
-
block_meta size:
(a) on heap
(b) on disk -
block_location size:
(a) on heap
(b) on disk -
Can you provide some use cases on how many about block_meta and block_location you have in your environment?
| * @param baseDir the base directory in which to store block store metadata | ||
| */ | ||
| public RocksBlockMetaStoreMetaOnly(String baseDir) { | ||
| super(baseDir); |
There was a problem hiding this comment.
RocksBlockMetaStore() creates BlockMeta and BlockLocation tables in RocksDB, and BlockLocation is not needed.
You can probably do
class RocksBlockMetaStore {
RocksBlockMetaStore() {
..
initDb();
}
public initDb() {
// create rocks configs etc
createBlockMetaTable();
createBlockLocationsTable();
}
}
public class RocksBlockMetaStoreMetaOnly extends RocksBlockMetaStore {
RocksBlockMetaStoreMetaOnly() {
super();
// create your TwoKeyHashMap
}
@Override
public initDb() {
// create rocks configs etc
createBlockMetaTable();
// do not create BlockLocation table
}
}
|
I get the BlockLocation jmap as following |
|
|
| @@ -57,7 +59,10 @@ public List<BlockLocation> getLocations(long blockid) { | |||
|
|
|||
| @Override | |||
| public void addLocation(long blockId, BlockLocation location) { | |||
| mBlockLocations.addInnerValue(blockId, location.getWorkerId(), location); | |||
| // NOTICE(maobaolong): mLocationCacheMap can be increase to WOKRER_COUNT X TIER_COUNT | |||
There was a problem hiding this comment.
consider when to remove from this map
There was a problem hiding this comment.
We can remove in LostWorkerDetectionExecutor heartbeat thread, when a worker is removed from mWorkers into mLostWorkers, we remove all worker location objects after processLostWorker()
| @@ -35,6 +36,7 @@ public class RocksBlockMetaStoreMetaOnly extends RocksBlockMetaStore { | |||
| // Map from block id to block locations. | |||
| public final TwoKeyConcurrentMap<Long, Long, BlockLocation, Map<Long, BlockLocation>> | |||
| mBlockLocations = new TwoKeyConcurrentMap<>(() -> new HashMap<>(4)); | |||
| private final Map<BlockLocation, BlockLocation> mLocationCacheMap = new ConcurrentHashMap<>(); | |||
There was a problem hiding this comment.
This is a very useful change! Let's separate it into a different PR. Thanks a lot in advance!
Also I think this applies to all BlockMetaStore. The change should be in
So when we need a BlockLocation, we always go to a cache instead of creating a new one.There was a problem hiding this comment.
In fact we have two caller as above figure.
There was a problem hiding this comment.
@jiacheliu3 Create a PR #16763 and move the cache logic to DefaultBlockMaster
What changes are proposed in this pull request?
The PR #14565 is the dependency PR of this.
From this figure, we can see one of the bottleneck of getFileInfo performance is
getLocation, so I propose to use HEAP to store the location of blocks, but store the block meta into rocks store.Why are the changes needed?
With this PR, we can get a tradeoff between performance and scalability.
Does this PR introduce any user facing changes?
Add a configure key
alluxio.master.block.metastoreto ROCKS_BLOCK_META_ONLY