diff --git a/CLAUDE.md b/CLAUDE.md index 5d761a2..70ab689 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -199,6 +199,10 @@ par un appel réseau (D26). Les tailles de page par type viennent de l'observation des timeouts serveur — ne les augmentez pas à l'aveugle. +Les chargements sont **dédupliqués par clé de cache** : sous appels concurrents, +une seule chaîne de fetch part par clé et les autres appelants la rejoignent +(D27) — deux applications différentes se chargent toujours en parallèle. + `get_application_summary` expose l'état des caches sans redémarrage. --- diff --git a/DECISIONS.md b/DECISIONS.md index facfced..a9a07cd 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -645,3 +645,51 @@ passé), et ajoutent un `hint` quand la recherche revient **vide** : - Il nomme `CustomApp` en clair, sauf quand c'est déjà l'application interrogée : c'est une connaissance statique, déjà portée par les descriptions d'outils, pas une donnée à aller chercher. + +--- + +## D27 — Chargements paresseux sous concurrence : single-flight par clé + +**Contexte (mesuré le 25/08/2026, `LIMAGRAI2512`).** Le serveur traite les +`tools/call` **en concurrence** : une rafale d'appels dans une même session +s'exécute en parallèle. Les trois services à cache chargeaient paresseusement +sans se coordonner — le premier appelant qui trouve le cache invalide lance le +fetch, et tous ceux qui arrivent pendant ce fetch trouvent le cache **encore** +invalide et lancent le leur. Mesures avant correction : + +| Rafale | Résultat | +|---|---| +| 6 × `search_workflows` (CustomApp) | 6 × `fetching from API` pour une seule clé | +| 6 × `query_wms_entities` (Container) | 6 × `[EntityResolver] Cache expired or empty` — soit 30 GET Metadata | +| 14 appels mixtes | l'API AD répond **HTTP 500** sur `Workflow/GetByApplication` (EasyWMS, ~4 000 workflows) — les 4 appels EasyWMS échouent, les mêmes passent en séquentiel | + +La dernière ligne est le vrai coût : la duplication ne gaspille pas seulement +des appels, elle **surcharge l'API AD au point de la faire échouer**. + +**Décision.** Un motif unique, `src/services/single-flight.js`, partagé par +`workflow-service`, `ad-service` et `entity-resolver` — une `Map` de promesses, +pas une dépendance externe : + +- **Une clé de single-flight par entrée de cache** : `workflows::` et + `applications` pour les workflows, `::` pour l'AD, une clé unique + pour le resolver. Deux clés distinctes se chargent toujours **en parallèle** — + le single-flight ne sérialise rien au-delà de la clé demandée, et + n'introduit aucun préchargement (D26 intact). +- **La promesse est retirée au règlement, succès *ou* échec.** Un fetch en + erreur ne reste pas coincé dans la Map : l'appel suivant refetche. Les + appelants joints reçoivent la même erreur, et rien n'est mis en cache. +- **Le log de fetch reste l'observable** (D6) : une ligne `fetching from API` + / `Cache expired or empty` par chargement **réel**. Les appelants joints + émettent une ligne distincte (`Fetch already in flight … joining it`) — ne + fusionnez pas les deux, c'est ce qui rend la déduplication vérifiable depuis + stderr. + +**Mesures après.** Rafale de 6 (CustomApp) → 1 fetch + 5 joins, les 6 réponses +à `count: 44`. Rafale de 6 (resolver) → 1 chargement, 5 GET Metadata au lieu de +30. Rafale mixte EasyWMS + CustomApp → **un fetch par application**, deux au +total, plus aucun HTTP 500. Le chemin séquentiel nominal est inchangé : 1 fetch +puis 1 `Using cached data`, 0 join. + +**Ce que cette décision ne couvre pas.** La bascule de profil concurrente aux +appels en vol (un `switch_wms_profile` qui redirige des requêtes déjà parties) +reste un point ouvert de la ROADMAP, distinct. diff --git a/ROADMAP.md b/ROADMAP.md index 045bd43..801cac9 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -95,12 +95,6 @@ Rejoués en séquentiel, les mêmes appels sont corrects. Trois défauts structurels dans les services à cache (`workflow-service`, `ad-service`, `entity-resolver`) : -### L6.1 — Dédupliquer les fetchs en vol - -Aucun service ne mémorise la promesse de chargement en cours : N appelants -concurrents sur la même clé déclenchent N chaînes de fetch complètes. -Single-flight par clé de cache (promesse partagée, libérée au règlement). - ### L6.2 — Protéger l'invalidation contre les fetchs en vol Un fetch parti avant `clearCache()`/`invalidateCache()` (bascule de profil, diff --git a/src/services/ad-service.js b/src/services/ad-service.js index 8ce9761..9ed4ac2 100644 --- a/src/services/ad-service.js +++ b/src/services/ad-service.js @@ -6,12 +6,16 @@ const apiService = require('./api-service').getInstance(); const profileManager = require('../config/profile-manager'); +const { createSingleFlight } = require('./single-flight'); // Cache state - one cache per (application, element type) (D26) const cache = {}; const cacheTimestamps = {}; const CACHE_TTL = parseInt(process.env.WORKFLOW_CACHE_TTL) || 3600000; // 1 hour +// Déduplication des chargements concurrents, une clé par (application, type) (D27). +const singleFlight = createSingleFlight('AD'); + /** * Application effective : celle demandée, sinon celle du profil actif. */ @@ -96,6 +100,15 @@ async function getElements(elementType, application) { return cache[key]; } + // Un seul chargement par (application, type), même sous rafale (D27). + return singleFlight.run(key, () => loadElements(app, elementType, key)); +} + +/** + * Chargement réel d'un (application, type) (pagination complète). + * Appelé au plus une fois par clé tant qu'il est en vol (D27). + */ +async function loadElements(app, elementType, key) { console.error(`[AD] Cache expired or empty, fetching ${key}...`); try { diff --git a/src/services/entity-resolver.js b/src/services/entity-resolver.js index 035892b..50a5751 100644 --- a/src/services/entity-resolver.js +++ b/src/services/entity-resolver.js @@ -11,6 +11,7 @@ const apiService = require('./api-service').getInstance(); const profileManager = require('../config/profile-manager'); +const { createSingleFlight } = require('./single-flight'); // Cache state — même TTL que les autres caches (D10) let resolutionMap = null; // Map lower(Name | TableName) -> TableName @@ -18,6 +19,10 @@ let tableNames = null; // TableName[] triés (suggestions + comptage) let cacheTimestamp = null; const CACHE_TTL = parseInt(process.env.WORKFLOW_CACHE_TTL) || 3600000; +// Table unique : une seule clé de single-flight (D27). +const singleFlight = createSingleFlight('EntityResolver'); +const METADATA_KEY = 'metadata'; + // La table de résolution est par tenant — invalidée à chaque bascule (D8). profileManager.onSwitch(() => invalidateCache()); @@ -37,6 +42,15 @@ function isCacheValid() { async function loadResolutionMap() { if (isCacheValid()) return; + // Un seul chargement Metadata, même sous rafale concurrente (D27) : sans + // lui, 6 appels concurrents déclenchaient 6 chargements complets. + await singleFlight.run(METADATA_KEY, fetchResolutionMap); +} + +/** + * Chargement réel de la table de résolution (D27). + */ +async function fetchResolutionMap() { console.error('[EntityResolver] Cache expired or empty, fetching Metadata...'); const apps = await apiService.get('/configuration/applications'); diff --git a/src/services/single-flight.js b/src/services/single-flight.js new file mode 100644 index 0000000..6fc6922 --- /dev/null +++ b/src/services/single-flight.js @@ -0,0 +1,70 @@ +/** + * Single-flight — déduplication des chargements paresseux en vol (D27) + * + * Les services à cache (workflow, AD, resolver) chargent paresseusement : le + * premier appelant qui trouve le cache invalide déclenche le fetch. Le serveur + * traitant les `tools/call` en concurrence, N appelants arrivés pendant ce + * fetch trouvaient tous le cache invalide et lançaient N chaînes complètes + * (mesuré : 6 chargements Metadata en parallèle pour une seule table). + * + * Le motif est le même partout : une Map `clé de cache -> promesse en vol`, + * pas de dépendance externe. Le single-flight est **par clé** — deux + * applications différentes se chargent toujours en parallèle (D26 : rien n'est + * préchargé, et rien n'est sérialisé au-delà de la clé demandée). + * + * @param {string} label - préfixe de log du service appelant (D6, MONITORING §2) + */ +function createSingleFlight(label) { + const inFlight = new Map(); // clé de cache -> promesse du chargement en cours + + /** + * Exécute `fetcher` pour cette clé, ou rejoint le chargement déjà en vol. + * La promesse est retirée de la Map au règlement, succès **ou** échec : un + * fetch en erreur ne reste pas coincé, l'appel suivant refetche. + * + * @param {string} key - clé de cache (une par entrée de cache indépendante) + * @param {() => Promise} fetcher - le chargement réel, appelé au plus + * une fois tant qu'il est en vol + * @returns {Promise} le résultat du chargement (partagé par les joignants) + */ + function run(key, fetcher) { + const pending = inFlight.get(key); + if (pending) { + console.error(`[${label}] Fetch already in flight for "${key}", joining it`); + return pending; + } + + const promise = (async () => fetcher())(); + inFlight.set(key, promise); + + // Libération au règlement. Le test d'identité évite qu'une promesse + // périmée (Map vidée par une invalidation, puis nouveau fetch démarré) + // supprime l'entrée de son successeur. + const release = () => { + if (inFlight.get(key) === promise) inFlight.delete(key); + }; + promise.then(release, release); + + return promise; + } + + /** + * Oublie les promesses en vol (invalidation de cache). Les appelants déjà + * en attente reçoivent bien leur résultat — c'est la mise en cache de ce + * résultat qui est refusée, ailleurs. + */ + function clear() { + inFlight.clear(); + } + + /** + * Nombre de chargements en vol — diagnostic seulement. + */ + function pendingCount() { + return inFlight.size; + } + + return { run, clear, pendingCount }; +} + +module.exports = { createSingleFlight }; diff --git a/src/services/workflow-service.js b/src/services/workflow-service.js index d981896..4488355 100644 --- a/src/services/workflow-service.js +++ b/src/services/workflow-service.js @@ -9,6 +9,7 @@ const apiService = require('./api-service').getInstance(); const profileManager = require('../config/profile-manager'); +const { createSingleFlight } = require('./single-flight'); // Cache state — un cache de workflows par application (D26) let workflowCaches = {}; // application -> workflows[] @@ -17,6 +18,10 @@ let applicationsCache = null; // liste allégée de POST /Application/GetAll let applicationsTimestamp = null; const CACHE_TTL = parseInt(process.env.WORKFLOW_CACHE_TTL) || 3600000; // 1 hour in milliseconds +// Déduplication des chargements concurrents, par clé de cache (D27). Deux +// clés distinctes ici : une par application, plus la liste d'applications. +const singleFlight = createSingleFlight('Workflow'); + // Clear cache when profile changes — workflows are per-tenant, so the previous // profile's cache is meaningless after a switch. profileManager.onSwitch(() => clearCache()); @@ -56,6 +61,15 @@ async function fetchAllWorkflows(application) { return workflowCaches[app]; } + // Un seul chargement par application, même sous rafale concurrente (D27). + return singleFlight.run(`workflows::${app}`, () => loadWorkflows(app)); +} + +/** + * Chargement réel des workflows d'une application (pagination complète). + * Appelé au plus une fois par application tant qu'il est en vol (D27). + */ +async function loadWorkflows(app) { console.error(`[Workflow] Cache expired or empty for "${app}", fetching from API...`); try { @@ -113,11 +127,17 @@ async function fetchAllWorkflows(application) { * @returns {Promise>} */ async function fetchApplications() { - if (applicationsCache && applicationsTimestamp && - Date.now() - applicationsTimestamp < CACHE_TTL) { - return applicationsCache; - } + const cached = getCachedApplications(); + if (cached) return cached; + // Même déduplication que les workflows, sur sa propre clé (D27). + return singleFlight.run('applications', loadApplications); +} + +/** + * Chargement réel de la liste d'applications (D27). + */ +async function loadApplications() { console.error('[Workflow] Fetching application list (Application/GetAll)...'); const response = await apiService.post('/Application/GetAll', null, true); const entities = response?.entities || [];