Skip to content

Commit 8d77bf8

Browse files
committed
refactor: Make the async readiness gate awaitable end-to-end
Addresses review feedback on the interim warm-start refresh. The async readiness path is now awaitable at every layer instead of a private two-step refresh feeding a synchronous gate: - AsyncDataSystem.data_availability and AsyncLDClient.is_initialized() become coroutines; the eval path awaits data_availability() directly. - AsyncFeatureStore gains an abstract async is_initialized(); the getattr duck-type is dropped, so a custom store that omits it fails at construction instead of silently serving DEFAULTS forever. - The warm-start store error is caught inside data_availability and degrades to DEFAULTS, so evaluation never propagates a store error. - Removes the interim refresh_availability, cold-path gating, and start-time refresh, which are subsumed by the awaitable gate. - Adds a per-iteration stop-event check in the synchronizer loop so a perpetually-ready queue cannot starve the stop signal. The sync data system is unchanged.
1 parent 14cae18 commit 8d77bf8

16 files changed

Lines changed: 157 additions & 120 deletions

contract-tests/async_client_entity.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,8 @@ async def start(self):
104104
await self._client.start(start_wait / 1000.0)
105105
self._listeners = AsyncListenerRegistry(self._client.flag_tracker)
106106

107-
def is_initializing(self) -> bool:
108-
return self._client.is_initialized() if self._client else False
107+
async def is_initializing(self) -> bool:
108+
return await self._client.is_initialized() if self._client else False
109109

110110
async def evaluate(self, params: dict) -> dict:
111111
response = {}

contract-tests/async_service.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ async def handle_create_client(request: aiohttp.web.Request) -> aiohttp.web.Resp
102102
await client.close()
103103
return aiohttp.web.Response(text=str(e), status=500)
104104

105-
if not client.is_initializing() and not options['configuration'].get('initCanFail', False):
105+
if not await client.is_initializing() and not options['configuration'].get('initCanFail', False):
106106
await client.close()
107107
return aiohttp.web.Response(text='Failed to initialize', status=500)
108108

ldclient/async_client.py

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,7 @@ async def __start_up(self, start_wait: float):
227227
log.info("Waiting up to " + str(start_wait) + " seconds for LaunchDarkly client to initialize...")
228228
await update_processor_ready.wait(start_wait)
229229

230-
# Warm the persistent store's initialized state so a store populated by
231-
# another process is recognized before the readiness check.
232-
await self._data_system.refresh_availability()
233-
234-
if self.is_initialized() is True:
230+
if await self.is_initialized() is True:
235231
log.info("Started LaunchDarkly Client: OK")
236232
else:
237233
log.warning("Initialization timeout exceeded for LaunchDarkly Client or an error occurred. " "Feature Flags may not yet be available.")
@@ -383,18 +379,20 @@ def is_offline(self) -> bool:
383379
"""Returns true if the client is in offline mode."""
384380
return self._config.offline
385381

386-
def is_initialized(self) -> bool:
382+
async def is_initialized(self) -> bool:
387383
"""Returns true if the client has successfully connected to LaunchDarkly.
388384
389385
If this returns false, it means that the client has not yet successfully connected to LaunchDarkly.
390386
It might still be in the process of starting up, or it might be attempting to reconnect after an
391387
unsuccessful attempt, or it might have received an unrecoverable error (such as an invalid SDK key)
392388
and given up.
389+
390+
This is a coroutine because determining readiness may query a persistent store.
393391
"""
394392
if self.is_offline() or self._config.use_ldd:
395393
return True
396394

397-
return self._data_system.data_availability.at_least(DataAvailability.CACHED)
395+
return (await self._data_system.data_availability()).at_least(DataAvailability.CACHED)
398396

