From 4d462a883a37ac0504c41f8cfef2cdf2e9662f83 Mon Sep 17 00:00:00 2001 From: "makabe.t" Date: Mon, 29 Jan 2024 02:52:24 +0000 Subject: [PATCH] =?UTF-8?q?Merged=20PR=20688:=20=E8=A1=8C=E3=83=AD?= =?UTF-8?q?=E3=83=83=E3=82=AF=E6=A8=AA=E5=B1=95=E9=96=8B1?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 概要 [Task3468: 行ロック横展開1](https://paruru.nds-tyo.co.jp:8443/tfs/ReciproCollection/fa4924a4-d079-4fab-9fb5-a9a11eb205f0/_workitems/edit/3468) - 各リポジトリのメソッドについて行ロックが必要な箇所に処理を追加しました。 - **accounts** - **cancelIssue** - `canselIssue`と`allocateLicense`での対象ライセンス取得にロックを追加しました。 - **updateAccountInfo** - `updateAccountInfo`でのプライマリ/セカンダリ管理者取得にロックを追加しました。 - **templates** - **upsertTemplateFile** - `upsertTemplateFile`のテンプレートファイル取得にロックを追加しました。 - **users** - **update** - ユーザー取得にロックを追加しました。 - 影響としてはAuthorIDの重複が考えられたのでその対応のために入れています。 - **findDelegateUser** - selectのみでデータの不整合はないので特に処置はしていません。 - **isAllowDelegationPermission** - selectのみでデータの不整合はないので特に処置はしていません。 ※こちらの資料を参考に各メソッド内で影響に関連すると思われるselectにロックを追加しています。 [行ロックに関する影響調査.xlsx](https://ndstokyo.sharepoint.com/:x:/r/sites/Piranha/Shared%20Documents/General/OMDS/%E8%A1%8C%E3%83%AD%E3%83%83%E3%82%AF%E3%81%AB%E9%96%A2%E3%81%99%E3%82%8B%E5%BD%B1%E9%9F%BF%E8%AA%BF%E6%9F%BB.xlsx?d=wdd6f3d97f7b04a538095c459f8eee2eb&csf=1&web=1&e=saqcTC) 上記資料を参考にタスク内で担当するメソッドについてロックの対応箇所を整理しました。 [Task3520](https://ndstokyo.sharepoint.com/:f:/r/sites/Piranha/Shared%20Documents/General/OMDS/%E3%82%B9%E3%82%AF%E3%83%AA%E3%83%BC%E3%83%B3%E3%82%B7%E3%83%A7%E3%83%83%E3%83%88/Task3520?csf=1&web=1&e=ewuJoe) ## レビューポイント - 競合の対応として適切でしょうか？ ## UIの変更 - なし ## 動作確認状況 - ローカルでマイグレーション確認 --- ...053-add_accounts_users_templates_index.sql | 19 +++++++++++++++++++ .../accounts/accounts.repository.service.ts | 3 +++ .../licenses/licenses.repository.service.ts | 7 ++----- .../template_files.repository.service.ts | 1 + .../users/users.repository.service.ts | 1 + 5 files changed, 26 insertions(+), 5 deletions(-) create mode 100644 dictation_server/db/migrations/053-add_accounts_users_templates_index.sql diff --git a/dictation_server/db/migrations/053-add_accounts_users_templates_index.sql b/dictation_server/db/migrations/053-add_accounts_users_templates_index.sql new file mode 100644 index 0000000..5b4a4a6 --- /dev/null +++ b/dictation_server/db/migrations/053-add_accounts_users_templates_index.sql @@ -0,0 +1,19 @@ +-- +migrate Up +ALTER TABLE `accounts` ADD INDEX `idx_accounts_tier` (tier); +ALTER TABLE `accounts` ADD INDEX `idx_accounts_parent_account_id` (parent_account_id); +ALTER TABLE `users` ADD INDEX `idx_users_external_id` (external_id); +ALTER TABLE `users` ADD INDEX `idx_users_email_verified` (email_verified); +ALTER TABLE `licenses` ADD INDEX `idx_licenses_order_id` (order_id); +ALTER TABLE `licenses` ADD INDEX `idx_licenses_status` (status); +ALTER TABLE `template_files` ADD INDEX `idx_template_files_account_id` (account_id); +ALTER TABLE `template_files` ADD INDEX `idx_template_files_file_name` (file_name(500)); + +-- +migrate Down +ALTER TABLE `accounts` DROP INDEX `idx_accounts_tier`; +ALTER TABLE `accounts` DROP INDEX `idx_accounts_parent_account_id`; +ALTER TABLE `users` DROP INDEX `idx_users_external_id`; +ALTER TABLE `users` DROP INDEX `idx_users_email_verified`; +ALTER TABLE `licenses` DROP INDEX `idx_licenses_order_id`; +ALTER TABLE `licenses` DROP INDEX `idx_licenses_status`; +ALTER TABLE `template_files` DROP INDEX `idx_template_files_account_id`; +ALTER TABLE `template_files` DROP INDEX `idx_template_files_file_name`; \ No newline at end of file diff --git a/dictation_server/src/repositories/accounts/accounts.repository.service.ts b/dictation_server/src/repositories/accounts/accounts.repository.service.ts index 6d7c758..2832672 100644 --- a/dictation_server/src/repositories/accounts/accounts.repository.service.ts +++ b/dictation_server/src/repositories/accounts/accounts.repository.service.ts @@ -821,6 +821,7 @@ export class AccountsRepositoryService { status: Not(LICENSE_ALLOCATED_STATUS.UNALLOCATED), }, comment: `${context.getTrackingId()}_${new Date().toUTCString()}`, + lock: { mode: 'pessimistic_write' }, }); // 存在した場合エラー @@ -1023,6 +1024,7 @@ export class AccountsRepositoryService { email_verified: true, }, comment: `${context.getTrackingId()}_${new Date().toUTCString()}`, + lock: { mode: 'pessimistic_write' }, }); if (!primaryAdminUser) { throw new AdminUserNotFoundError( @@ -1040,6 +1042,7 @@ export class AccountsRepositoryService { email_verified: true, }, comment: `${context.getTrackingId()}_${new Date().toUTCString()}`, + lock: { mode: 'pessimistic_write' }, }); if (!secondryAdminUser) { throw new AdminUserNotFoundError( diff --git a/dictation_server/src/repositories/licenses/licenses.repository.service.ts b/dictation_server/src/repositories/licenses/licenses.repository.service.ts index 5aeb576..a5e03a3 100644 --- a/dictation_server/src/repositories/licenses/licenses.repository.service.ts +++ b/dictation_server/src/repositories/licenses/licenses.repository.service.ts @@ -12,7 +12,6 @@ import { LICENSE_ALLOCATED_STATUS, LICENSE_ISSUE_STATUS, LICENSE_TYPE, - NODE_ENV_TEST, SWITCH_FROM_TYPE, TIERS, USER_LICENSE_STATUS, @@ -423,10 +422,7 @@ export class LicensesRepositoryService { po_number: poNumber, }, comment: `${context.getTrackingId()}_${new Date().toUTCString()}`, - // テスト環境の場合はロックを行わない(sqliteがlockに対応していないため) - ...(process.env.NODE_ENV !== NODE_ENV_TEST - ? { lock: { mode: 'pessimistic_write' } } - : {}), + lock: { mode: 'pessimistic_write' }, }); if (!issuingOrder) { // 注文が存在しない場合、エラー @@ -570,6 +566,7 @@ export class LicensesRepositoryService { id: newLicenseId, }, comment: `${context.getTrackingId()}_${new Date().toUTCString()}`, + lock: { mode: 'pessimistic_write' }, }); // ライセンスが存在しない場合はエラー diff --git a/dictation_server/src/repositories/template_files/template_files.repository.service.ts b/dictation_server/src/repositories/template_files/template_files.repository.service.ts index e3d3319..5fc805b 100644 --- a/dictation_server/src/repositories/template_files/template_files.repository.service.ts +++ b/dictation_server/src/repositories/template_files/template_files.repository.service.ts @@ -52,6 +52,7 @@ export class TemplateFilesRepositoryService { const template = await templateFilesRepo.findOne({ where: { account_id: accountId, file_name: fileName }, comment: `${context.getTrackingId()}_${new Date().toUTCString()}`, + lock: { mode: 'pessimistic_write' }, }); // 同名ファイルは同じものとして扱うため、すでにファイルがあれば更新（更新日時の履歴を残しておきたい） diff --git a/dictation_server/src/repositories/users/users.repository.service.ts b/dictation_server/src/repositories/users/users.repository.service.ts index ae3fccc..8c39441 100644 --- a/dictation_server/src/repositories/users/users.repository.service.ts +++ b/dictation_server/src/repositories/users/users.repository.service.ts @@ -289,6 +289,7 @@ export class UsersRepositoryService { const targetUser = await repo.findOne({ where: { id: id, account_id: accountId }, comment: `${context.getTrackingId()}_${new Date().toUTCString()}`, + lock: { mode: 'pessimistic_write' }, }); // 運用上ユーザがいないことはあり得ないが、プログラム上発生しうるのでエラーとして処理