Skip to content

Commit dba6f9b

Browse files
pbhandar2meta-codesync[bot]
authored andcommitted
use allocator to configure event tracker instead of admin
Summary: Add eventTrackerConfigFactory to CacheAllocatorConfig — a factory function (std::function<EventTracker::Config()>) that creates a fresh EventTracker::Config on demand. EventTracker::Config contains unique_ptr members (EventSink, SamplerInterface) for dependency injection of abstract interfaces. This makes the config move-only, so it can only be consumed once. During warm roll recovery, CacheAllocatorConfig is reused to re-initialize the allocator, which means a directly-stored config would be invalid after the first use. The factory pattern solves this by producing a new EventTracker::Config with fresh unique_ptr instances each time the allocator initializes. This moves event tracker setup from the admin server into the allocator config, keeping configuration self-contained. Reviewed By: rlyerly Differential Revision: D95624875 fbshipit-source-id: 2d78dfc522daa281e24aa32d727e21f5568c3772
1 parent d8153d4 commit dba6f9b

3 files changed

Lines changed: 82 additions & 0 deletions

File tree

cachelib/allocator/CacheAllocator.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2673,6 +2673,9 @@ void CacheAllocator<CacheTrait>::initCommon(bool dramCacheAttached) {
26732673
}
26742674
initStats();
26752675
initNvmCache(dramCacheAttached);
2676+
if (config_.eventTrackerConfigFactory) {
2677+
setEventTracker(config_.eventTrackerConfigFactory());
2678+
}
26762679

