# Performance Fix Plan — Booking Window Activity Service _Companion to `ARCHITECTURE_NOTES.md` §9. This is a step-by-step "how to fix it" for each bottleneck identified there. Originally written before any code changed — now updated to reflect what's actually shipped. I don't have credentials to run SQL against your live database from this session, so the index migration is written as a script for you/your DBA to run; the code fixes are written as exact before/after so whoever picks this up can apply them with confidence._ **Recommended order**: do items 1–3 first (cheap, low-risk, highest payoff). Do 4–6 next. Treat item 7 (the pricing-pipeline dedup) as its own project — don't bundle it with the others, and don't ship it without a side-by-side pricing comparison against production data first, since it changes code that computes what customers get charged. ## Status (last updated 2026-08-25) | # | Item | Status | |---|---|---| | 1 | Indexes on `fd_activities_booking`/`booking_transactions` | ⏳ Not started — needs a DBA to run the migration directly, can't be done from this session | | 2 | `create_booking` connection-holding fix | ✅ Done — `controllers/webController.js` | | 3 | Debug logging cleanup | ✅ Done — gated behind `Config.DEBUG_SQL` (`config.js` + `DEBUG_SQL` env var) across `webController.js`, `activityController.js`, `activityFrontController.js`, `activityInventoryController.js`; hardcoded `inventoryMap[873]` dumps deleted | | 4 | Trim `SELECT *` on list endpoints | ⏳ Not started — needs frontend coordination first | | 5 | Puppeteer pooling/queueing | ✅ Done (Option A) — `services/concurrencyLimiter.js` (vendored, zero-dependency; `p-limit` couldn't be installed, npm registry is blocked from this session) caps `bookingPdfService.js` at `PDF_CONCURRENCY` (default 2) concurrent PDF generations. Option B (shared browser instance) not done. | | 6 | Redis for markup/currency caching | ✅ Done — `services/redisClient.js` (new), `globalService.js` now L1 in-memory → L2 Redis → L3 DB, all Redis calls best-effort (never throws to callers, falls through to DB on any Redis error). `ioredis` added to `package.json` but **not yet installed** — no network access to npm from this session; run `npm install` and provision `REDIS_HOST`/`REDIS_PORT`/`REDIS_PASSWORD`/`REDIS_DB` before this does anything (falls through to the pre-existing in-memory cache until then, so it's safe to have shipped ahead of the Redis instance existing). | | 7 | Dedup the markup+currency pipeline | 🔶 Partially done — see the dedicated section below. The risk-free part (merging `activityFrontController.js`'s 3 byte-identical copies) shipped. The `webController.js` reconciliation (D/E/F) is confirmed **intentional, not a gap** — deliberately not touched. | | 8 | Indexes on `activity`/`sub_activity` | ⏳ Not started — same DBA-migration blocker as #1 | --- ## 1. Add missing indexes on `fd_activities_booking` and `booking_transactions` **Root cause**: both tables only have `PRIMARY KEY (id)`. Every list/filter/sort query on them is a full table scan. **Fix** — run this migration (adjust table name prefixes if your environment uses `Config.DB_PREFIX`): ```sql -- fd_activities_booking ALTER TABLE fd_activities_booking ADD INDEX idx_fab_agent_id (agent_id), ADD INDEX idx_fab_staff_id (staff_id), ADD INDEX idx_fab_reference (reference_type, reference_id), ADD INDEX idx_fab_booking_ref_no (booking_ref_no), ADD INDEX idx_fab_ops_confirmed (ops_confirmed), ADD INDEX idx_fab_source (source), ADD INDEX idx_fab_created (created), ADD INDEX idx_fab_from_date (from_date), ADD INDEX idx_fab_create_payout (create_payout), ADD INDEX idx_fab_city (city), ADD INDEX idx_fab_country (country); -- Composite for the single most common query shape (list filtered by reference_type=1 -- + status flags + sorted by created) — covers get_booking_list's default sort: ALTER TABLE fd_activities_booking ADD INDEX idx_fab_list_common (reference_type, ops_confirmed, created); -- booking_transactions ALTER TABLE booking_transactions ADD INDEX idx_bt_reference (refrence_type, refrence_id), ADD INDEX idx_bt_ref_no (ref_no), ADD INDEX idx_bt_user_id (user_id), ADD INDEX idx_bt_created (created), ADD INDEX idx_bt_status (status); ``` **Why these specific columns**: matched directly against the `WHERE`/`ORDER BY` clauses in `get_booking_list`, `get_operations_list`, `listActivitiesPayouts`, `getBookingTransactions`, `addBookingTransaction`, `ChangeRefundStatus` in `webController.js`. The composite `idx_fab_list_common` targets `get_booking_list`'s default (no-filter) case, which is the most-hit path. **Before running in production**: 1. Run `EXPLAIN` on the actual queries (copy them out of `webController.js` — they're logged to console already, so grab one from your logs) against a staging copy with production-like row counts, before and after, and confirm the plan switches from `type: ALL` (full scan) to `type: ref`/`range`. 2. Check current row counts (`SELECT COUNT(*) FROM fd_activities_booking; SELECT COUNT(*) FROM booking_transactions;`) — if either table is already large (millions of rows), still expect the `ADD INDEX` itself to take real time and I/O even though MySQL 8.4 does it online (`ALGORITHM=INPLACE`, no table lock for secondary index creation) — run it during a low-traffic window regardless, and watch replication lag if you have replicas. 3. Don't add every index blind — if `EXPLAIN` shows MySQL isn't choosing a given index (low selectivity, e.g. `ops_confirmed` only has 3 values), drop that one from the migration. Composite indexes usually beat single-column ones for the exact filter combos above; the list here is a starting point, not gospel. **Risk**: low. Adding indexes doesn't change query results, only speed. The only downside is slightly slower writes (each `INSERT`/`UPDATE` on `fd_activities_booking` now maintains 8 more indexes) and extra disk space — acceptable for a table that's read far more than it's written. **Files affected**: none in the repo (this is a DB-only change) — unless you keep migrations versioned in the repo, in which case add a new file under wherever your migration files live (I didn't find a `migrations/` folder in this project, so you may be applying schema changes by hand today — worth setting one up so this change is tracked). --- ## 2. Fix the connection-holding bug in `create_booking` — ✅ DONE **Root cause**: `webController.js`, `create_booking` (~line 2582) acquires a pool connection and doesn't release it until after `await DiductbalanceInWebsiteUserWallet(...)` (~line 2777/2785) completes. That helper (top of `webController.js`, ~line 50-111) does its own `pool.promise().getConnection()` — held for the entire duration of an `axios.post` to the external FB Admin API (10s timeout, up to 3 retries) — **without ever running a query on it**. Two pool connections held idle per booking, for as long as that external call takes. **Fix, part A — release the outer connection before the external call.** Move the wallet-deduction call to happen *after* the connection is released, since nothing after the `INSERT`/score-update needs the DB connection: ```js // BEFORE (current, abbreviated) const connection = await pool.promise().getConnection(); try { // ...wallet balance check, duplicate check, INSERT booking, UPDATE activity.score... var booking_id = result.insertId; let narration = `Booking for FD Activity : ${title} / ${booking_ref_no}`; await DiductbalanceInWebsiteUserWallet(agent_id, booking_ref_no, narration, booking_id, total_price, StaffData.id); setImmediate(() => { sendBookingMail({ ... }); }); resultJson.replyCode = 'success'; // ... connection.release(); callback(200, null, resultJson); } catch (error) { ... } // AFTER const connection = await pool.promise().getConnection(); let booking_id, booking_ref_no_local; try { // ...wallet balance check, duplicate check, INSERT booking, UPDATE activity.score... booking_id = result.insertId; booking_ref_no_local = booking_ref_no; } catch (error) { connection.release(); // ...existing error handling... return; } finally { // release as soon as the DB work is done, BEFORE the external HTTP call connection.release(); } // now outside the connection's lifetime let narration = `Booking for FD Activity : ${title} / ${booking_ref_no_local}`; await DiductbalanceInWebsiteUserWallet(agent_id, booking_ref_no_local, narration, booking_id, total_price, StaffData.id); setImmediate(() => { sendBookingMail({ ... }); }); resultJson.replyCode = 'success'; // ... callback(200, null, resultJson); ``` (Adjust variable scoping to fit the actual function — the point is: everything that needs `connection` happens in one block that releases the connection immediately after, and the wallet call happens afterward with no connection held.) **Fix, part B — remove the pointless connection acquisition inside the helper.** In `DiductbalanceInWebsiteUserWallet` (top of `webController.js`), the `pool.promise().getConnection()` / `connection.release()` wrapping does nothing (no query runs on `connection` inside it) — delete it: ```js // BEFORE const DiductbalanceInWebsiteUserWallet = async (user_id, booking_ref_no, narration, booking_id, amount, created_by_id) => { user_id = user_id || null; amount = parseFloat(amount || 0); let resultJson = {}; try { const connection = await pool.promise().getConnection(); // <-- never queried try { const payload = { ... }; const result = await insertAccountTransaction(payload); resultJson = result.success ? { replyCode: "success", ... } : { replyCode: "error", ... }; } catch (queryError) { resultJson = { replyCode: "error", replyMsg: queryError.message, ... }; } finally { connection.release(); } } catch (connectionError) { resultJson = { replyCode: "error", replyMsg: "Database connection failed: " + connectionError.message, ... }; } return resultJson; }; // AFTER const DiductbalanceInWebsiteUserWallet = async (user_id, booking_ref_no, narration, booking_id, amount, created_by_id) => { user_id = user_id || null; amount = parseFloat(amount || 0); let resultJson = {}; try { const payload = { account_type: "activity_booking", type: "fd_activity", user_id, amount, narration: narration || 'Amount deducted for booking ref no:' + booking_ref_no, booking_id, bank_id: 0, reference_id: booking_ref_no, rec_staff_id: created_by_id }; const result = await insertAccountTransaction(payload); resultJson = result.success ? { replyCode: "success", replyMsg: "Balance deducted successfully", cmd: "DiductbalanceInWebsiteUserWallet function" } : { replyCode: "error", replyMsg: "Failed to add balance.", cmd: "DiductbalanceInWebsiteUserWallet function" }; } catch (err) { resultJson = { replyCode: "error", replyMsg: err.message, cmd: "DiductbalanceInWebsiteUserWallet function" }; } return resultJson; }; ``` **Testing**: create a test booking end-to-end (including a case where the FB Admin API is slow/unreachable — point `FB_ADMIN_API_URL` at a delayed test endpoint if you have one) and confirm: the booking row is still created correctly, the wallet deduction still happens/retries correctly, and — using `SHOW PROCESSLIST` or your pool's connection-count metric — confirm connection count no longer climbs during a burst of concurrent test bookings. **Risk**: low-medium. It's a small, mechanical change, but it touches the money-movement path, so test the failure case (external API down) specifically — confirm the booking doesn't silently succeed with no ledger entry and no visible error, since that behavior (booking commits before the ledger call, no rollback on ledger failure) already exists today and this fix doesn't change that pre-existing gap — see the "also worth fixing" note at the end. **Files affected**: `controllers/webController.js` only. --- ## 3. Clean up debug logging on hot paths — ✅ DONE **Root cause**: `console.log` of full SQL + params (and one hardcoded `console.log('...inventoryMap[873]...')` debug leftover) fires on every request in several controllers. **Fix**: gate these behind a debug flag instead of deleting outright (useful in dev): ```js // add near the top of each affected file, or centralize in config.js const DEBUG_SQL = process.env.DEBUG_SQL === 'true'; // default off // then wrap existing logs: if (DEBUG_SQL) console.log('selectQuery', selectQuery, selectParams); ``` Delete the hardcoded `console.log('JSON.stringify(inventoryMap[873]...')` lines outright (in `activityFrontController.js`, both `getAllActivities` and `getlActivityById`) — those reference a specific activity ID and were clearly left in from local debugging, not meant to gate on anything. **Risk**: none — purely subtractive/additive logging change, doesn't touch logic. **Files affected**: `controllers/activityController.js`, `activityFrontController.js`, `activityInventoryController.js`, `webController.js` (wherever `console.log(query, params)` patterns appear — grep for `console.log(.*[Qq]uery`). --- ## 4. Trim `SELECT *` on list endpoints **Root cause**: `activity.*` and similar wildcard selects pull large `text`/`json` columns (`description`, `highlights`, `inclusion`, `exclusion`, `restrictions`, `other_images`, `supplier`, `addon`, `combo_activities`, `restaurants`, `video_url`) into list responses that typically render a card/row, not the full detail. **Fix approach**: for each list endpoint, replace `SELECT activity.*` with an explicit column list of what the list UI actually renders (name, city, country, images, adult/child/infant price, status, score, from_date/to_date, and whatever else the frontend list view uses), and keep `SELECT *` only in the `_by_id`/detail endpoints (`getActivityById`, `getlActivityById`) where the full payload is expected. **Do this one carefully and incrementally** — before removing a column from a list query, confirm with the frontend (or by searching the frontend repo) that nothing in the list view actually reads it. This is the kind of change that's invisible in testing and breaks a UI field in production if you guess wrong. Suggested process: add the explicit column list, deploy to staging, diff the JSON response shape against production for the same request, and only then ship. **Files affected**: `controllers/activityController.js` (`getAllActivities`), `controllers/activityFrontController.js` (`getAllActivities`), `controllers/webController.js` (`getAllActivities`, `getMealActivities`). **Risk**: medium — not because the DB change is risky, but because trimming a response shape can silently break a frontend that reads a field you removed. Coordinate with whoever owns the frontend before shipping. --- ## 5. Puppeteer: pool or queue PDF generation instead of launch-per-booking — ✅ DONE (Option A) **Root cause**: `services/bookingPdfService.js`'s `generateBookingPdf` does `puppeteer.launch()` / `browser.close()` per call, fired via `setImmediate` in `create_booking` with no concurrency limit. **Two options, in order of effort**: **Option A (smaller change) — cap concurrency with a simple queue.** Use a lightweight in-process limiter so no more than N PDF generations run at once, regardless of how many bookings land simultaneously: ```js // services/bookingPdfService.js const pLimit = require('p-limit'); // npm install p-limit const limit = pLimit(2); // tune based on server CPU/memory — start conservative async function generateBookingPdf(args) { return limit(() => _generateBookingPdfInner(args)); } // rename the existing function body to _generateBookingPdfInner, export generateBookingPdf as above ``` This bounds the damage from a booking burst without touching the browser-launch code at all. Low risk, small diff. **Option B (bigger, better long-term) — one shared browser instance, new page per PDF.** Launch Puppeteer once at process start, keep it alive, and call `browser.newPage()` per request instead of `puppeteer.launch()`: ```js // services/bookingPdfService.js let browserPromise = null; function getBrowser() { if (!browserPromise) { browserPromise = puppeteer.launch({ headless: 'new', args: ['--no-sandbox', '--disable-setuid-sandbox', '--allow-file-access-from-files'] }); } return browserPromise; } async function generateBookingPdf({ booking, activity, agent_data, guest, pax, userAgent }) { const browser = await getBrowser(); const page = await browser.newPage(); try { // ...same setViewport/setContent/pdf logic as today, but using `page` from the shared browser... } finally { await page.close(); // close the PAGE, not the browser } } ``` This removes the ~1-3s browser cold-start cost from every booking, at the cost of needing to handle browser crash/recovery (wrap `getBrowser()` to detect a dead browser and relaunch) and watching for page leaks if `page.close()` isn't reliably reached on every code path (use `try/finally` as shown, and audit the existing error paths in the function for the same). **Recommendation**: do Option A now (an hour of work, contains the immediate risk), consider Option B later if PDF volume grows enough that the per-booking latency from cold Chromium starts becomes a real user-facing delay (right now it's fire-and-forget via `setImmediate`, so it doesn't block the booking response — it mainly matters for server resource pressure under a burst, which Option A already addresses). **Files affected**: `services/bookingPdfService.js`. Add `p-limit` to `package.json` if going with Option A. **Risk**: low for Option A, medium for Option B (introduces a long-lived process resource that needs lifecycle/error handling). --- ## 6. Wire in a shared cache (Redis) for markup/currency lookups — ✅ DONE **Root cause**: `services/globalService.js` caches markup rules and currency rates in a per-process `Map` (60s TTL) — not shared across instances, wiped on restart, and this is currently the *only* caching in the whole service (nothing caches listing responses). **Fix approach** (assuming you provision or already have a Redis instance available — I found zero Redis wiring in this repo, so this is new infrastructure for this service specifically): 1. `npm install ioredis`. 2. Add Redis connection config to `config.js` (`config.REDIS_HOST`, `config.REDIS_PORT`, etc., sourced from env like everything else in that file) and a small `services/redisClient.js` that exports a single connected client. 3. In `globalService.js`, replace the `Map`-based caches (`currencyRatesCache`, `currencyChargesCache`, `userGroupMarkupCache`, `countryMarkupCache`) with Redis `GET`/`SETEX` calls using the same keys and a similar TTL (60s is reasonable to start; currency rates in particular could probably go longer, e.g. 5-15 minutes, since they don't change every minute in practice — confirm with whoever owns that data before changing the TTL). 4. Keep the function signatures identical (`getRatesAndChargesMap`, `getUserGroupMarkupRule`, `getCountryMarkupRule`) — only the storage backend changes, so callers in `activityController.js`/`activityFrontController.js`/`webController.js` need no changes. **Bigger win, separate follow-up**: once Redis is wired in for this, it's also the natural place to cache full listing responses (`getAllActivities`/`getlActivityById`) keyed by the filter params + date, with a short TTL (seconds, not minutes, since inventory/availability changes) and explicit invalidation on `updateActivityInventory`/`add_activity_inventory`. That's a bigger design decision (cache key shape, invalidation triggers) — I'd scope it as its own task rather than bundling it into the markup/currency swap. **Risk**: low for the markup/currency swap (drop-in replacement behind existing function signatures) — the main risk is operational (a new dependency that can itself go down; make sure the code falls back gracefully — e.g. treat a Redis error as a cache miss and fall through to the DB query, don't let a Redis outage take down pricing). **Files affected**: `config.js`, new `services/redisClient.js`, `services/globalService.js`, `package.json`. --- ## 7. Deduplicate and parallelize the markup+currency pipeline (do this last, as its own project) — 🔶 PARTIALLY DONE **Update, 2026-08-25 — what actually happened, superseding the "5 copies" framing below:** the codebase moved between when this plan was written and when §7 was picked up (`activityFrontController.js` grew a new `getActivityHolidaybyIds` endpoint that copy-pastes the same pipeline a third time). A full re-diff (see `MARKUP_PIPELINE_DEDUP_FINDINGS.md`) found **6 call sites, only 4 distinct implementations**: `activityFrontController.js`'s `getAllActivities`/`getlActivityById`/`getActivityHolidaybyIds` turned out to be **byte-for-byte identical**, not just "near-identical" — so that part was a zero-risk merge, not a reconciliation. Asked whether `webController.js`'s simpler `getAllActivities`/`getMealActivities`/`getlActivityById` (missing category-based inventory, guide pricing, per-category conversion, cheapest-category promotion) is a gap or intentional — confirmed **intentional, different consumer**, not a bug. Done: the three byte-identical `activityFrontController.js` copies are now one shared `createActivityProcessor(...)` factory in the new `services/activityMarkupPipeline.service.js`; each endpoint just calls it with its local closure variables. Verified via source diffing (zero differences) plus an end-to-end old-vs-new response-equality test against a fixture exercising category inventory, guide pricing, addon markup, and transport conversion — responses matched exactly. `activityFrontController.js` shrank from 3858 to 2089 lines. Zero behavior change; no pricing-comparison rollout was needed for this part specifically because the inputs were provably identical, not because the usual step 4/5 rollout process below was skipped on judgment. Not done, and not currently planned unless requested: `webController.js`'s D/E/F are deliberately left as their own simpler pipeline (per the "intentional, different consumer" answer) — steps 1–5 below (the reconciliation-across-different-behavior work, and the inner-loop parallelization) still apply to that boundary if someone later decides D/E/F should be unified with A/B/C's shared function; right now the working assumption is they should not be. The original 5-copies analysis below is kept for reference but the current source of truth is `MARKUP_PIPELINE_DEDUP_FINDINGS.md`. **Root cause**: the ~600-line per-activity markup+currency block (`processActivity` and its helpers) is copy-pasted near-identically in `activityFrontController.getAllActivities`, `activityFrontController.getlActivityById`, `webController.getAllActivities`, `webController.getMealActivities`, and `webController.getlActivityById`. Within each copy, category/addon/guide markup calls are `await`ed one at a time in a loop instead of batched. **Why this is its own project, not a quick fix**: I read all 5 copies and they are *not* byte-identical — there are small divergences (e.g. which fields get `org_` prefixes, slightly different fallback behavior when a markup call fails). Blindly extracting "the" shared function risks silently changing behavior in whichever endpoint's quirk gets discarded. This needs a deliberate reconciliation step before any refactor: 1. **Diff the 5 copies line by line** and produce a table of every place they differ, with a decision for each (which behavior is correct / intentional vs. accidental drift). 2. **Write the reconciled version as a new shared function** in `services/activityPricing.service.js` (which already exists, unused, as a partial attempt at exactly this — worth checking whether its logic matches what you decide in step 1, since it may already encode one specific endpoint's behavior rather than a true reconciliation). 3. **Parallelize the inner loop** — instead of: ```js for (const cat of act.categories) { cat.adult_price = await applyMarkupToPriceWithCountry(cat.adult_price, act.country); cat.child_price = await applyMarkupToPriceWithCountry(cat.child_price, act.country); cat.infant_price = await applyMarkupToPriceWithCountry(cat.infant_price, act.country); } ``` do: ```js await Promise.all(act.categories.map(async (cat) => { [cat.adult_price, cat.child_price, cat.infant_price] = await Promise.all([ applyMarkupToPriceWithCountry(cat.adult_price, act.country), applyMarkupToPriceWithCountry(cat.child_price, act.country), applyMarkupToPriceWithCountry(cat.infant_price, act.country) ]); })); ``` (This matters far less once §6's Redis cache is in place — most of these calls become cache hits — so doing §6 first reduces the urgency and the risk of this item.) 4. **Swap each of the 5 call sites over one at a time**, not all at once — deploy, compare a sample of real responses (same request, before/after) for price differences, and only move to the next call site once one is confirmed clean. 5. **Regression-test pricing specifically**: for a handful of real activities across different countries/currencies/markup-group setups, capture the exact prices returned before the change and assert they match after (existing behavior preserved) except where step 1 intentionally corrected a divergence — and get sign-off on those specific intentional changes from whoever owns pricing rules, since this affects money charged to customers. **Risk**: medium-high — not because the code is hard to write, but because pricing correctness is high-stakes and the existing divergences between copies make "just merge them" genuinely ambiguous without a human decision on which behavior is right. Budget real review time here, not just dev time. **Files affected**: `controllers/activityFrontController.js`, `controllers/webController.js`, `services/activityPricing.service.js`, `services/activityInventory.service.js` (fix its broken `require("../config/db")` → `require("../database/connection")` as part of this if you end up using it). --- ## 8. Index `activity`, `sub_activity`, and reconsider `city`/`country` storage **Fix (indexes now, low risk)**: ```sql ALTER TABLE activity ADD INDEX idx_activity_status (status), ADD INDEX idx_activity_status_score (status, score), -- covers "WHERE status=... ORDER BY score DESC" ADD INDEX idx_activity_country (country), ADD INDEX idx_activity_city (city); ALTER TABLE sub_activity ADD INDEX idx_subactivity_activity_id (activity_id); ``` **Reconsider later, not now**: normalizing `city`/`country` off comma-separated strings into a join table (`activity_city`, `activity_country`) is a real schema migration — new tables, a data-migration script to backfill from the existing comma-separated values, and updating every `LIKE`/`FIND_IN_SET` query to a `JOIN`. Only worth it if `EXPLAIN`/slow-query-log actually shows this as a measurable cost in production — the indexes above already fix the *exact-match* filter case (`activityFrontController` uses `country = ?` and `city IN (...)`, which the new indexes will help); it's specifically the admin keyword search's `LIKE '%...%'` that can't be fixed by indexing at all. If that search is genuinely slow in practice, the more surgical fix is adding MySQL full-text indexes on `activity.name`/`description`/`highlights` and switching that one query to `MATCH ... AGAINST`, which is much less work than normalizing city/country. **Risk**: low for the indexes, medium+ for the normalization (only pursue if measured, not guessed). **Files affected**: none for the index migration (DB only); `controllers/activityController.js` if/when the full-text search change happens. --- ## Suggested rollout order | Phase | Items | Why this order | |---|---|---| | 1 | §1, §2, §3, §8 (indexes only) | Cheapest, lowest-risk, biggest immediate payoff. Can ship this week. | | 2 | §5 (Option A), §4 | Contained, mechanical, moderate testing needs. | | 3 | §6 | New infrastructure dependency — provision Redis, wire it in behind existing function signatures. | | 4 | §7 | Do only after §6 is live (reduces how much this one matters) and only with dedicated review time for the pricing reconciliation step. | ## One more thing worth fixing while you're in `create_booking` (not in the original bottleneck list, but adjacent) — ⏳ STILL OPEN, not fixed as part of §2 This was deliberately left alone during the §2 fix (it's a business-rule decision, not a pure connection-handling fix) and remains true today: booking commits even if the wallet-deduction call ultimately fails. While implementing §2, note that today a booking **commits to the database even if the wallet-deduction call to the external FB Admin API ultimately fails** (after all 3 retries) — there's no compensating rollback of the `fd_activities_booking` row, and the caller still gets a success response either way since the wallet call's result isn't checked before responding. That's a data-consistency gap, not strictly a performance issue, but it's sitting right next to the code you'll already be touching for §2 — worth a conscious decision (fail the booking, flag it for manual reconciliation, or accept the current behavior) rather than leaving it as an accident of the current code structure. Flagging it here rather than silently fixing it, since changing booking-failure behavior is a business-rule decision, not a pure performance fix.