Skip to content

トランザクションを使うようにする#65

Open
mix3 wants to merge 1 commit intofurikake6000:masterfrom
mix3:feature/use-transaction
Open

トランザクションを使うようにする#65
mix3 wants to merge 1 commit intofurikake6000:masterfrom
mix3:feature/use-transaction

Conversation

@mix3
Copy link
Copy Markdown
Contributor

@mix3 mix3 commented Dec 24, 2017

SQLite から Postgresql に変わったことで同時書き込みエラーが減り、同時更新による二重加算やロストアップデートなどのデータ不整合が顕在化してきているのではないかという気がしていますが、いかがでしょうか

スコア計算の変更でデータ不整合の問題が無くなっているようであれば、このPRは無視してください


ロストアップデートというのは前の更新を後の更新で上書きしてしまうことです

参考: https://qiita.com/tikamoto/items/f867050ff77d06a94215#ロストアップデートが起こる例

二重加算も二重リクエストなどで更新済みフラグを同時に読んでしまって両方の更新処理が通ってしまうなどすると発生します

こういったデータ不整合を防ぐにはロックを使う必要があります

参考: https://qiita.com/tikamoto/items/f867050ff77d06a94215#ロストアップデートを防ぐ

ということでこのPRでは transaction を使うのと select for update によるロックを入れてみました

これの対応をするにあたって以下の理由で view での更新がネックになるので controller や model にその処理を移動させたりもしています

  • controller や model でトランザクションを貼ったのにその外で更新されてしまって意味がない
  • view がデータを触るのは MVC 的にあまり良くない

テストがなく トップページ 摘発 摘発履歴 あたりの画面を目視確認しただけなので、これを適用して問題があったとき責任が取れないので、適用する場合はよくよく開発環境での確認のうえ慎重にお願いします
データ不整合対策の参考にして頂ければ幸いです

@furikake6000
Copy link
Copy Markdown
Owner

ありがとうございます!暗号が14個配布されてしまう問題はこのlockが原因な気もします...ソースコードを確認してからコミットしたいと思っております!お時間がかかります...すみません

@furikake6000
Copy link
Copy Markdown
Owner

(大変お時間空けてしまって申し訳ありません)
このコミットについて自分なりに調べてみました。
スコアを毎度計算する方式にしたため、ご指摘のようなスコア計算エラーについてはもう発生しなくなりました。しかし、いただいたプルリクの一部については適用させていただきました。(具体的に言うと、ビューでデータ操作を行っていた部分とtw_account_update関数、saveをsave!に置き換えた、です)
また、この件については今後も注視したいため引き続きOpenのままにさせていただきたいと思います。
本当にありがとうございました!今後ともよろしくお願いします!

@mix3
Copy link
Copy Markdown
Contributor Author

mix3 commented Jan 5, 2018

無駄にデカイPRに目を通していただいてありがとうございます。また一部取り込んでいただきありがとうございました 🙇

ちなみに PR は close しても残りますし close した後で必要になれば reopen(再度open状態にすること) も可能なので、これぐらい処遇が決まっていれば個人的には close にしてしまっても良いかなと思いました。(細かいことなのでご対応はお任せします)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants