Merge pull request #778 from isucon/fix/bench.evaluation

idk指定時の２回目以降のリクエストが常に成功レスポンスを返されていた

bench/benchmarker/world/errors.go

@@ -25,6 +25,8 @@ const (
 	_
 	// ErrorCodeFailedToEvaluate ユーザーが送迎の評価をしようとしたが失敗した
 	ErrorCodeFailedToEvaluate
+	// ErrorCodeEvaluateTimeout ユーザーが送迎の評価をしようとしたがタイムアウトした
+	ErrorCodeEvaluateTimeout
 	// ErrorCodeFailedToCheckRequestHistory ユーザーがリクエスト履歴を確認しようとしたが失敗した
 	ErrorCodeFailedToCheckRequestHistory
 	// ErrorCodeFailedToCreateRequest ユーザーがリクエストを作成しようとしたが失敗した
@@ -88,13 +90,15 @@ var CriticalErrorCodes = map[ErrorCode]bool{
 	ErrorCodeIncorrectAmountOfFareCharged:                   true,
 	ErrorCodeUncontrollableRequestReceived:                  true,
 	ErrorCodeMatchingTimeout:                                true,
+	ErrorCodeEvaluateTimeout:                                true,
 }
 
 var ErrorTexts = map[ErrorCode]string{
 	ErrorCodeFailedToSendChairCoordinate:                    "椅子の座標送信に失敗しました",
 	ErrorCodeFailedToDepart:                                 "椅子が出発できませんでした",
 	ErrorCodeFailedToAcceptRequest:                          "椅子がライドを受理できませんでした",
 	ErrorCodeFailedToEvaluate:                               "ユーザーのライド評価に失敗しました",
+	ErrorCodeEvaluateTimeout:                                "ユーザーのライド評価がタイムアウトしました",
 	ErrorCodeFailedToCheckRequestHistory:                    "ユーザーがライド履歴の取得に失敗しました",
 	ErrorCodeFailedToCreateRequest:                          "ユーザーが新しくライドを作成できませんでした",
 	ErrorCodeUserNotRequestingButStatusChanged:              "ユーザーが想定していない通知を受け取りました",

bench/benchmarker/world/user.go

@@ -1,6 +1,7 @@
 package world
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"log/slog"
@@ -175,6 +176,9 @@ func (u *User) Tick(ctx *Context) error {
 				res, err := u.Client.SendEvaluation(ctx, u.Request, score)
 				if err != nil {
 					u.Request.Statuses.Unlock()
+					if errors.Is(err, context.DeadlineExceeded) {
+						return WrapCodeError(ErrorCodeEvaluateTimeout, err)
+					}
 					return WrapCodeError(ErrorCodeFailedToEvaluate, err)
 				}
 

bench/payment/handler.go

@@ -61,8 +61,6 @@ func (s *Server) PostPaymentsHandler(w http.ResponseWriter, r *http.Request) {
 			writeJSON(w, http.StatusUnprocessableEntity, map[string]string{"message": "リクエストペイロードがサーバーに記録されているものと異なります"})
 			return
 		}
-		writeResponse(w, p.Status)
-		return
 	} else {
 		p.Token = token
 		p.Amount = req.Amount
@@ -81,14 +79,16 @@ func (s *Server) PostPaymentsHandler(w http.ResponseWriter, r *http.Request) {
 	if failurePercentage > 50 {
 		failurePercentage = 50
 	}
-	failureCount, _ := s.failureCounts.GetOrSetDefault(token, func() int { return 0 })
-	if rand.IntN(100) > failurePercentage || failureCount >= 4 {
+	retryCount, _ := s.retryCounts.GetOrSetDefault(token, func() int { return -1 })
+	s.retryCounts.Set(token, retryCount+1)
+	if rand.IntN(100) > failurePercentage || retryCount >= 4 {
 		// lock はここでしか触らない。lock が true の場合は idempotency key が同じリクエストが処理中の場合のみ
 		if p.locked.CompareAndSwap(false, true) {
+			defer p.locked.Store(false)
 			alreadyProcessed := false
 			if !newPayment {
 				for _, processed := range s.processedPayments.ToSlice() {
-					if processed.payment == p {
+					if processed.payment.IdempotencyKey == p.IdempotencyKey {
 						alreadyProcessed = true
 						break
 					}
@@ -100,18 +100,15 @@ func (s *Server) PostPaymentsHandler(w http.ResponseWriter, r *http.Request) {
 				if p.Status.Err != nil {
 					s.errChan <- p.Status.Err
 				}
-				s.failureCounts.Delete(token)
-				p.locked.Store(false) // idenpotency key が同じリクエストが来たときにエラーを返さないように
 			}
-			if rand.IntN(100) > failurePercentage || failureCount >= 4 {
+			if rand.IntN(100) > failurePercentage || retryCount >= 4 {
+				s.retryCounts.Set(token, 0)
 				writeResponse(w, p.Status)
 			} else {
 				writeRandomError(w)
 			}
 			return
 		}
-	} else {
-		s.failureCounts.Set(token, failureCount+1)
 	}
 
 	// 不安定なエラーを再現

bench/payment/server.go

@@ -19,7 +19,7 @@ type processedPayment struct {
 type Server struct {
 	mux               *http.ServeMux
 	knownKeys         *concurrent.SimpleMap[string, *Payment]
-	failureCounts     *concurrent.SimpleMap[string, int]
+	retryCounts       *concurrent.SimpleMap[string, int]
 	processedPayments *concurrent.SimpleSlice[*processedPayment]
 	processTime       time.Duration
 	verifier          Verifier
@@ -31,7 +31,7 @@ func NewServer(verifier Verifier, processTime time.Duration, errChan chan error)
 	s := &Server{
 		mux:               http.NewServeMux(),
 		knownKeys:         concurrent.NewSimpleMap[string, *Payment](),
-		failureCounts:     concurrent.NewSimpleMap[string, int](),
+		retryCounts:       concurrent.NewSimpleMap[string, int](),
 		processedPayments: concurrent.NewSimpleSlice[*processedPayment](),
 		processTime:       processTime,
 		verifier:          verifier,