399397
async def flush(self):
400398
"""Flushes all pending analytics events.
@@ -495,13 +493,9 @@ async def _evaluate_internal(self, key: str, context: Context, default: Any, eve
495493
if self._config.offline:
496494
return EvaluationDetail(default, None, error_reason('CLIENT_NOT_READY')), None
497495

498-
# Refresh the store's initialized state only while still uninitialized;
499-
# once initialized or a basis arrives, the gate reads a cached value.
500-
if self._data_system.data_availability == DataAvailability.DEFAULTS:
501-
await self._data_system.refresh_availability()
502-
503-
if self._data_system.data_availability != DataAvailability.REFRESHED:
504-
if self._data_system.data_availability == DataAvailability.CACHED:
496+
availability = await self._data_system.data_availability()
497+
if availability != DataAvailability.REFRESHED:
498+
if availability == DataAvailability.CACHED:
505499
log.warning("Feature Flag evaluation attempted before client has initialized - using last known values from feature store for feature key: " + key)
506500
else:
507501
log.warning("Feature Flag evaluation attempted before client has initialized! Feature store unavailable - returning default: " + str(default) + " for feature key: " + key)
@@ -571,13 +565,9 @@ async def all_flags_state(self, context: Context, **kwargs) -> FeatureFlagsState
571565
log.warning("all_flags_state() called, but client is in offline mode. Returning empty state")
572566
return FeatureFlagsState(False)
573567

574-
# Refresh the store's initialized state only while still uninitialized;
575-
# once initialized or a basis arrives, the gate reads a cached value.
576-
if self._data_system.data_availability == DataAvailability.DEFAULTS:
577-
await self._data_system.refresh_availability()
578-
579-
if self._data_system.data_availability != DataAvailability.REFRESHED:
580-
if self._data_system.data_availability == DataAvailability.CACHED:
568+
availability = await self._data_system.data_availability()
569+
if availability != DataAvailability.REFRESHED:
570+
if availability == DataAvailability.CACHED:
581571
log.warning("all_flags_state() called before client has finished initializing! Using last known values from feature store")
582572
else:
583573
log.warning("all_flags_state() called before client has finished initializing! Feature store unavailable - returning empty state")

ldclient/async_feature_store.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,10 @@ def initialized(self) -> bool:
8888
""" """
8989
return self._initialized
9090

91+
async def is_initialized(self) -> bool:
92+
""" """
93+
return self._initialized
94+
9195
async def close(self) -> None:
9296
""" """
9397
pass

ldclient/async_feature_store_helpers.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,22 +100,23 @@ def initialized(self) -> bool:
100100
"""Returns the store's last observed initialized state."""
101101
return self._inited
102102

103-
async def refresh_initialized(self) -> None:
104-
"""Refreshes :attr:`initialized` from the store's own initialized state.
103+
async def is_initialized(self) -> bool:
104+
"""Queries the store's initialized state, updating :attr:`initialized`.
105105
106106
Honors the cache: with caching off the store is queried on every call;
107107
with a TTL it is queried once per interval; with an infinite TTL it is
108108
queried once. Once the store reports initialized the state latches and
109109
later calls return without I/O.
110110
"""
111111
if self._inited:
112-
return
112+
return True
113113
result = self._cache.get(AsyncCachingStoreWrapper.__INITED_CACHE_KEY__)
114114
if result is None:
115115
result = bool(await self._core.initialized_internal())
116116
self._cache[AsyncCachingStoreWrapper.__INITED_CACHE_KEY__] = result
117117
if result:
118118
self._inited = True
119+
return result
119120

120121
async def close(self) -> None:
121122
"""Releases the cache and closes the underlying core if it supports it."""

ldclient/impl/datasystem/__init__.py

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,11 @@ def flag_change_listeners(self) -> Listeners:
208208
"""
209209
raise NotImplementedError
210210

211-
@property
212211
@abstractmethod
213-
def data_availability(self) -> DataAvailability:
212+
async def data_availability(self) -> DataAvailability:
214213
"""
215-
Indicates what form of data is currently available.
214+
Indicates what form of data is currently available, awaiting the store's
215+
readiness so a persistent store populated by another process is recognized.
216216
"""
217217
raise NotImplementedError
218218

