perf(classroom): GET /classrooms の DynamoDB 読み取り削減とページング対応 (#1146) - #1148
Conversation
GET /classrooms は #1138 で Classrooms のフルスキャンが 2 回になっていた。課題単位の 共同管理 (contains(coTeacherEmails)) とクラス経由の課題 (groupId IN (...)) を 1 つの Scan に統合する。RCU はフィルタ適用前に読んだ項目に課金されるため Scan 回数がそのまま 課金に効く。IN のオペランド上限 100 のためクラスが 100 を超える場合はチャンク分割し、 email 述語は先頭チャンクにだけ載せて二重評価を避ける。 あわせて Scan/Query のページング未対応を解消する。DynamoDB の 1MB 上限はフィルタ適用 前に効くため、LastEvaluatedKey を辿らないとテーブル肥大時にエラー無しで取りこぼす。 queryAll / scanAll ヘルパーを追加し一覧系の読み取りを寄せた。無限ループ防止に DDB_MAX_PAGES (既定 25) で打ち切り、打ち切り時は warn を残す。 Closes #1146 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🤖 autopilot status
Linked issue #1146. Maintained by autopilot (single writer); do not edit. |
レビュー指摘の反映 (#1146)。 - 列挙結果がそのまま書き込みを駆動する経路が未ページングのまま残っていた。 1MB 上限はフィルタ適用前に効くので、取りこぼすと「一部だけ更新されない」 状態になり、読み取りの取りこぼしより気付きにくい: - handleMigrateGroups (組への移行プランが不完全になる) - propagateStudentCountToClassrooms (一部の課題が古い人数のまま残る) - 課題名の重複連番 (既存名を見落として重複を配ってしまう) - 前回コメントの recap (best-effort だが同じ列挙パターン) - DDB_MAX_PAGES は Lambda の environment 許可リストに無く、どのステージでも 設定できないのに docs では knob として書かれていた。stack に追加して実態を 合わせる。あわせて不正値 (NaN / 0 以下) を既定へフォールバックする — そのまま使うと 1 ページも読まずに空配列を返し、このヘルパーが無くそうと している「黙って取りこぼす」挙動そのものになる。 - テスト: 不正値フォールバック、人数伝播のページング (どちらも RED を確認)。 「email を持たない教師」テストは dev bypass の identity が常に email を持つ ため実態と乖離していたので、実際の性質 (2 述語が 1 Scan に OR 結合される) を assert する形に直す。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…isions import / export を持たないテストファイルは TS の「スクリプト」扱いになり、 ts-jest が 1 プロセスで複数のテストを型付けすると `const mockSend` などが グローバルスコープで衝突して "Cannot redeclare block-scoped variable" になる。 キャッシュが温まっていると通ってしまうため気付きにくいが、`npx jest --clearCache` 後は 7 suites が「Test suite failed to run」で丸ごと落ちる (#1146 で追加したテストも含む = DoD の根拠が実際には走っていなかった)。 smalruby-classroom は ci-infra.yml にジョブが無く CI でも検知されない。 各ファイル末尾に空 export を足してモジュール化し、ファイルごとのスコープに 閉じる。cold cache でも 16 suites / 254 tests green。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
[Must] 本 PR が追加したテスト自体が、クリーンな ts-jest キャッシュでは suite ごと実行されない(修正済み: 0286eca)
原因は 影響が「Must」なのは、この PR の DoD の根拠(読み取り回数の固定・ページングの固定)が実際には走っていない 対応: 対象 8 ファイルの末尾に空 |
|
[Must] 「一覧系の呼び出しをページングヘルパーに寄せる」が未完で、列挙結果が書き込みを駆動する経路が取りこぼす(修正済み: ecb3223) Issue #1146 は「Scan / Query を
対応: 上記 4 箇所を |
|
[Question]
さらに 対応: stack の |
|
[Question] テスト「統合後も email を持たない教師では Scan がクラス経由分だけになる」が実態と乖離していた(修正済み: ecb3223) このテストが使う dev bypass の identity は 対応: テスト名と assert を実際の #1146 の性質(2 つの述語が 1 本の |
|
[Question] ページ上限 25 × 1MB は Lambda の memorySize 256MB に対して強気(人間の判断に委ねる。未対応)
つまりトレードオフを「黙って取りこぼす」から「たまに 500 を返す」に付け替えている面がある(検知はできるので
数値の決め(何件を上限とみなすか / どこまでコストをかけるか)は運用判断なので、こちらでは変更せず記載のみ。 |
|
[Question]
そのため、上の [Must] の cold-cache 破綻(テストが丸ごと走らない)も、 本 PR で対応しない理由: |
|
[FYI] 細かい点(いずれも動作に影響しないため未対応)
|
|
🤖 敵対的レビュー完了。指摘 7 件(Must 2 / Question 4 / FYI 1)。Must は修正済み。
検証: 人間の確認事項(PR 本文のとおり未実施): stg へのデプロイと課題一覧の表示確認。prod へのデプロイは行わない。 |
takaokouji
left a comment
There was a problem hiding this comment.
conflict を解消して、最新のmainに追従して。
Question | ページ上限 25 × 1MB × 4 本 vs memorySize: 256 の余裕 | 記載のみ(設計判断)
逆引きindexを導入して。
Question | smalruby-classroom だけ CI ジョブが無い | 記載のみ(.github/workflows/ は bot 権限外)
対象外でOK
FYI | 型キャスト / 1 行ラッパ / センチネルの読みやすさ | 対応せず
ここで修正しないと今後も修正しないので、すべて対応して。
# Conflicts: # infra/smalruby-classroom/lambda/handler.ts # infra/smalruby-classroom/lambda/tests/handler-archive-recovery.test.ts # infra/smalruby-classroom/lambda/tests/handler-co-teacher-authz.test.ts # infra/smalruby-classroom/lambda/tests/handler-group-co-teacher.test.ts # infra/smalruby-classroom/lambda/tests/handler-group-seatcount.test.ts # infra/smalruby-classroom/lambda/tests/handler-notifications.test.ts # infra/smalruby-classroom/lambda/tests/handler-shared-assignments.test.ts # infra/smalruby-classroom/lambda/tests/handler-shared-recommend-projection.test.ts
|
🤖 base 追従のコンフリクトを解決しました( develop の #1145(クラス単位の列挙を 解決方針
マージに伴うテスト更新
確認
人間の判断が要る論点は出なかったのでそのまま進めた(どちらを採るかは「両立できる形が 1 つだけある」ケースだった)。 |
レビュー指摘(#1148)への対応の第一段。coTeacherEmails は list 属性で GSI に できないため、email → 資源 の逆引き行を持つ ClassroomCoTeacherIndex テーブルを 追加し、共同管理者リストを書き換えるたびに差分同期する。あわせて課題を組で 引くための groupId-index GSI を追加した(読み取りの切り替えは次段)。 paginateAll の ScanCommand → QueryCommand キャストは型の嘘だったので、 instanceof で分岐して本来の出力型を得る形に直した。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🤖 レビューへの対応(途中・チェックポイント) いただいた 4 点のうち 1 点は対応済み、逆引き index は土台まで進めて時間切れの区切りで一旦止めました(残りは
今回入れたもの(
|
|
🚀 Preview deployed: https://smalruby.jp/smalruby3-editor/topic/autopilot-1146/ |
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🤖 レビューへの対応(完了) 前回のチェックポイントで残していた作業を終えました。いただいた 4 点はすべて処理済みです。
読み取りを索引に切り替えた(
|
| FYI | どうなったか |
|---|---|
as QueryCommand の型の嘘 |
instanceof ScanCommand で分岐して本来の出力型を得る形に(ご提案の Parameters<typeof docClient.send>[0] は戻り値が ServiceOutputTypes に潰れて Items / LastEvaluatedKey が引けなくなったため別の形にしました) |
listAssignmentsInGroups が 1 行ラッパ |
groupId-index を引く実体になったのでラッパではなくなりました |
chunks のセンチネル |
listSharedAssignments 自体を削除したので消滅 |
| 述語ごとに計測できない | 索引 Query と groupId-index Query が別コマンドになったので CloudWatch で分けて見えます |
あわせて、「ページ上限 25 × 1MB × 4 本 vs memorySize: 256」の Question も前提ごと解消しました(フィルタ前に読む量がテーブルサイズに比例しなくなったため)。
書き込み側の取りこぼしも埋めました
coTeacherEmails の書き手は 4 経路あり、v2 移行(POST /classroom-groups/migrate)が課題の共同管理者を組へ引き上げる分が同期から漏れていました。索引に書かない経路が残ると その共同管理者にだけ資源が一覧に出ない(権限は落ちないので直リンクでは操作できる)という気付きにくい壊れ方になるため、4 経路すべてを回帰テストで固定しています。
バックフィルが必要です(デプロイ順に注意)
索引は後付けなので、それ以前に登録された共同管理者の行がありません。bin/backfill-coteacher-index.ts(既定 dry-run・冪等)を用意しました。
1. cdk deploy(テーブル + GSI + Lambda)
2. 直後に npx ts-node bin/backfill-coteacher-index.ts --apply
3. 課題一覧を確認
この順序を守る必要があるため、こちらからは deploy-stg ラベルを付けていません(付けるとバックフィル前の状態で stg が動く時間ができます)。手順は docs/classroom/operations.md に runbook として書きました。
確認
jest --clearCache && npx jest: 21 suites / 314 tests すべて green(cold cache)npx tsc --noEmit: エラーなし- テストは Scan 前提のモックを索引・GSI に追随させ、co-teacher のモックは一覧経路で Scan が来たら throw するようにしています(退行が green にならないように)
なお逆引き index は元 Issue では「スコープ外」としていた項目ですが、ご指示に従って本 PR に含めています。PR 本文にもその旨を残しました。
|
🚀 stg にデプロイしました:
stg は共有環境です。この SHA が載っている前提で 動作確認してください。 |
一覧経路の Scan を逆引き索引と GSI の Query に置き換えたことで、handler の scanAll は呼び出し元がゼロになっていた。残しておくと「ページングしてくれる Scan」という体裁で表全体を読む経路が再び生えやすいので、paginateAll ごと Query 専用にする。ScanCommand を narrow するための instanceof 分岐も不要に なった(Query/Scan の union を送るためだけの分岐だったため)。 docs/classroom/architecture.md のページングの記述もヘルパー名に追随させる。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🤖 レビュー対応の再確認(新しいご指摘なし・追加修正 1 件) 差し戻しを受けて PR 全体(diff + 全コメント + レビュー)を読み直しましたが、前回の完了報告(07:04)以降に新しいご指摘は入っていませんでした。既存の指摘がすべて実際に反映されているかを、コードとテストで検証し直しています。 指摘の棚卸し(人間 4 件 / bot 自身 7 件 = 11 件)
実コードでの確認: 追加修正 1 件(
|
|
🚀 stg にデプロイしました:
stg は共有環境です。この SHA が載っている前提で 動作確認してください。 |
develop 由来(#1133)の `handler-classroom-groups.test.ts` が import/export を 持たないため、admin の `test-files-are-modules.test.ts`(#1144/#1155 の回帰テスト)が 落ち、infra/** を触るすべての PR で `[admin] Unit Tests` が赤になっていた。 本 PR は smalruby-admin を触っていないが、この PR の CI もそれで赤いままなので、 classroom 側で使っているのと同じ `export {};` の宣言を足して解いておく。 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
🤖 レビュー対応(CI の赤を 1 件解消・新しいご指摘なし) 差し戻しを受けて PR 全体(diff + 全コメント + レビュー + inline)を読み直しましたが、前回の再確認(10:52)以降に新しいご指摘は入っていません(inline コメントも 0 件)。既存の指摘 11 件(レビュー 4 / bot 自身 7)の対応状況は前回コメントの表のとおりで変化はありません。 かわりに、前回の完了後に回った CI で 新しい赤が出ていたので、それを解消しました。
|
|
🚀 stg にデプロイしました:
stg は共有環境です。この SHA が載っている前提で 動作確認してください。 |
|
🚀 stg にデプロイしました:
stg は共有環境です。この SHA が載っている前提で 動作確認してください。 |
stg で実際に流したところ、cdk deploy 直後の groupId-index は CREATING で、 項目 100 件程度でも ACTIVE まで約 5 分かかった。この間はその索引への Query が 失敗しうるので、待つ手順を明記する。 あわせて dry-run が索引テーブル無しでも動くこと(デプロイ前に件数を確認できる) とステージ切り替えの方法を追記する。 Refs #1146 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🤖 stg でデプロイ → バックフィル → 検証まで実施しました(人間と一緒に実行)。レビューの材料としてどうぞ。 実施内容と結果
バックフィルの意味を実データで再現しました「別の先生が所有し、dev アカウントを共同管理者にしたクラス」を stg に一時作成して確認: 検証用データは削除済み(索引も本来の 2 行に戻っています)。 実運用で見つかった手順の抜け(
|
|
🚀 stg にデプロイしました:
stg は共有環境です。この SHA が載っている前提で 動作確認してください。 |
|
🚀 stg にデプロイしました:
stg は共有環境です。この SHA が載っている前提で 動作確認してください。 |
Summary
GET /classroomsの DynamoDB 読み取りから Scan を全廃し、Scan / Query のページング未対応を解消する。Scan は RCU も 1MB 上限も フィルタ適用「前」 に効くため、コストと取りこぼしのリスクが「その先生に見える量」ではなく テーブル全体の量(= 他の先生のデータ量) に比例して増える。当初は Scan の統合(2 回 → 1 回)で足りるか見る方針だったが、レビューで逆引きインデックスの導入指示を受けたので、統合ではなく置き換えた。
ClassroomCoTeacherIndex{suffix}(PKcoTeacherEmail/ SKresourceKey)の Query +BatchGetItem。coTeacherEmailsはリスト属性で GSI のキーにできないため別テーブルになる。groupId-indexをクラスごとに Query(旧groupId IN (...)の Scan を置換)。INのオペランド 100 個上限と、それに伴うチャンク分割も不要になった。LastEvaluatedKeyのページング: 1MB 上限は Query でも効くので、辿らないと エラー無しで黙って取りこぼす。共通ヘルパーqueryAll/scanAll/batchGetAllに寄せた。Changes Made
infra/smalruby-classroom/lib/classroom-stack.tsClassroomCoTeacherIndex{suffix}テーブル(TTLttl・DESTROY)と Classrooms のgroupId-indexGSI を追加。envCO_TEACHER_INDEX_TABLE_NAMEとDDB_MAX_PAGESを配線。infra/smalruby-classroom/lambda/handler.tspaginateAll/queryAll/scanAll/batchGetAllを追加。DDB_MAX_PAGES(既定 25、不正値は既定へフォールバック)で無限ループを防ぎ、打ち切り時は CloudWatch にwarn。batchGetAllはUnprocessedKeysを再試行する(スロットリングはエラーではなく未処理キーの返却で表れるため、捨てると同じ「黙って取りこぼす」形になる)。syncCoTeacherIndex():coTeacherEmailsを書き換える 4 経路すべて(課題の共同管理者 追加 / 削除、組の更新、v2 移行での組への引き上げ)から差分だけ Put/Delete。listCoTeacherResourceIds/listAssignmentsCoTaughtBy/listCoManagedGroups/listAssignmentsInGroupsを索引・GSI ベースに。listSharedAssignments(OR 統合 Scan + センチネル + 1 行ラッパ)は削除。infra/smalruby-classroom/lambda/coteacher-index-lib.ts+bin/backfill-coteacher-index.ts— 既存データのバックフィル(既定 dry-run・冪等)。判断は純粋関数に置いてテストで固定し、CLI は I/O だけ。docs/classroom/architecture.md— 読み取り構成、逆引き索引の役割(認可は索引を見ない)、ページングの線引きを更新。docs/classroom/operations.md— バックフィルの runbook(デプロイ順を含む)。.claude/rules/infra/smalruby-classroom.md— テーブル一覧(8 → 9)と GSI を追記。1 リクエストあたりの読み取り(
GET /classrooms)teacherSub-index)groupId-index)Scan が消えたので、レビューで挙がった 「ページ上限 25 × 1MB × 4 本 vs
memorySize: 256」は前提ごと解消した(フィルタ前に読む量がテーブルサイズに比例しなくなった)。Test Coverage
lambda/tests/handler-list-classrooms-reads.test.ts(8 件)— Classrooms / ClassroomGroups への Scan が 0 回、逆引き索引とgroupId-indexで引くこと、各経路のページング、索引が消し忘れを返しても資源本体のリストで再確認して除外すること、BatchGet の 100 キー分割、壊れたページャで打ち切ること。lambda/tests/handler-coteacher-index-sync.test.ts(4 件・新規)—coTeacherEmailsを書き換える 4 経路すべてが索引を同期し、据え置きの email には書かない(差分のみ)こと。書かない経路が残ると、その共同管理者に資源が見えなくなる。lambda/tests/coteacher-index-lib.test.ts(10 件・新規)— バックフィルの行生成(正規化・重複・アーカイブ済みも対象・壊れた行は無視)と BatchWrite のチャンク分割。handler-group-co-teacher/handler-group-enumeration/handler-group-seatcount)を索引・GSI に追随。co-teacher のモックは一覧経路で Scan が来たら throw するようにして、退行が green にならないようにした。jest --clearCache && npx jest: 21 suites / 314 tests すべて green(cold cache)npx tsc --noEmit: エラーなし(infra/はルートの ESLint / Prettier 対象外)人間の確認事項
stg へのデプロイと課題一覧の表示確認。デプロイ順を守る必要があるので、こちらからは
deploy-stgを付けていない:cdk deploy(テーブル + GSI + Lambda)npx ts-node bin/backfill-coteacher-index.ts --apply(既存の共同管理者の索引行を作る)バックフィル前は共同管理の課題・クラスが一覧に出ない(権限は item 上の
coTeacherEmailsで判定しているので落ちない)。手順はdocs/classroom/operations.md。prod へのデプロイは行わない(人間の判断で別途。prod も同じ順序でバックフィルが必要)。
Related Issues
Closes #1146