Skip to content

Commit

Permalink
Track if a setting has had callbacks
Browse files Browse the repository at this point in the history
Summary: * Add an API to tell if a setting has ever had a callback (RFC on naming here, seems like we could do better)

Reviewed By: Gownta

Differential Revision: D67534645

fbshipit-source-id: 45223bb408528224ab319c0d25a0f1b9473c13bf
  • Loading branch information
Kevin Doherty authored and facebook-github-bot committed Jan 3, 2025
1 parent aea6096 commit b5650df
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 0 deletions.
6 changes: 6 additions & 0 deletions folly/settings/detail/SettingsImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class SettingCoreBase {
virtual void forceResetToDefault(SnapshotBase* snapshot) = 0;
virtual const SettingMetadata& meta() const = 0;
virtual uint64_t accessCount() const = 0;
virtual bool hasHadCallbacks() const = 0;
virtual ~SettingCoreBase() {}

/**
Expand Down Expand Up @@ -192,6 +193,7 @@ class SnapshotBase {
std::pair<std::string, std::string> valueAndReason() const;
const std::string& fullName() const { return fullName_; }
uint64_t accessCount() const { return core_.accessCount(); }
bool hasHadCallbacks() const { return core_.hasHadCallbacks(); }

private:
const std::string& fullName_;
Expand Down Expand Up @@ -335,6 +337,8 @@ class SettingCore : public SettingCoreBase {

uint64_t accessCount() const override { return AccessCounter::count(); }

bool hasHadCallbacks() const override { return hasHadCallbacks_.load(); }

/**
* @param trivialStorage must refer to the same location
* as the internal trivialStorage_. This hint will
Expand Down Expand Up @@ -387,6 +391,7 @@ class SettingCore : public SettingCoreBase {
SettingCore<T, Tag>& setting_;
};
CallbackHandle addCallback(UpdateCallback callback) {
hasHadCallbacks_.store(true);
auto callbackPtr = copy_to_shared_ptr(std::move(callback));

auto copiedPtr = callbackPtr;
Expand Down Expand Up @@ -420,6 +425,7 @@ class SettingCore : public SettingCoreBase {
std::atomic<uint64_t>& trivialStorage_;

F14FastSet<std::shared_ptr<UpdateCallback>> callbacks_;
std::atomic<bool> hasHadCallbacks_{false};

/* Thread local versions start at 0, this will force a read on first access.
*/
Expand Down
10 changes: 10 additions & 0 deletions folly/settings/test/SettingsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -645,6 +645,11 @@ TEST(SettingsTest, callback) {
some_ns::FOLLY_SETTING(follytest, some_flag).set("e");
EXPECT_EQ(callbackInvocations, 4);
EXPECT_EQ(lastCallbackValue, "d");

folly::settings::Snapshot snapshot;
snapshot.forEachSetting([](const auto& s) {
EXPECT_EQ(s.hasHadCallbacks(), s.fullName() == "follytest_some_flag");
});
}

TEST(SettingsTest, observers) {
Expand All @@ -661,6 +666,11 @@ TEST(SettingsTest, observers) {
folly::observer_detail::ObserverManager::waitForAllUpdates();
EXPECT_EQ(**observer, "new value");
EXPECT_EQ(updatedFromCallback, "new value");

folly::settings::Snapshot snapshot;
snapshot.forEachSetting([](const auto& s) {
EXPECT_EQ(s.hasHadCallbacks(), s.fullName() == "follytest_some_flag");
});
}

TEST(Settings, immutables) {
Expand Down

0 comments on commit b5650df

Please sign in to comment.