@@ -232,18 +232,6 @@ def store(self) -> AsyncReadOnlyStore:
232232
"""
233233
raise NotImplementedError
234234

235-
@abstractmethod
236-
async def refresh_availability(self) -> None:
237-
"""
238-
Refreshes any awaited state that :attr:`data_availability` depends on.
239-
240-
The client awaits this before reading :attr:`data_availability` so that a
241-
persistent store initialized by another process is reflected while a data
242-
source is configured but has not yet supplied a basis. Data systems with
243-
no such state treat this as a no-op.
244-
"""
245-
raise NotImplementedError
246-
247235

248236
class DiagnosticAccumulator(Protocol):
249237
def record_stream_init(self, timestamp, duration, failed):

ldclient/impl/datasystem/async_fdv1.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,6 @@ async def stop(self):
124124
def store(self) -> AsyncReadOnlyStore:
125125
return self._store_view
126126

127-
async def refresh_availability(self) -> None:
128-
"""No-op; ``data_availability`` reads the store's initialized state directly."""
129-
return None
130-
131127
def set_diagnostic_accumulator(self, diagnostic_accumulator: DiagnosticAccumulator):
132128
"""
133129
Sets the diagnostic accumulator for streaming initialization metrics.
@@ -147,18 +143,22 @@ def data_store_status_provider(self) -> DataStoreStatusProvider:
147143
def flag_change_listeners(self) -> Listeners:
148144
return self._flag_change_listeners
149145

150-
@property
151-
def data_availability(self) -> DataAvailability:
146+
async def data_availability(self) -> DataAvailability:
152147
if self._config.offline:
153148
return DataAvailability.DEFAULTS
154149

155150
if self._update_processor is not None and self._update_processor.initialized():
156151
return DataAvailability.REFRESHED
157152

158-
if self._store.initialized:
159-
return DataAvailability.CACHED
153+
# Awaits the store so a persistent store populated by another process is
154+
# recognized. A persistent-store error is logged and reported as no data.
155+
try:
156+
ready = await self._store.is_initialized()
157+
except Exception as e:
158+
log.warning("Error checking persistent store readiness: %s", e)
159+
return DataAvailability.DEFAULTS
160160

161-
return DataAvailability.DEFAULTS
161+
return DataAvailability.CACHED if ready else DataAvailability.DEFAULTS
162162

163163
@property
164164
def target_availability(self) -> DataAvailability:

ldclient/impl/datasystem/async_fdv2.py

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,11 @@
2121
join_handle,
2222
spawn_handle
2323
)
24-
from ldclient.impl.datasystem import AsyncDataSystem, DiagnosticSource
24+
from ldclient.impl.datasystem import (
25+
AsyncDataSystem,
26+
DataAvailability,
27+
DiagnosticSource
28+
)
2529
from ldclient.impl.datasystem.async_store import AsyncStore
2630
from ldclient.impl.datasystem.fdv2_common import (
2731
ConditionDirective,
@@ -92,16 +96,13 @@ async def delete(self, kind: VersionedDataKind, key: str, version: int) -> bool:
9296
def initialized(self) -> bool:
9397
return self._store.initialized
9498

95-
async def refresh_initialized(self) -> None:
96-
"""Refreshes the inner store's initialized state, if it supports it.
99+
async def is_initialized(self) -> bool:
100+
"""Queries the inner store's initialized state.
97101
98102
Runs through the availability wrapper so a failed query marks the store
99103
unavailable like any other operation.
100104
"""
101-
refresh = getattr(self._store, "refresh_initialized", None)
102-
if refresh is None:
103-
return
104-
await self._wrap(refresh)
105+
return await self._wrap(lambda: self._store.is_initialized())
105106

