Skip to content

Commit 4e5bfe7

Browse files
committed
Skip events for soft stop failures
1 parent c2bbd07 commit 4e5bfe7

6 files changed

Lines changed: 51 additions & 13 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Added `JobListParams.TagsAll` and `JobListParams.TagsAny` for filtering jobs that match every or any exact tag, respectively. [PR #1339](https://github.com/riverqueue/river/pull/1339).
1313

14+
### Changed
15+
16+
- Jobs that didn't finish in time organically while a client was stopping and had to have their context cancelled no longer have this cancellation counted as an error. `attempt` is reset to the number it was before the job started working, `errors` is left unchanged, and `state` is made `available` so jobs are eligible to be retried immediately. [PR #1290](https://github.com/riverqueue/river/pull/1290)
17+
1418
## [0.42.0] - 2026-07-31
1519

1620
### Added
@@ -80,7 +84,6 @@ river migrate-get --database-url sqlite:// --version 7 --down > river7.down.sql
8084
- Convert SQLite JSON columns to JSONB (including migration). [PR #1224](https://github.com/riverqueue/river/pull/1224).
8185
- Change SQLite driver operations over to use bulk inserts where possible now that sqlc has better support for `json_each`. [PR #1276](https://github.com/riverqueue/river/pull/1276)
8286
- Detect duplicate step names across `river.ResumableStep` and return a validation error. [PR #1281](https://github.com/riverqueue/river/pull/1281)
83-
- Jobs that didn't finish in time organically while a client was stopping and had to have their context cancelled no longer have this cancellation counted as an error. `attempt` is reset to the number it was before the job started working, `errors` is left unchanged, and `state` is made `available` so jobs are eligible to be retried immediately. [PR #1290](https://github.com/riverqueue/river/pull/1290)
8487
- Earlier backpressure from `BatchCompleter` when it's throughput is saturated with fewer warnings to console. [PR #1292](https://github.com/riverqueue/river/pull/1292)
8588
- Series of minor optimizations in `BatchCompleter` raising throughput ~20% when it's the bottleneck in job processing (e.g. in benchmarks). [PR #1293](https://github.com/riverqueue/river/pull/1293)
8689

client_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2750,6 +2750,21 @@ func Test_Client_SoftStopTimeout(t *testing.T) {
27502750
testutil.JobArgsReflectKind[JobArgs]
27512751
}
27522752

2753+
requireNoJobEvents := func(t *testing.T, subscribeChan <-chan *Event) {
2754+
t.Helper()
2755+
2756+
var unexpectedEvent *Event
2757+
require.Eventually(t, func() bool {
2758+
select {
2759+
case unexpectedEvent = <-subscribeChan:
2760+
return unexpectedEvent == nil
2761+
default:
2762+
return true
2763+
}
2764+
}, 500*time.Millisecond, 10*time.Millisecond)
2765+
require.Nil(t, unexpectedEvent)
2766+
}
2767+
27532768
t.Run("EscalatesToHardStopAfterTimeout", func(t *testing.T) {
27542769
t.Parallel()
27552770

@@ -2766,6 +2781,8 @@ func Test_Client_SoftStopTimeout(t *testing.T) {
27662781
}))
27672782

27682783
client := runNewTestClient(ctx, t, config)
2784+
subscribeChan, cancelSubscribe := client.Subscribe(EventKindJobCancelled, EventKindJobCompleted, EventKindJobFailed, EventKindJobSnoozed)
2785+
t.Cleanup(cancelSubscribe)
27692786

27702787
_, err := client.Insert(ctx, JobArgs{}, nil)
27712788
require.NoError(t, err)
@@ -2782,6 +2799,7 @@ func Test_Client_SoftStopTimeout(t *testing.T) {
27822799
default:
27832800
t.Fatal("expected job to have been cancelled by soft stop timeout")
27842801
}
2802+
requireNoJobEvents(t, subscribeChan)
27852803
})
27862804

27872805
t.Run("ErroringJobGetsFreshAttempt", func(t *testing.T) {
@@ -2893,6 +2911,8 @@ func Test_Client_SoftStopTimeout(t *testing.T) {
28932911

28942912
client, err := NewClient(driver, config)
28952913
require.NoError(t, err)
2914+
subscribeChan, cancelSubscribe := client.Subscribe(EventKindJobCancelled, EventKindJobCompleted, EventKindJobFailed, EventKindJobSnoozed)
2915+
t.Cleanup(cancelSubscribe)
28962916

28972917
startCtx, startCtxCancel := context.WithCancel(ctx)
28982918
defer startCtxCancel()
@@ -2915,6 +2935,7 @@ func Test_Client_SoftStopTimeout(t *testing.T) {
29152935
default:
29162936
t.Fatal("expected job to have been cancelled by soft stop timeout")
29172937
}
2938+
requireNoJobEvents(t, subscribeChan)
29182939
})
29192940
}
29202941

internal/jobcompleter/job_completer.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,9 @@ func (c *InlineCompleter) JobSetStateIfRunning(ctx context.Context, stats *jobst
9595

9696
// The driver intentionally returns 0 rows when a job is deleted while the
9797
// completer is finalizing it (see UnknownJobIgnored shared driver test).
98-
// Guard against an index-out-of-range panic in that case.
99-
if len(jobs) < 1 {
98+
// Guard against an index-out-of-range panic in that case, and also skip
99+
// publishing an event when requested by the caller.
100+
if params.SkipEvent || len(jobs) < 1 {
100101
return nil
101102
}
102103

@@ -210,8 +211,9 @@ func (c *AsyncCompleter) JobSetStateIfRunning(ctx context.Context, stats *jobsta
210211

211212
// The driver intentionally returns 0 rows when a job is deleted while the
212213
// completer is finalizing it (see UnknownJobIgnored shared driver test).
213-
// Guard against an index-out-of-range panic in that case.
214-
if len(jobs) < 1 {
214+
// Guard against an index-out-of-range panic in that case, and also skip
215+
// publishing an event when requested by the caller.
216+
if params.SkipEvent || len(jobs) < 1 {
215217
return nil
216218
}
217219

@@ -503,19 +505,24 @@ func (c *BatchCompleter) handleBatch(ctx context.Context) error {
503505

504506
var (
505507
completeTime = c.Time.Now()
506-
events = make([]CompleterJobUpdated, len(jobRows))
508+
events = make([]CompleterJobUpdated, 0, len(jobRows))
507509
)
508-
for i, jobRow := range jobRows {
510+
for _, jobRow := range jobRows {
509511
setState := setStateBatch[jobRow.ID]
512+
if setState.Params.SkipEvent {
513+
continue
514+
}
510515
setState.Stats.CompleteDuration = completeTime.Sub(setState.StartTime)
511-
events[i] = CompleterJobUpdated{
516+
events = append(events, CompleterJobUpdated{
512517
Job: jobRow,
513518
JobStats: setState.Stats,
514519
Snoozed: setState.Params.Snoozed,
515-
}
520+
})
516521
}
517522

518-
c.subscribeCh <- events
523+
if len(events) > 0 {
524+
c.subscribeCh <- events
525+
}
519526

520527
func() {
521528
c.setStateParamsMu.Lock()

internal/jobexecutor/job_executor.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,7 @@ func (e *JobExecutor) reportError(ctx context.Context, jobRow *rivertype.JobRow,
485485

486486
if softStopped {
487487
params := riverdriver.JobSetStateErrorAvailable(jobRow.ID, now, ptrutil.Ptr(max(jobRow.Attempt-1, 0)), nil, metadataUpdates)
488+
params.SkipEvent = true
488489
if err := e.Completer.JobSetStateIfRunning(ctx, e.stats, params); err != nil {
489490
e.Logger.ErrorContext(ctx, e.Name+": Failed to make soft-stopped job available", logAttrs...)
490491
}

internal/jobexecutor/job_executor_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,11 @@ func TestJobExecutor_Execute(t *testing.T) {
383383
executor.WorkUnit = newWorkUnitFactoryWithCustomRetry(func() error { return context.Canceled }, nil).MakeUnit(bundle.jobRow)
384384

385385
executor.Execute(workCtx)
386-
riversharedtest.WaitOrTimeout(t, bundle.updateCh)
386+
select {
387+
case update := <-bundle.updateCh:
388+
t.Fatalf("unexpected completion update for soft-stop cancel: %#v", update)
389+
default:
390+
}
387391

388392
job, err := bundle.exec.JobGetByID(ctx, &riverdriver.JobGetByIDParams{
389393
ID: bundle.jobRow.ID,

riverdriver/river_driver_interface.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -565,8 +565,10 @@ type JobSetStateIfRunningParams struct {
565565
MetadataUpdates []byte
566566
ScheduledAt *time.Time
567567
Schema string // added by completer
568-
Snoozed bool
569-
State rivertype.JobState
568+
// SkipEvent suppresses event publication for this transition.
569+
SkipEvent bool
570+
Snoozed bool
571+
State rivertype.JobState
570572
}
571573

572574
func JobSetStateCancelled(id int64, finalizedAt time.Time, errData []byte, metadataUpdates []byte) *JobSetStateIfRunningParams {

0 commit comments

Comments
 (0)