Skip to content

Commit 9a87643

Browse files
committed
coordinator: refund attempts on coordinator-side dispatch failures
formatProverTask/applyUniversal/InsertProverTask failures after Update*Attempts previously refunded only active_attempts, permanently burning total_attempts. After SessionAttempts (5) such tasks became invisible to GetUnassigned* (total_attempts < maxAttempts) with no prover_task row, so the cron timeout checker could never mark them failed either — silent starvation (shadow-testing Trap 22/24). Add RefundAttemptsByHash (chunk/batch/bundle orm) and recoverAttempts: when the charge happened in this Assign call (hasAssignedTask == nil), refund both counters and reset proving_status to unassigned; re-polls of already-assigned tasks keep the old active-only recovery. Also cover the previously unrecovered fixCompatibility failure sites in batch/bundle assign.
1 parent 877fa44 commit 9a87643

7 files changed

Lines changed: 313 additions & 14 deletions

File tree

coordinator/internal/logic/provertask/batch_prover_task.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
199199

200200
taskMsg, err := bp.formatProverTask(ctx.Copy(), proverTask, batchTask, hardForkName)
201201
if err != nil {
202-
bp.recoverActiveAttempts(ctx, batchTask)
202+
bp.recoverAttempts(ctx, taskCtx, batchTask)
203203
log.Error("format prover task failure", "task_id", batchTask.Hash, "err", err)
204204
return nil, ErrCoordinatorInternalFailure
205205
}
@@ -208,7 +208,7 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
208208

