Skip to content

Commit cc44864

Browse files
fix: move warn-and-proceed logic to evaluation path for lazy load
Reworked approach based on review feedback: Initialized() should return false when $inited is not set (consistent with other SDK implementations), and the evaluation path should handle this case by warning and proceeding rather than blocking. Changes: - Added CanEvaluateWhenNotInitialized() virtual method to IDataSystem interface (defaults to false) - LazyLoad overrides to return true (can serve on demand) - PreEvaluationChecks warns and proceeds when data system can evaluate while not initialized, instead of returning CLIENT_NOT_READY - AllFlagsState similarly warns and proceeds instead of returning empty - Reverted LazyLoad::Initialized() to original behavior (truthfully reports whether $inited key exists) - Added unit test for CanEvaluateWhenNotInitialized() This matches the pattern used in the Erlang SDK where the evaluation path distinguishes between 'not initialized' (blocks) and 'store initialized' (warns but proceeds). Co-Authored-By: rlamb@launchdarkly.com <kingdewman@gmail.com>
1 parent e5bf095 commit cc44864

5 files changed

Lines changed: 61 additions & 62 deletions

File tree

libs/server-sdk/src/client_impl.cpp

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -186,11 +186,19 @@ AllFlagsState ClientImpl::AllFlagsState(Context const& context,
186186
std::unordered_map<Client::FlagKey, Value> result;
187187

188188
if (!Initialized()) {
189-
LD_LOG(logger_, LogLevel::kWarn)
190-
<< "AllFlagsState() called before client has finished "
191-
"initializing. Data source not available. Returning empty state";
189+
if (data_system_->CanEvaluateWhenNotInitialized()) {
190+
LD_LOG(logger_, LogLevel::kWarn)
191+
<< "AllFlagsState() called before LaunchDarkly client "
192+
"initialization completed; using last known values "
193+
"from data store";
194+
} else {
195+
LD_LOG(logger_, LogLevel::kWarn)
196+
<< "AllFlagsState() called before client has finished "
197+
"initializing. Data source not available. Returning "
198+
"empty state";
192199

193-
return {};
200+
return {};
201+
}
194202
}
195203

196204
AllFlagsStateBuilder builder{options};
@@ -418,7 +426,16 @@ EvaluationDetail<Value> ClientImpl::VariationInternal(
418426
std::optional<enum EvaluationReason::ErrorKind> ClientImpl::PreEvaluationChecks(
419427
Context const& context) const {
420428
if (!Initialized()) {
421-
return EvaluationReason::ErrorKind::kClientNotReady;
429+
if (data_system_->CanEvaluateWhenNotInitialized()) {
430+
LD_LOG(logger_, LogLevel::kWarn)
431+
<< "Evaluation called before LaunchDarkly client "
432+
"initialization completed; using last known values "
433+
"from data store. The $inited key was not found in "
434+
"the store; typically a Relay Proxy or other SDK "
435+
"should set this key.";
436+
} else {
437+
return EvaluationReason::ErrorKind::kClientNotReady;
438+
}
422439
}
423440
if (!context.Valid()) {
424441
return EvaluationReason::ErrorKind::kUserNotSpecified;

libs/server-sdk/src/data_interfaces/system/idata_system.hpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,22 @@ class IDataSystem : public IStore {
2121
*/
2222
virtual void Initialize() = 0;
2323

24+
/**
25+
* @brief Returns true if the data system is capable of serving
26+
* flag evaluations even when Initialized() returns false.
27+
*
28+
* This is the case for Lazy Load (daemon mode), where data can be
29+
* fetched on-demand from the persistent store regardless of whether
30+
* the $inited key has been set. In contrast, Background Sync
31+
* cannot serve evaluations until initial data is received.
32+
*
33+
* When this returns true, the evaluation path should log a warning
34+
* (rather than returning CLIENT_NOT_READY) if Initialized() is false.
35+
*/
36+
[[nodiscard]] virtual bool CanEvaluateWhenNotInitialized() const {
37+
return false;
38+
}
39+
2440
virtual ~IDataSystem() override = default;
2541
IDataSystem(IDataSystem const& item) = delete;
2642
IDataSystem(IDataSystem&& item) = delete;

libs/server-sdk/src/data_systems/lazy_load/lazy_load_system.cpp

Lines changed: 12 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -79,12 +79,9 @@ std::string const& LazyLoad::Identity() const {
7979

8080
void LazyLoad::Initialize() {
8181
status_manager_.SetState(DataSourceState::kInitializing);
82-
// In lazy load mode, we always consider the system ready for
83-
// evaluations. The Initialized() call here will log a warning if
84-
// the underlying source reports not initialized (e.g. $inited key
85-
// not found), but we proceed regardless.
86-
Initialized();
87-
status_manager_.SetState(DataSourceState::kValid);
82+
if (Initialized()) {
83+
status_manager_.SetState(DataSourceState::kValid);
84+
}
8885
}
8986

9087
std::shared_ptr<data_model::FlagDescriptor> LazyLoad::GetFlag(
@@ -124,35 +121,25 @@ LazyLoad::AllSegments() const {
124121
}
125122

126123
bool LazyLoad::Initialized() const {
127-
/* In lazy load mode, the system is always considered initialized for
128-
* the purpose of flag evaluations. Data is fetched on-demand from the
129-
* underlying source regardless of the $inited key state.
130-
*
131-
* However, we still check the underlying source's initialized state
132-
* so we can warn if $inited is not set. A properly configured
133-
* Relay Proxy or other SDK populating the store should set the
134-
* $inited key. */
124+
/* Since the memory store isn't provisioned with an initial SDKDataSet
125+
* like in the Background Sync system, we can't forward this call to
126+
* MemoryStore::Initialized(). Instead, we need to check the state of the
127+
* underlying source. */
135128

136129
auto const state = tracker_.State(Keys::kInitialized, time_());
137130
if (initialized_.has_value()) {
131+
/* Once initialized, we can always return true. */
138132
if (initialized_.value()) {
139133
return true;
140134
}
135+
/* If not yet initialized, then we can return false only if the state is
136+
* fresh - otherwise we should make an attempt to refresh. */
141137
if (data_components::ExpirationTracker::TrackState::kFresh == state) {
142-
return true;
138+
return false;
143139
}
144140
}
145141
RefreshInitState();
146-
if (!initialized_.value_or(false) && !logged_init_warning_) {
147-
LD_LOG(logger_, LogLevel::kWarn)
148-
<< "LazyLoad: data source reports not initialized "
149-
"(the $inited key was not found in the store). "
150-
"Evaluations will proceed using available data. "
151-
"Typically a Relay Proxy or other SDK should set this "
152-
"key; verify your configuration if this is unexpected.";
153-
logged_init_warning_ = true;
154-
}
155-
return true;
142+
return initialized_.value_or(false);
156143
}
157144

158145
void LazyLoad::RefreshAllFlags() const {

libs/server-sdk/src/data_systems/lazy_load/lazy_load_system.hpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ class LazyLoad final : public data_interfaces::IDataSystem {
5858

5959
bool Initialized() const override;
6060

61+
[[nodiscard]] bool CanEvaluateWhenNotInitialized() const override {
62+
return true;
63+
}
64+
6165
// Public for usage in tests.
6266
struct Kinds {
6367
static integrations::FlagKind const Flag;
@@ -187,7 +191,6 @@ class LazyLoad final : public data_interfaces::IDataSystem {
187191
mutable data_components::ExpirationTracker tracker_;
188192
TimeFn time_;
189193
mutable std::optional<bool> initialized_;
190-
mutable bool logged_init_warning_{false};
191194

192195
ClockType::duration fresh_duration_;
193196

libs/server-sdk/tests/lazy_load_system_test.cpp

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ TEST_F(LazyLoadTest, AllSegmentsRefreshesIndividualSegment) {
283283
ASSERT_EQ(segment2->version, 2);
284284
}
285285

286-
TEST_F(LazyLoadTest, InitializedReturnsTrueEvenWhenSourceNotInitialized) {
286+
TEST_F(LazyLoadTest, InitializeNotQueriedRepeatedly) {
287287
built::LazyLoadConfig const config{
288288
built::LazyLoadConfig::EvictionPolicy::Disabled,
289289
std::chrono::seconds(10), mock_reader};
@@ -292,10 +292,8 @@ TEST_F(LazyLoadTest, InitializedReturnsTrueEvenWhenSourceNotInitialized) {
292292

293293
data_systems::LazyLoad const lazy_load(logger, config, status_manager);
294294

295-
// In lazy load mode, Initialized() always returns true even when the
296-
// underlying source reports not initialized ($inited key not found).
297295
for (std::size_t i = 0; i < 10; i++) {
298-
ASSERT_TRUE(lazy_load.Initialized());
296+
ASSERT_FALSE(lazy_load.Initialized());
299297
}
300298
}
301299

@@ -331,46 +329,24 @@ TEST_F(LazyLoadTest, InitializeCalledAgainAfterTTL) {
331329
data_systems::LazyLoad const lazy_load(logger, config, status_manager,
332330
[&]() { return now; });
333331

334-
// Always returns true even when source reports not initialized.
335332
for (std::size_t i = 0; i < 10; i++) {
336-
ASSERT_TRUE(lazy_load.Initialized());
333+
ASSERT_FALSE(lazy_load.Initialized());
337334
now += std::chrono::seconds(1);
338335
}
339336

340-
// Still true after TTL when source now reports initialized.
341337
for (std::size_t i = 0; i < 10; i++) {
342338
ASSERT_TRUE(lazy_load.Initialized());
343339
}
344340
}
345341

346-
TEST_F(LazyLoadTest, InitializedLogsWarningWhenSourceNotInitialized) {
347-
built::LazyLoadConfig const config{
348-
built::LazyLoadConfig::EvictionPolicy::Disabled,
349-
std::chrono::seconds(10), mock_reader};
350-
351-
EXPECT_CALL(*mock_reader, Initialized).WillOnce(Return(false));
352-
353-
data_systems::LazyLoad const lazy_load(logger, config, status_manager);
354-
355-
ASSERT_TRUE(lazy_load.Initialized());
356-
357-
// A warning should have been logged about $inited not being found.
358-
ASSERT_TRUE(spy_logger_backend->Contains(
359-
0, LogLevel::kWarn, "$inited"));
360-
}
361-
362-
TEST_F(LazyLoadTest, InitializedDoesNotLogWarningWhenSourceIsInitialized) {
342+
TEST_F(LazyLoadTest, CanEvaluateWhenNotInitialized) {
363343
built::LazyLoadConfig const config{
364344
built::LazyLoadConfig::EvictionPolicy::Disabled,
365345
std::chrono::seconds(10), mock_reader};
366346

367-
EXPECT_CALL(*mock_reader, Initialized).WillOnce(Return(true));
368-
369347
data_systems::LazyLoad const lazy_load(logger, config, status_manager);
370348

371-
ASSERT_TRUE(lazy_load.Initialized());
372-
373-
// No warning should be logged when source is properly initialized.
374-
// Only debug-level messages should be present (or none at all).
375-
ASSERT_TRUE(spy_logger_backend->Count(0));
349+
// LazyLoad can always serve evaluations on demand, even if not
350+
// initialized (i.e. $inited key not found in store).
351+
ASSERT_TRUE(lazy_load.CanEvaluateWhenNotInitialized());
376352
}

0 commit comments

Comments
 (0)