Details
-
Bug
-
Resolution: Done
-
Major
-
5.5.0, 5.5.1, 5.5.2, 5.5.3, 6.0.0, 6.0.1
-
Untriaged
-
Unknown
Description
Spotted whilst looking into other perf regressions in the DcpProducer. The encodeItemHotness method is called for every DCP mutation we send. This is an issue as the current implementation reads an engine configuration value on every call. Engine configuration values are guarded behind a mutex which we have to acquire to read/write. This is rather costly, particularly in the case where we have multiple DcpProducers (i.e. multiple nodes). As the current write heavy daily test (the one that I've been looking most closely at) only has 2 nodes, this has definitely slipped under the radar in perf profiles.
uint8_t DcpProducer::encodeItemHotness(const Item& item) const {
|
if (engine_.getConfiguration().getHtEvictionPolicy() == "hifi_mfu") {
|
auto freqCount = item.getFreqCounterValue();
|
if (supportsHifiMFU) {
|
// The consumer supports the hifi_mfu eviction
|
// policy, therefore use the frequency counter.
|
return freqCount;
|
}
|
// The consumer does not support the hifi_mfu
|
// eviction policy, therefore map from the 8-bit
|
// probabilistic counter (256 states) to NRU (4 states).
|
return ItemEviction::convertFreqCountToNRUValue(freqCount);
|
}
|
/* We are using the 2-bit_lru and therefore get the value */
|
return item.getNRUValue();
|
}
|
Had a quick chat with Daniel Owen, who said it should be okay if functionality is reduced to allowing high mfu config to change at restart/recreation of a DcpProducer. There is a mechanism currently used in the DcpConsumer to allow some engine config values to change at runtime so will have a quick look at employing that here and will do so if it is relatively easy.
Attachments
For Gerrit Dashboard: MB-33151 | ||||||
---|---|---|---|---|---|---|
# | Subject | Branch | Project | Status | CR | V |
105337,4 | MB-33151: Rename supportsHifiMfu | master | kv_engine | Status: MERGED | +2 | +1 |
105338,9 | MB-33151: Remove hifi mfu config param | master | kv_engine | Status: MERGED | +2 | +1 |