106107
def disable_cache(self) -> None:
107108
"""Disables the inner store's cache if it supports it."""
@@ -547,6 +548,10 @@ async def reader():
547548
sync_reader = spawn_handle("AsyncFDv2-sync-reader", reader)
548549

549550
while True:
551+
# Honor a stop request every iteration so a queue that always has
552+
# an item ready cannot starve the check.
553+
if self._stop_event.is_set():
554+
return ConditionDirective.FALLBACK
550555
update = await action_queue.get()
551556
if isinstance(update, str):
552557
if update == "quit":
@@ -608,16 +613,21 @@ def store(self) -> AsyncReadOnlyStore:
608613
"""Get the underlying store for flag evaluation."""
609614
return self._store_view
610615

611-
async def refresh_availability(self) -> None:
612-
"""Refreshes the persistent store's initialized state so a store populated
613-
by another process satisfies the availability gate before a synchronizer
614-
supplies a basis. A store error is logged and swallowed so the gate
615-
degrades to DEFAULTS and evaluation returns the default rather than
616-
propagating the error."""
616+
async def data_availability(self) -> DataAvailability: # type: ignore[override]
617+
"""Reports what form of data is currently available, awaiting the store's
618+
readiness so a persistent store populated by another process is recognized
619+
before a synchronizer supplies a basis. A persistent-store error is treated
620+
as no data: it is logged and reported as ``DEFAULTS`` rather than raised."""
621+
if self._store.selector().is_defined():
622+
return DataAvailability.REFRESHED
623+
if not self._configured_with_data_sources:
624+
return DataAvailability.CACHED
617625
try:
618-
await self._store.refresh_persistent_initialized()
626+
ready = await self._store.is_ready()
619627
except Exception as e:
620-
log.warning("Failed to refresh persistent store initialized state: %s", e)
628+
log.warning("Error checking persistent store readiness: %s", e)
629+
return DataAvailability.DEFAULTS
630+
return DataAvailability.CACHED if ready else DataAvailability.DEFAULTS
621631

622632

623633
__all__ = [

ldclient/impl/datasystem/async_store.py

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -206,21 +206,18 @@ def get_data_store_status_provider(self) -> Optional[DataStoreStatusProvider]:
206206
with self._lock:
207207
return self._persistent_store_status_provider
208208

209-
async def refresh_persistent_initialized(self) -> None:
210-
"""Refreshes the persistent store's initialized state while it is active.
209+
async def is_ready(self) -> bool:
210+
"""Reports whether the active store holds usable data.
211211
212-
Once the in-memory store is active, the persistent store is no longer read
213-
from, so its initialized state no longer gates reads and no query is made.
212+
Once the in-memory store is active its readiness is authoritative and no
213+
query is made. While the persistent store is active, its readiness is
214+
queried (awaiting the store), so a store populated by another process is
215+
recognized.
214216
"""
215217
store = self._persistent_store
216-
if store is None:
217-
return
218-
if self._active_store is self._memory_store:
219-
return
220-
refresh = getattr(store, "refresh_initialized", None)
221-
if refresh is None:
222-
return
223-
await refresh()
218+
if store is None or self._active_store is self._memory_store:
219+
return self._active_store.initialized
220+
return await store.is_initialized()
224221

225222

226223
__all__ = ["AsyncStore"]

ldclient/interfaces.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,16 @@ async def delete(self, kind: VersionedDataKind, key: str, version: int) -> bool:
408408
@abstractmethod
409409
def initialized(self) -> bool:
410410
"""
411-
Returns whether the store has been initialized yet or not.
411+
Returns the store's last observed initialized state without querying it.
412+
"""
413+
414+
@abstractmethod
415+
async def is_initialized(self) -> bool:
416+
"""
417+
Queries whether the store has been initialized, awaiting the store if a query is required.
418+
419+
A persistent store may have been populated by another process, so this can require I/O.
420+
Implementations should latch a positive result: once the store is initialized it stays so.
412421
"""
413422

414423
async def close(self) -> None:

0 commit comments

Comments
 (0)