perf(collab): три фикса горячего пути — connect-vs-unload гонка, двойная перекодировка, isDeepStrictEqual #433

Merged
vvzvlad merged 1 commits from perf/401-collab-hotpath into develop 2026-07-10 04:25:49 +03:00
Collaborator

Summary

Три независимых серверных фикса горячего пути collab (побочные находки инцидента #400). #400 снижает частоту попадания, но не устраняет сами дефекты (человеческие правки большой страницы бьют по тем же местам).

  1. Гонка connect-vs-unload в @hocuspocus/server 3.4.4 (вероятный источник 25с connect-таймаутов): createDocument проверяет loadingDocuments/documents, но НЕ ждёт unloadingDocuments → новое соединение может захендшейкаться на умирающий Document, redis-sync идёт по пути «doc не загружен», провайдер висит до таймаута. Апстрим (main) не починен (проверил — только documentName.trim() guard добавлен). pnpm-патч (инфра как у yjs-патча): в начале createDocument await in-flight unload, обёрнутый в try/catch (отклонённый unload не отравляет открытие — поведение как до патча), в ОБОИХ рантаймах (.cjs+.esm.js). Тест hocuspocus-unload-race: реальный createDocument с засеянным in-flight unload → не грузит пока unload не осел; при откате патча тест краснеет (мутационно).

  2. Двойная перекодировка в onLoadDocument: хук строил НОВЫЙ Y.Doc и возвращал его → hocuspocus делал applyUpdate(encodeStateAsUpdate(...)) ВТОРОЙ раз (315КБ на каждую холодную загрузку); в JSON-ветке результат encode выбрасывался (мёртвый вызов). Теперь стейт применяется прямо в data.document, возврат undefined (hocuspocus мержит только при возврате Doc — подтверждено чтением dist loadDocument); мёртвый encode убран. Содержимое документа не меняется — только меньше encode/alloc.

  3. isDeepStrictEqual по 84КБ JSON на каждом store: замерил — 1.32мс на 90КБ (318КБ → 4.68мс), обе цифры далеко ниже ~50мс порога материальности → по правилу задачи изменений кода НЕТ (доминируют fromYdoc+encodeStateAsUpdate, как и предполагалось; dirty-флаг только при material).

closes #401

How verified

  • pnpm --filter ./apps/server exec jest --maxWorkers=2 hocuspocus-unload-race persistence-load persistence-store collaboration.handler28 passed (race-тест проходит, happy-path не изменён).
  • pnpm --filter ./apps/server exec tsc --noEmit → чисто.
  • Патч применён к обоим рантаймам (сверено в материализованном dist); lockfile-дифф — только строки patch_hash @hocuspocus/server (без посторонней churn), проводка зеркалит существующий yjs-патч.
  • Fix 2: все три ветки мутируют data.document и возвращают undefined; содержимое == develop (внутреннее ревью подтвердило по dist).

Внутренний цикл: coder → внутреннее ревреview (патч в обоих рантаймах + семантика onLoadDocument + Fix3-решение) → APPROVE WITH SUGGESTIONS → применил обе (try/catch на await unload — чтобы отклонённый unload не отравлял открытие; чистка encode-нита в тесте).

Checklist

  • патч в patches/, lockfile обновлён; race-тест зелёный
  • onLoadDocument — один encode+apply вместо двух, содержимое не изменено
  • Fix 3 — замер в описании, кода не меняли (immaterial)
## Summary Три независимых серверных фикса горячего пути collab (побочные находки инцидента #400). #400 снижает частоту попадания, но не устраняет сами дефекты (человеческие правки большой страницы бьют по тем же местам). 1. **Гонка connect-vs-unload в `@hocuspocus/server` 3.4.4** (вероятный источник 25с connect-таймаутов): `createDocument` проверяет `loadingDocuments`/`documents`, но НЕ ждёт `unloadingDocuments` → новое соединение может захендшейкаться на умирающий `Document`, redis-sync идёт по пути «doc не загружен», провайдер висит до таймаута. **Апстрим (`main`) не починен** (проверил — только `documentName.trim()` guard добавлен). pnpm-патч (инфра как у yjs-патча): в начале `createDocument` `await` in-flight unload, обёрнутый в try/catch (отклонённый unload не отравляет открытие — поведение как до патча), в **ОБОИХ** рантаймах (`.cjs`+`.esm.js`). Тест `hocuspocus-unload-race`: реальный `createDocument` с засеянным in-flight unload → не грузит пока unload не осел; при откате патча тест краснеет (мутационно). 2. **Двойная перекодировка в `onLoadDocument`**: хук строил НОВЫЙ `Y.Doc` и возвращал его → hocuspocus делал `applyUpdate(encodeStateAsUpdate(...))` ВТОРОЙ раз (315КБ на каждую холодную загрузку); в JSON-ветке результат encode выбрасывался (мёртвый вызов). Теперь стейт применяется прямо в `data.document`, возврат `undefined` (hocuspocus мержит только при возврате Doc — подтверждено чтением dist `loadDocument`); мёртвый encode убран. **Содержимое документа не меняется** — только меньше encode/alloc. 3. **`isDeepStrictEqual` по 84КБ JSON на каждом store**: замерил — **1.32мс на 90КБ** (318КБ → 4.68мс), обе цифры далеко ниже ~50мс порога материальности → по правилу задачи **изменений кода НЕТ** (доминируют `fromYdoc`+`encodeStateAsUpdate`, как и предполагалось; dirty-флаг только при material). closes #401 ## How verified - `pnpm --filter ./apps/server exec jest --maxWorkers=2 hocuspocus-unload-race persistence-load persistence-store collaboration.handler` → **28 passed** (race-тест проходит, happy-path не изменён). - `pnpm --filter ./apps/server exec tsc --noEmit` → чисто. - Патч применён к обоим рантаймам (сверено в материализованном dist); lockfile-дифф — только строки patch_hash `@hocuspocus/server` (без посторонней churn), проводка зеркалит существующий yjs-патч. - Fix 2: все три ветки мутируют `data.document` и возвращают undefined; содержимое == develop (внутреннее ревью подтвердило по dist). Внутренний цикл: coder → внутреннее ревреview (патч в обоих рантаймах + семантика onLoadDocument + Fix3-решение) → APPROVE WITH SUGGESTIONS → применил обе (try/catch на await unload — чтобы отклонённый unload не отравлял открытие; чистка encode-нита в тесте). ## Checklist - [x] патч в patches/, lockfile обновлён; race-тест зелёный - [x] onLoadDocument — один encode+apply вместо двух, содержимое не изменено - [x] Fix 3 — замер в описании, кода не меняли (immaterial)
agent_coder added 1 commit 2026-07-10 02:54:01 +03:00
Побочные находки инцидента #400 (правка большой таблицы через MCP подвешивает всё).

1. Гонка connect-vs-unload в @hocuspocus/server 3.4.4 (вероятный источник 25s
   connect-таймаутов): createDocument проверяет loadingDocuments/documents, но НЕ
   ждёт unloadingDocuments -> новое соединение может захендшейкаться на умирающий
   Document -> redis-sync идёт по пути 'doc не загружен', провайдер висит до
   таймаута. Апстрим (main) не починен. pnpm-патч (инфра как у yjs-патча): в начале
   createDocument await in-flight unload (обёрнут в try/catch — отклонённый unload
   не отравляет открытие, поведение как до патча), в ОБОИХ рантаймах (cjs+esm).
   Тест hocuspocus-unload-race: реальный createDocument с засеянным in-flight
   unload -> не грузит пока unload не осел; при откате патча тест краснеет.

2. Двойная перекодировка в onLoadDocument (persistence.extension.ts): хук строил
   НОВЫЙ Y.Doc и возвращал его -> hocuspocus делал applyUpdate(encodeStateAsUpdate)
   ВТОРОЙ раз (315КБ на каждую холодную загрузку); в JSON-ветке результат encode
   выбрасывался (мёртвый вызов). Теперь стейт применяется прямо в data.document,
   возврат undefined (hocuspocus мержит только при возврате Doc); мёртвый encode
   убран. Содержимое документа не меняется — только меньше encode/alloc.

3. isDeepStrictEqual по 84КБ JSON на каждом store: замерил — 1.32мс на 90КБ
   (immaterial, <50мс порога; доминируют fromYdoc+encodeStateAsUpdate). Изменений
   кода НЕТ по правилу задачи (dirty-флаг только при material).

closes #401

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
agent_coder added the review/needs label 2026-07-10 02:54:03 +03:00
Collaborator

Ревью — #433 perf(collab): три фикса горячего пути сервера — connect-vs-unload гонка, двойная перекодировка, isDeepStrictEqual (замер)

Вердикт: PASS

Полный веер из 9 аспектов по всему диффу PR (merge-base d90c3b8b..96850742). Гейт зелёный (clean-room /home/claude/gm-ci) с проверкой патч-фиделити: pnpm install применил новый патч (exit 0), и текст PATCH(gitmost #401) РЕАЛЬНО присутствует в установленном node_modules/@hocuspocus/server/dist/*.{cjs,esm.js} — т.е. патч применяется на чистой установке, а не только лежит файлом. server tsc 0; обе новые спеки 7/7 против ПАТЧЕНОГО пакета.

Дифф содержит ДВА кодовых фикса + один замер:

Fix 1 — connect-vs-unload гонка (патч @hocuspocus/server) — проверено (клик)

Патч в createDocument: перед проверками loadingDocuments/documents теперь await-ит любой in-flight unload (unloadingDocuments.get(name)), чтобы новое соединение не сделало хендшейк на умирающий Document; отклонённый unload глотается (fresh load). Патчит обе сборки (cjs+esm).

  • Аудит патча (supply-chain): только этот 20-строчный guard, никаких изменений auth/сети/секретов; версия пиннится @hocuspocus/server@3.4.4 (bump уронит патч), unload-race спека ловит потерю патча.
  • Риск deadlock — проверил по исходнику hocuspocus: unloadDocument (dist:2606-2627) гарантирует, что unloading сеттлится — beforeUnloadDocument в try/catch (реджект → return), afterUnloadDocument реджект → unloading реджектится, delete после await. Вечно висеть может только если СОБСТВЕННЫЙ unload-хук gitmost зависнет навсегда (не реджект) — это pre-existing свойство store'а. Предложенный timeout-митигейт НЕ добавляю: при срабатывании он бы пропустил соединение мимо ещё-идущего unload → вернул бы ровно ту гонку, что патч закрывает.
  • Мутация: убрать await existingUnloadingDoc → unload-race спека падает (loadSpy зовётся до settle). Ловится.
Fix 2 — двойная перекодировка (persistence.extension.ts) — проверено против реального hocuspocus (клик)

onLoadDocument теперь применяет DB-стейт ПРЯМО в hook-документ (Y.applyUpdate(document, dbState)) и возвращает undefined, вместо new Y.Doc()+apply+return doc (что заставляло hocuspocus ре-encode+apply весь стейт ВТОРОЙ раз). Все 3 ветки (db ydoc / json-content / fresh).

  • Контракт hocuspocus подтверждён напрямую (dist:2498-2506): колбэк ре-энкодит ТОЛЬКО if (loadedDocument?.constructor.name === "Document"||"Doc"). Возврат undefined → второй encode/apply НЕ делается, hocuspocus держит hookPayload.document (=== тот самый инстанс, что мутирует расширение) как есть. Потери данных нет, document — тот же инстанс (mutation-identity ок).
  • Мутация: вернуть return document/return new Y.Doc() → все 4 persistence-load теста падают на expect(result).toBeUndefined(). Ловится. Тесты проверяют observable-контракт (undefined) + что контент применён — верная стратегия (не хрупкий счётчик encode).

Fix 3 — isDeepStrictEqual (замер): НЕ кодовое изменение. isDeepStrictEqual уже был в persistence.extension.ts:305,387 (store-skip / history-guard) до PR; третий пункт — замер этого существующего поведения. Диффа по нему нет, «(замер)» в заголовке это и отмечает. Корректно.

DROP — кодеру НЕ делать (лог калибровки)
  • [below-threshold] low/med [test-coverage] прогон новых collab-спек даёт jest-варнинг «worker failed to exit gracefully / tests leaking / ensure .unref()» (+ jsdom [Document] soft-delete) — тесты проходят (7/7) и мутационно держат фиксы; force-exit это частый артефакт collab/jsdom-тестов. Гигиенически стоит добавить teardown/.unref в новых спеках, но отдельного раунда не требует. (Учитывая твоё внимание к open-handle-аудиту — помечаю явно.)

DO/ESCALATE нет. Оба кодовых фикса корректны и мутационно-подтверждены; патч аудирован (без бэкдора), версионно-пиннут и проверен на чистой установке; контракт hocuspocus для fix 2 сверен с исходником пакета. Approve.

9 аспектов (security · stability · regressions · test-coverage · conventions · documentation · simplification · architecture · coherence). Обе «непроверенные допущения» от stability (контракт onLoadDocument, hang-риск unload) разрешены мной прямой сверкой с исходником @hocuspocus/server. Патч-фиделити проверен на чистом install. Escalate нет.

<!-- state:review reviewed_head=96850742 round=1 verdict=approved --> ## Ревью — #433 `perf(collab): три фикса горячего пути сервера — connect-vs-unload гонка, двойная перекодировка, isDeepStrictEqual (замер)` **Вердикт: PASS** ✅ Полный веер из 9 аспектов по всему диффу PR (merge-base `d90c3b8b`..`96850742`). Гейт зелёный (clean-room `/home/claude/gm-ci`) с **проверкой патч-фиделити**: `pnpm install` применил новый патч (exit 0), и текст `PATCH(gitmost #401)` РЕАЛЬНО присутствует в установленном `node_modules/@hocuspocus/server/dist/*.{cjs,esm.js}` — т.е. патч применяется на чистой установке, а не только лежит файлом. server tsc 0; обе новые спеки **7/7** против ПАТЧЕНОГО пакета. Дифф содержит ДВА кодовых фикса + один замер: <details> <summary>Fix 1 — connect-vs-unload гонка (патч @hocuspocus/server) — проверено (клик)</summary> Патч в `createDocument`: перед проверками `loadingDocuments`/`documents` теперь `await`-ит любой in-flight unload (`unloadingDocuments.get(name)`), чтобы новое соединение не сделало хендшейк на умирающий Document; отклонённый unload глотается (fresh load). Патчит обе сборки (cjs+esm). - **Аудит патча (supply-chain)**: только этот 20-строчный guard, никаких изменений auth/сети/секретов; версия пиннится `@hocuspocus/server@3.4.4` (bump уронит патч), unload-race спека ловит потерю патча. - **Риск deadlock — проверил по исходнику hocuspocus**: `unloadDocument` (dist:2606-2627) гарантирует, что `unloading` сеттлится — `beforeUnloadDocument` в try/catch (реджект → return), `afterUnloadDocument` реджект → `unloading` реджектится, delete после await. Вечно висеть может только если СОБСТВЕННЫЙ unload-хук gitmost зависнет навсегда (не реджект) — это pre-existing свойство store'а. Предложенный timeout-митигейт НЕ добавляю: при срабатывании он бы пропустил соединение мимо ещё-идущего unload → вернул бы ровно ту гонку, что патч закрывает. - **Мутация**: убрать `await existingUnloadingDoc` → unload-race спека падает (loadSpy зовётся до settle). Ловится. </details> <details> <summary>Fix 2 — двойная перекодировка (persistence.extension.ts) — проверено против реального hocuspocus (клик)</summary> `onLoadDocument` теперь применяет DB-стейт ПРЯМО в hook-документ (`Y.applyUpdate(document, dbState)`) и возвращает `undefined`, вместо `new Y.Doc()`+apply+`return doc` (что заставляло hocuspocus ре-encode+apply весь стейт ВТОРОЙ раз). Все 3 ветки (db ydoc / json-content / fresh). - **Контракт hocuspocus подтверждён напрямую** (`dist:2498-2506`): колбэк ре-энкодит ТОЛЬКО `if (loadedDocument?.constructor.name === "Document"||"Doc")`. Возврат `undefined` → второй encode/apply НЕ делается, hocuspocus держит `hookPayload.document` (=== тот самый инстанс, что мутирует расширение) как есть. Потери данных нет, `document` — тот же инстанс (mutation-identity ок). - **Мутация**: вернуть `return document`/`return new Y.Doc()` → все 4 persistence-load теста падают на `expect(result).toBeUndefined()`. Ловится. Тесты проверяют observable-контракт (undefined) + что контент применён — верная стратегия (не хрупкий счётчик encode). </details> **Fix 3 — isDeepStrictEqual (замер)**: НЕ кодовое изменение. `isDeepStrictEqual` уже был в `persistence.extension.ts:305,387` (store-skip / history-guard) до PR; третий пункт — замер этого существующего поведения. Диффа по нему нет, «(замер)» в заголовке это и отмечает. Корректно. <details> <summary>⛔ DROP — кодеру НЕ делать (лог калибровки)</summary> - `[below-threshold]` `low/med` **[test-coverage]** прогон новых collab-спек даёт jest-варнинг «worker failed to exit gracefully / tests leaking / ensure .unref()» (+ jsdom `[Document]` soft-delete) — тесты проходят (7/7) и мутационно держат фиксы; force-exit это частый артефакт collab/jsdom-тестов. Гигиенически стоит добавить teardown/.unref в новых спеках, но отдельного раунда не требует. (Учитывая твоё внимание к open-handle-аудиту — помечаю явно.) </details> DO/ESCALATE нет. Оба кодовых фикса корректны и мутационно-подтверждены; патч аудирован (без бэкдора), версионно-пиннут и проверен на чистой установке; контракт hocuspocus для fix 2 сверен с исходником пакета. Approve. <sub>9 аспектов (security · stability · regressions · test-coverage · conventions · documentation · simplification · architecture · coherence). Обе «непроверенные допущения» от stability (контракт onLoadDocument, hang-риск unload) разрешены мной прямой сверкой с исходником @hocuspocus/server. Патч-фиделити проверен на чистом install. Escalate нет.</sub>
agent_reviewer added review/approved and removed review/needs labels 2026-07-10 03:13:09 +03:00
vvzvlad merged commit f2ad0121a5 into develop 2026-07-10 04:25:49 +03:00
Sign in to join this conversation.