209209
taskMsg, metadata, err = bp.applyUniversal(taskMsg)
210210
if err != nil {
211-
bp.recoverActiveAttempts(ctx, batchTask)
211+
bp.recoverAttempts(ctx, taskCtx, batchTask)
212212
log.Error("Generate universal prover task failure", "task_id", batchTask.Hash, "type", "batch", "err", err)
213213
return nil, ErrCoordinatorInternalFailure
214214
}
@@ -217,7 +217,8 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
217217
if isCompatibilityFixingVersion(taskCtx.ProverVersion) {
218218
log.Info("Apply compatibility fixing for prover", "version", taskCtx.ProverVersion)
219219
if err := fixCompatibility(taskMsg); err != nil {
220-
log.Error("apply compatibility failure", "err", err)
220+
bp.recoverAttempts(ctx, taskCtx, batchTask)
221+
log.Error("apply compatibility failure", "task_id", batchTask.Hash, "err", err)
221222
return nil, ErrCoordinatorInternalFailure
222223
}
223224
}
@@ -226,7 +227,7 @@ func (bp *BatchProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
226227
// Store session info.
227228
if taskCtx.hasAssignedTask == nil {
228229
if err = bp.proverTaskOrm.InsertProverTask(ctx.Copy(), proverTask); err != nil {
229-
bp.recoverActiveAttempts(ctx, batchTask)
230+
bp.recoverAttempts(ctx, taskCtx, batchTask)
230231
log.Error("insert batch prover task info fail", "task_id", batchTask.Hash, "publicKey", taskCtx.PublicKey, "err", err)
231232
return nil, ErrCoordinatorInternalFailure
232233
}
@@ -288,7 +289,17 @@ func (bp *BatchProverTask) formatProverTask(ctx context.Context, task *orm.Prove
288289
return taskMsg, nil
289290
}
290291

291-
func (bp *BatchProverTask) recoverActiveAttempts(ctx *gin.Context, batchTask *orm.Batch) {
292+
// recoverAttempts rolls back the attempt charged by UpdateBatchAttempts when the coordinator
293+
// fails to dispatch a freshly assigned task (hasAssignedTask == nil): both counters are refunded
294+
// and proving_status is reset to unassigned. For a re-poll of an already assigned task nothing
295+
// was charged in this call, so only active_attempts is decremented as before.
296+
func (bp *BatchProverTask) recoverAttempts(ctx *gin.Context, taskCtx *proverTaskContext, batchTask *orm.Batch) {
297+
if taskCtx.hasAssignedTask == nil {
298+
if err := bp.batchOrm.RefundAttemptsByHash(ctx.Copy(), batchTask.Hash); err != nil {
299+
log.Error("failed to refund batch attempts", "hash", batchTask.Hash, "error", err)
300+
}
301+
return
302+
}
292303
if err := bp.batchOrm.DecreaseActiveAttemptsByHash(ctx.Copy(), batchTask.Hash); err != nil {
293304
log.Error("failed to recover batch active attempts", "hash", batchTask.Hash, "error", err)
294305
}

coordinator/internal/logic/provertask/bundle_prover_task.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -196,15 +196,15 @@ func (bp *BundleProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinat
196196

197197
taskMsg, err := bp.formatProverTask(ctx.Copy(), proverTask, hardForkName)
198198
if err != nil {
199-
bp.recoverActiveAttempts(ctx, bundleTask)
199+
bp.recoverAttempts(ctx, taskCtx, bundleTask)
200200
log.Error("format bundle prover task failure", "task_id", bundleTask.Hash, "err", err)
201201
return nil, ErrCoordinatorInternalFailure
202202
}
203203
if getTaskParameter.Universal {
204204
var metadata []byte
205205
taskMsg, metadata, err = bp.applyUniversal(taskMsg)
206206
if err != nil {
207-
bp.recoverActiveAttempts(ctx, bundleTask)
207+
bp.recoverAttempts(ctx, taskCtx, bundleTask)
208208
log.Error("Generate universal prover task failure", "task_id", bundleTask.Hash, "type", "bundle", "err", err)
209209
return nil, ErrCoordinatorInternalFailure
210210
}
@@ -215,7 +215,8 @@ func (bp *BundleProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinat
215215
if isCompatibilityFixingVersion(taskCtx.ProverVersion) {
216216
log.Info("Apply compatibility fixing for prover", "version", taskCtx.ProverVersion)
217217
if err := fixCompatibility(taskMsg); err != nil {
218-
log.Error("apply compatibility failure", "err", err)
218+
bp.recoverAttempts(ctx, taskCtx, bundleTask)
219+
log.Error("apply compatibility failure", "task_id", bundleTask.Hash, "err", err)
219220
return nil, ErrCoordinatorInternalFailure
220221
}
221222
}
@@ -224,7 +225,7 @@ func (bp *BundleProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinat
224225
// Store session info.
225226
if taskCtx.hasAssignedTask == nil {
226227
if err = bp.proverTaskOrm.InsertProverTask(ctx.Copy(), proverTask); err != nil {
227-
bp.recoverActiveAttempts(ctx, bundleTask)
228+
bp.recoverAttempts(ctx, taskCtx, bundleTask)
228229
log.Error("insert bundle prover task info fail", "task_id", bundleTask.Hash, "publicKey", taskCtx.PublicKey, "err", err)
229230
return nil, ErrCoordinatorInternalFailure
230231
}
@@ -313,7 +314,17 @@ func (bp *BundleProverTask) formatProverTask(ctx context.Context, task *orm.Prov
313314
return taskMsg, nil
314315
}
315316

316-
func (bp *BundleProverTask) recoverActiveAttempts(ctx *gin.Context, bundleTask *orm.Bundle) {
317+
// recoverAttempts rolls back the attempt charged by UpdateBundleAttempts when the coordinator
318+
// fails to dispatch a freshly assigned task (hasAssignedTask == nil): both counters are refunded
319+
// and proving_status is reset to unassigned. For a re-poll of an already assigned task nothing
320+
// was charged in this call, so only active_attempts is decremented as before.
321+
func (bp *BundleProverTask) recoverAttempts(ctx *gin.Context, taskCtx *proverTaskContext, bundleTask *orm.Bundle) {
322+
if taskCtx.hasAssignedTask == nil {
323+
if err := bp.bundleOrm.RefundAttemptsByHash(ctx.Copy(), bundleTask.Hash); err != nil {
324+
log.Error("failed to refund bundle attempts", "hash", bundleTask.Hash, "error", err)
325+
}
326+
return
327+
}
317328
if err := bp.bundleOrm.DecreaseActiveAttemptsByHash(ctx.Copy(), bundleTask.Hash); err != nil {
318329
log.Error("failed to recover bundle active attempts", "hash", bundleTask.Hash, "error", err)
319330
}

coordinator/internal/logic/provertask/chunk_prover_task.go

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
194194

195195
taskMsg, err := cp.formatProverTask(ctx.Copy(), proverTask, chunkTask, hardForkName)
196196
if err != nil {
197-
cp.recoverActiveAttempts(ctx, chunkTask)
197+
cp.recoverAttempts(ctx, taskCtx, chunkTask)
198198
log.Error("format prover task failure", "task_id", chunkTask.Hash, "err", err)
199199
return nil, ErrCoordinatorInternalFailure
200200
}
@@ -203,7 +203,7 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
203203
var metadata []byte
204204
taskMsg, metadata, err = cp.applyUniversal(taskMsg)
205205
if err != nil {
206-
cp.recoverActiveAttempts(ctx, chunkTask)
206+
cp.recoverAttempts(ctx, taskCtx, chunkTask)
207207
log.Error("Generate universal prover task failure", "task_id", chunkTask.Hash, "type", "chunk", "err", err)
208208
return nil, ErrCoordinatorInternalFailure
209209
}
@@ -212,7 +212,7 @@ func (cp *ChunkProverTask) Assign(ctx *gin.Context, getTaskParameter *coordinato
212212

213213
if taskCtx.hasAssignedTask == nil {
214214
if err = cp.proverTaskOrm.InsertProverTask(ctx.Copy(), proverTask); err != nil {
215-
cp.recoverActiveAttempts(ctx, chunkTask)
215+
cp.recoverAttempts(ctx, taskCtx, chunkTask)
216216
log.Error("insert chunk prover task fail", "task_id", chunkTask.Hash, "publicKey", taskCtx.PublicKey, "err", err)
217217
return nil, ErrCoordinatorInternalFailure
218218
}
@@ -269,7 +269,17 @@ func (cp *ChunkProverTask) formatProverTask(ctx context.Context, task *orm.Prove
269269
return proverTaskSchema, nil
270270
}
271271

272-
func (cp *ChunkProverTask) recoverActiveAttempts(ctx *gin.Context, chunkTask *orm.Chunk) {
272+
// recoverAttempts rolls back the attempt charged by UpdateChunkAttempts when the coordinator
273+
// fails to dispatch a freshly assigned task (hasAssignedTask == nil): both counters are refunded
274+
// and proving_status is reset to unassigned. For a re-poll of an already assigned task nothing
275+
// was charged in this call, so only active_attempts is decremented as before.
276+
func (cp *ChunkProverTask) recoverAttempts(ctx *gin.Context, taskCtx *proverTaskContext, chunkTask *orm.Chunk) {
277+
if taskCtx.hasAssignedTask == nil {
278+
if err := cp.chunkOrm.RefundAttemptsByHash(ctx, chunkTask.Hash); err != nil {
279+
log.Error("failed to refund chunk attempts", "hash", chunkTask.Hash, "error", err)
280+
}
281+
return
282+
}
273283
if err := cp.chunkOrm.DecreaseActiveAttemptsByHash(ctx, chunkTask.Hash); err != nil {
274284
log.Error("failed to recover chunk active attempts", "hash", chunkTask.Hash, "error", err)
275285
}

coordinator/internal/orm/batch.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,3 +471,32 @@ func (o *Batch) DecreaseActiveAttemptsByHash(ctx context.Context, batchHash stri
471471
}
472472
return nil
473473
}
474+
475+
// RefundAttemptsByHash refunds a full assignment attempt of a batch given its hash:
476+
// it decrements both total_attempts and active_attempts and resets proving_status to unassigned.
477+
// It is used to roll back UpdateBatchAttempts when the coordinator fails to dispatch the task
478+
// (e.g. task formatting or prover task insertion failure), so the attempt is not burned permanently.
479+
func (o *Batch) RefundAttemptsByHash(ctx context.Context, batchHash string, dbTX ...*gorm.DB) error {
480+
db := o.db
481+
if len(dbTX) > 0 && dbTX[0] != nil {
482+
db = dbTX[0]
483+
}
484+
db = db.WithContext(ctx)
485+
db = db.Model(&Batch{})
486+
db = db.Where("hash = ?", batchHash)
487+
db = db.Where("total_attempts > ?", 0)
488+
db = db.Where("active_attempts > ?", 0)
489+
db = db.Where("proving_status != ?", int(types.ProvingTaskVerified))
490+
result := db.Updates(map[string]interface{}{
491+
"total_attempts": gorm.Expr("total_attempts - 1"),
492+
"active_attempts": gorm.Expr("active_attempts - 1"),
493+
"proving_status": int(types.ProvingTaskUnassigned),
494+
})
495+
if result.Error != nil {
496+
return fmt.Errorf("Batch.RefundAttemptsByHash error: %w, batch hash: %v", result.Error, batchHash)
497+
}
498+
if result.RowsAffected == 0 {
499+
log.Warn("No rows were affected in RefundAttemptsByHash", "batch hash", batchHash)
500+
}
501+
return nil
502+
}

coordinator/internal/orm/bundle.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,3 +243,32 @@ func (o *Bundle) DecreaseActiveAttemptsByHash(ctx context.Context, bundleHash st
243243
}
244244
return nil
245245
}
246+
247+
// RefundAttemptsByHash refunds a full assignment attempt of a bundle given its hash:
248+
// it decrements both total_attempts and active_attempts and resets proving_status to unassigned.
249+
// It is used to roll back UpdateBundleAttempts when the coordinator fails to dispatch the task
250+
// (e.g. task formatting or prover task insertion failure), so the attempt is not burned permanently.
251+
func (o *Bundle) RefundAttemptsByHash(ctx context.Context, bundleHash string, dbTX ...*gorm.DB) error {
252+
db := o.db
253+
if len(dbTX) > 0 && dbTX[0] != nil {
254+
db = dbTX[0]
255+
}
256+
db = db.WithContext(ctx)
257+
db = db.Model(&Bundle{})
258+
db = db.Where("hash = ?", bundleHash)
259+
db = db.Where("total_attempts > ?", 0)
260+
db = db.Where("active_attempts > ?", 0)
261+
db = db.Where("proving_status != ?", int(types.ProvingTaskVerified))
262+
result := db.Updates(map[string]interface{}{
263+
"total_attempts": gorm.Expr("total_attempts - 1"),
264+
"active_attempts": gorm.Expr("active_attempts - 1"),
265+
"proving_status": int(types.ProvingTaskUnassigned),
266+
})
267+
if result.Error != nil {
268+
return fmt.Errorf("Bundle.RefundAttemptsByHash error: %w, bundle hash: %v", result.Error, bundleHash)
269+
}
270+
if result.RowsAffected == 0 {
271+
log.Warn("No rows were affected in RefundAttemptsByHash", "bundle hash", bundleHash)
272+
}
273+
return nil
274+
}

coordinator/internal/orm/chunk.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -422,3 +422,32 @@ func (o *Chunk) DecreaseActiveAttemptsByHash(ctx context.Context, chunkHash stri
422422
}
423423
return nil
424424
}
425+
426+
// RefundAttemptsByHash refunds a full assignment attempt of a chunk given its hash:
427+
// it decrements both total_attempts and active_attempts and resets proving_status to unassigned.
428+
// It is used to roll back UpdateChunkAttempts when the coordinator fails to dispatch the task
429+
// (e.g. task formatting or prover task insertion failure), so the attempt is not burned permanently.
430+
func (o *Chunk) RefundAttemptsByHash(ctx context.Context, chunkHash string, dbTX ...*gorm.DB) error {
431+
db := o.db
432+
if len(dbTX) > 0 && dbTX[0] != nil {
433+
db = dbTX[0]
434+
}
435+
db = db.WithContext(ctx)
436+
db = db.Model(&Chunk{})
437+
db = db.Where("hash = ?", chunkHash)
438+
db = db.Where("total_attempts > ?", 0)
439+
db = db.Where("active_attempts > ?", 0)
440+
db = db.Where("proving_status != ?", int(types.ProvingTaskVerified))
441+
result := db.Updates(map[string]interface{}{
442+
"total_attempts": gorm.Expr("total_attempts - 1"),
443+
"active_attempts": gorm.Expr("active_attempts - 1"),
444+
"proving_status": int(types.ProvingTaskUnassigned),
445+
})
446+
if result.Error != nil {
447+
return fmt.Errorf("Chunk.RefundAttemptsByHash error: %w, chunk hash: %v", result.Error, chunkHash)
448+
}
449+
if result.RowsAffected == 0 {
450+
log.Warn("No rows were affected in RefundAttemptsByHash", "chunk hash", chunkHash)
451+
}
452+
return nil
453+
}

0 commit comments

Comments
 (0)