26772680
if (!config_.delayCacheWorkersStart) {
26782681
initWorkers();

cachelib/allocator/CacheAllocatorConfig.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "cachelib/allocator/RebalanceStrategy.h"
4242
#include "cachelib/allocator/Util.h"
4343
#include "cachelib/common/EventInterface.h"
44+
#include "cachelib/common/EventTracker.h"
4445
#include "cachelib/common/Throttler.h"
4546

4647
namespace facebook {
@@ -343,6 +344,12 @@ class CacheAllocatorConfig {
343344
// starts
344345
CacheAllocatorConfig& setEventTracker(LegacyEventTrackerSharedPtr&&);
345346

347+
// Set a factory function to create EventTracker::Config on demand.
348+
// Creates a fresh config each time, avoiding issues when
349+
// CacheAllocatorConfig is reused (e.g., during warm roll recovery).
350+
CacheAllocatorConfig& setEventTrackerConfigFactory(
351+
std::function<EventTracker::Config()> factory);
352+
346353
// Set the minimum TTL for an item to be admitted into NVM cache.
347354
// If nvmAdmissionMinTTL is set to be positive, any item with configured TTL
348355
// smaller than this will always be rejected by NvmAdmissionPolicy.
@@ -577,6 +584,11 @@ class CacheAllocatorConfig {
577584
// construction.
578585
LegacyEventTrackerSharedPtr legacyEventTracker{nullptr};
579586

587+
// Factory function to create EventTracker::Config on demand.
588+
// Creates a fresh config each time, avoiding issues when
589+
// CacheAllocatorConfig is reused (e.g., during warm roll recovery).
590+
std::function<EventTracker::Config()> eventTrackerConfigFactory{nullptr};
591+
580592
// whether to allow tracking tail hits in MM2Q
581593
bool trackTailHits{false};
582594

@@ -1164,6 +1176,13 @@ CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::setEventTracker(
11641176
return *this;
11651177
}
11661178

1179+
template <typename T>
1180+
CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::setEventTrackerConfigFactory(
1181+
std::function<EventTracker::Config()> factory) {
1182+
eventTrackerConfigFactory = std::move(factory);
1183+
return *this;
1184+
}
1185+
11671186
template <typename T>
11681187
CacheAllocatorConfig<T>& CacheAllocatorConfig<T>::setNvmAdmissionMinTTL(
11691188
uint64_t ttl) {
@@ -1353,6 +1372,8 @@ std::map<std::string, std::string> CacheAllocatorConfig<T>::serialize() const {
13531372
configMap["defaultPoolRebalanceStrategy"] =
13541373
stringifyRebalanceStrategy(defaultPoolRebalanceStrategy);
13551374
configMap["eventTracker"] = legacyEventTracker ? "set" : "empty";
1375+
configMap["eventTrackerConfigFactory"] =
1376+
eventTrackerConfigFactory ? "set" : "empty";
13561377
configMap["nvmAdmissionMinTTL"] = std::to_string(nvmAdmissionMinTTL);
13571378
configMap["delayCacheWorkersStart"] =
13581379
delayCacheWorkersStart ? "true" : "false";

cachelib/allocator/tests/CacheAllocatorConfigTest.cpp

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
#include "cachelib/allocator/CacheAllocatorConfig.h"
1818
#include "cachelib/allocator/MemoryTierCacheConfig.h"
1919
#include "cachelib/allocator/tests/TestBase.h"
20+
#include "cachelib/common/EventSink.h"
21+
#include "cachelib/common/EventTracker.h"
2022

2123
namespace facebook {
2224
namespace cachelib {
@@ -91,6 +93,62 @@ TEST_F(CacheAllocatorConfigTest, SerializeEvictionPolicyWTinyLFU) {
9193
EXPECT_EQ(serialized["evictionPolicy"], "MMWTinyLFU");
9294
}
9395

96+
TEST_F(CacheAllocatorConfigTest, SetEventTrackerConfigFactory) {
97+
AllocatorT::Config config;
98+
int factoryCallCount = 0;
99+
config.setEventTrackerConfigFactory([&factoryCallCount]() {
100+
++factoryCallCount;
101+
EventTracker::Config trackerConfig;
102+
trackerConfig.queueSize = 100;
103+
trackerConfig.eventSink = std::make_unique<InMemoryEventSink>();
104+
trackerConfig.sampler = std::make_unique<FurcHashSampler>(1);
105+
return trackerConfig;
106+
});
107+
EXPECT_TRUE(config.eventTrackerConfigFactory != nullptr);
108+
EXPECT_EQ(factoryCallCount, 0);
109+
110+
// Invoke the factory and verify it produces a valid config.
111+
auto trackerConfig = config.eventTrackerConfigFactory();
112+
EXPECT_EQ(factoryCallCount, 1);
113+
EXPECT_EQ(trackerConfig.queueSize, 100);
114+
EXPECT_NE(trackerConfig.eventSink, nullptr);
115+
EXPECT_NE(trackerConfig.sampler, nullptr);
116+
}
117+
118+
TEST_F(CacheAllocatorConfigTest, EventTrackerConfigFactoryProducesFreshConfig) {
119+
AllocatorT::Config config;
120+
config.setEventTrackerConfigFactory([]() {
121+
EventTracker::Config trackerConfig;
122+
trackerConfig.queueSize = 50;
123+
trackerConfig.eventSink = std::make_unique<InMemoryEventSink>();
124+
return trackerConfig;
125+
});
126+
127+
// Call factory twice and verify each produces independent, valid configs.
128+
auto config1 = config.eventTrackerConfigFactory();
129+
auto config2 = config.eventTrackerConfigFactory();
130+
EXPECT_NE(config1.eventSink, nullptr);
131+
EXPECT_NE(config2.eventSink, nullptr);
132+
// The two sinks must be distinct objects.
133+
EXPECT_NE(config1.eventSink.get(), config2.eventSink.get());
134+
}
135+
136+
TEST_F(CacheAllocatorConfigTest, SerializeEventTrackerConfigFactory) {
137+
AllocatorT::Config config;
138+
// Before setting, serialization should show "empty".
139+
auto serialized = config.serialize();
140+
EXPECT_EQ(serialized["eventTrackerConfigFactory"], "empty");
141+
142+
config.setEventTrackerConfigFactory([]() {
143+
EventTracker::Config trackerConfig;
144+
trackerConfig.queueSize = 10;
145+
trackerConfig.eventSink = std::make_unique<InMemoryEventSink>();
146+
return trackerConfig;
147+
});
148+
serialized = config.serialize();
149+
EXPECT_EQ(serialized["eventTrackerConfigFactory"], "set");
150+
}
151+
94152
} // namespace tests
95153
} // namespace cachelib
96154
} // namespace facebook

0 commit comments

Comments
 (0)