perf(collab): три фикса горячего пути — connect-vs-unload гонка, двойная перекодировка, isDeepStrictEqual #433
Reference in New Issue
Block a user
Delete Branch "perf/401-collab-hotpath"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Три независимых серверных фикса горячего пути collab (побочные находки инцидента #400). #400 снижает частоту попадания, но не устраняет сами дефекты (человеческие правки большой страницы бьют по тем же местам).
Гонка connect-vs-unload в
@hocuspocus/server3.4.4 (вероятный источник 25с connect-таймаутов):createDocumentпроверяетloadingDocuments/documents, но НЕ ждётunloadingDocuments→ новое соединение может захендшейкаться на умирающийDocument, redis-sync идёт по пути «doc не загружен», провайдер висит до таймаута. Апстрим (main) не починен (проверил — толькоdocumentName.trim()guard добавлен). pnpm-патч (инфра как у yjs-патча): в началеcreateDocumentawaitin-flight unload, обёрнутый в try/catch (отклонённый unload не отравляет открытие — поведение как до патча), в ОБОИХ рантаймах (.cjs+.esm.js). Тестhocuspocus-unload-race: реальныйcreateDocumentс засеянным in-flight unload → не грузит пока unload не осел; при откате патча тест краснеет (мутационно).Двойная перекодировка в
onLoadDocument: хук строил НОВЫЙY.Docи возвращал его → hocuspocus делалapplyUpdate(encodeStateAsUpdate(...))ВТОРОЙ раз (315КБ на каждую холодную загрузку); в JSON-ветке результат encode выбрасывался (мёртвый вызов). Теперь стейт применяется прямо вdata.document, возвратundefined(hocuspocus мержит только при возврате Doc — подтверждено чтением distloadDocument); мёртвый encode убран. Содержимое документа не меняется — только меньше encode/alloc.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→ чисто.@hocuspocus/server(без посторонней churn), проводка зеркалит существующий yjs-патч.data.documentи возвращают undefined; содержимое == develop (внутреннее ревью подтвердило по dist).Внутренний цикл: coder → внутреннее ревреview (патч в обоих рантаймах + семантика onLoadDocument + Fix3-решение) → APPROVE WITH SUGGESTIONS → применил обе (try/catch на await unload — чтобы отклонённый unload не отравлял открытие; чистка encode-нита в тесте).
Checklist
Ревью — #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).@hocuspocus/server@3.4.4(bump уронит патч), unload-race спека ловит потерю патча.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).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 нет.