エンジニア研修 2020/7/1
レビュー会
フロント
ebiken.iconPayPayとか、QRを読み取った後の金額入力はよく考えたら無駄なフロー メニュー毎にQRがあって金額を入力する必要がない方がスマートでは
設計
ebiken.icon今回のコード発行サービスはシンプルな情報で良いのでできるだけ早く楽に実装ができるものを選択した。
残高チャージで不整合が発生したときの対策
ebiken.icon基本的にはロールバック + リトライ
ebiken.iconコード発行サービスにロールバックエンドポイントを生やしている
APIサーバー
認証情報はどこに置いているのか
ebiken.iconCognitoに保存している。id_tokenがユーザーのトークンになる テストはどうなってるか
ebiken.iconDBを直接叩くテストも書いてる
API設計
APIドキュメントにエラーが起きるケースとかちゃんと書こう
ebiken.icon404が発生する場合があるということしかドキュメントに書いてなかった。確かに実装する際にこれはどのエラーコードだ?となってたので、ちゃんと書いておいた方が良かったな
いまはidはintだけど、スケールを考えたらUUIDとか使おう 冪等性を担保するためにクライアント側で生成しているtransaction_id dbに保存する必要はなさそう。Redisとか一時的にデータを置く場所に保存するのが良い。時間が立つとテーブルが肥大化してくるので定期的にクリーンしたい クライアント側でfingerprint(uuidとかランダム生成したもの)を送る
決済を行える payment_code を一度しか決済できないものにしてしまえば良さそう
vendor と customer でかなりコードが重複している
ebiken.icon機能がほぼ一緒だったので重複になってしまった。今回のケースでは一緒のほうが良かったかもしれない。
charge とか code とか命名がわかりにくい
ebiken.icon英語だと charge は日本語で残高をチャージするという意味と同じだと勝手に思ってたが、ぜんぜん違うぽい。ちゃんと単語の意味は調べておこう...
ebiken.icon命名むずい
コード発行サービスに認証欲しい
ebiken.iconスケジュールを理由に実装スキップした。本番だと認証無いのはアウト。
DB設計
命名わかりにくい、customerとvendorの重複(apiサーバーと一緒)
テーブルのロックを考えて、正規化から外れて一部別テーブルにしていたが、今回のケースでは必要なさそう。同時にwriteが同じテーブルにかかる場合にロックのことを考える。
ebiken.iconwriteとreadのテーブルは分けたほうが良いものだと勘違いしていた。実際のユースケースレベルでどこにwriteがかかるかとか考えていなかった。
ebiken.iconDBのロック周り全然わからない
複数のテーブルに同じ情報がまたがると同期に失敗した時つらくなる
ebiken.iconどっちが正しいものかわからなくなる
temporaryなコードはrdbに保存しなくて良い
ログ目的なら別のサービスに保存しておく
ebiken.icon別のサービスに保存するのが面倒でRDBにいれちゃうケースも結構ありそう。
ebiken.icon確かにRedisとかに保存するので十分 + むしろそっちのほうが expires_at とか指定できて便利だった。 インフラ
今回のケースでは必要ないが、DBだけ別のVPCにいれることがある ebiken.iconこの発想は今まで無かったので驚き
追加質問
バックエンドのパッケージ構成、サービスを分割することを想定して auth, payment の2つに分けて作ってみたが、どう思うか
ebiken.iconこんな感じのパッケージ構成になっている
code:_
- main.go
- restapi # go-swagger の自動生成コード
- server
- server.go
- handler
- <service_name>_handler.go
- auth_customer_handler.go
- auth_vendor_handler.go
- <service_name>
- <service_name>.go
- entity
- db
- <entity>.go
- view
- <entity>.go
- repository
- <entity>_repository.go
- infrastructure
- <infrastructure(ex. db)>.go
yes, division on auth and payment sounds reasonable. in microservice architecture typically the authentication is in its own services, especially when you use JWT
ebiken.icon最初からマイクロサービス化することを前提にパッケージ構成を考えて作ったが、問題なさそうということだった
from what i saw in the code, it looks fine
DBの外部キー制約について
DBをサービス毎に置いて分割したら外部キー制約貼れなくなる
ebiken.iconUUIDをIDに指定すれば良い? ebiken.iconGoogleとか超大規模のサービスではどうしてるんだろう
ebiken.icon結果整合性とかそういう話かな。全部が成功するのを待つとか that’s right, if you have separate databases you won’t be able to use foreign keys. so your data integrity must rely on other methods.
the next two questions are related. in super scale services, typically you have many data centers to handle different geolocations. it’s a form of sharding.
when you shard your databases into multiple data centers, it’s very important to use UUID as primary key to identify objects. UUID makes sure you won’t get collisions on objects in different data centers.
ebiken.icon基本的にはUUIDで良さそう。 ebiken.icon int で primary key をつくっておいて、後からUUIDにするというのは結構ありそう。
ebiken.iconソートしたい場合はソート用のカラムを用意するとか、timestampでソートするとか、ulidみたいなソートできるuidを使うとかかな 外部サービスへのリクエストをロールバックしたとき、ロールバックが失敗したらどうする?
ebiken.iconまず何回かリトライする
ebiken.iconリトライして何回か失敗したら運用で解決
lastly, to ensure integrity, you can use the approach you have (retry + logging)
another way to enforce it is using an intermediate state. for example, when money is being charged, you create a “payment-authorization” object in the database (in a db transaction)
and on the other server, you use the payment-authorization id to create a payment-receive object. once you can guarantee both actions are successful, you finalize the transaction on each database independently.
if anything fail in the process, the intermediate object can be used to rollback the transaction.
and a worker can clean up any successful transaction’s intermediate object after it verifies with both servers.
テストについて
理想は100%だろうけど、テストケースのカバレッジって何%目指とかある?
インテグレーションテストってどのくらい網羅して行う?
many companies aim for 100% test coverage but this can be misleading. not having 100% isn’t necessarily bad (some logic may not need mechanical testing) and having 100% isn’t necessarily true (if you have more than 2 if branches in sequence, you really should test each combination of the if condition to fully test the code so the real needed tests sometimes grow exponentially to the complexity of the logic).
for this reason, it’s best to decide your target test coverage based on the complexity of your logic.
the same goes for integration tests; if the unit tests are well designed and mocked then maybe you don’t even need integration tests. however in most situations you should test with a combination of unit and integration test. Decide on a case by case basis.
ebiken.icon基本的には100%を目指すけど、それを目指すことが本質ではない
今日からbackend/frontend合わせて全体の動作確認+バグ修正をやっている
チーム開発
良かったこと
レビュー会めちゃくちゃ学びがあった
ebiken.iconCognito使った認証フローと分散トランザクションのロールバック処理を goodと言ってもらえたのは嬉しかった ebiken.iconちゃんとアジェンダ用意してレビュー会に臨んだのも良かった
改善/トライしたいこと
test の coverageを上げていく
エラーハンドリング、ロギング周りの改善
感想/その他
レビュー会を今日は2hくらいがっつりやってもらった。非常に学びがあった
レビュー会後半では頑張って英語で説明+質問したが、英語力のなさを痛感した。なんとなく言いたいことわかるレベルでは議論にならないのと、2~3割聞き逃してしまっている気がするので凄くもったいないと思った
1on1でチーム開発についてタスクを適切にアサインするのが難しいという話をした。
任せるタスクの粒度が荒すぎて時間がかかってしまった
最初は小さな成功体験を積むことで効率よく学ぶ
モチベーションが上がる要因を把握して適切なアサインの仕方をする
ざっくり任せられるのが好きとか、小さく渡して短期的に処理していくのが好きとか