Skip to content

Commit c9f94c5

Browse files
committed
Add Pilot.JobRescueMany for active job rescue
This one's presented as an alternative to #1256 with its own alternative Pro implementation and which may provide some superior properties around potential bloat for table bloat in `river_job`. Changes: * Pilot picks up a new `JobRescueMany` function. In the `StandardPilot`, this just goes directly to the function of the same name in the driver's `Executor`. * We now inject `ProducerReportInterval` in `PilotInitParams`. This value is useful to an underlying pilot in knowing what age of producer row it should respect when performing a job rescue.
1 parent 1222644 commit c9f94c5

6 files changed

Lines changed: 64 additions & 6 deletions

File tree

client.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -885,9 +885,10 @@ func NewClient[TTx any](driver riverdriver.Driver[TTx], config *Config) (*Client
885885
client.pilot = &riverpilot.StandardPilot{}
886886
}
887887
client.pilot.PilotInit(archetype, (&riverpilot.PilotInitParams{
888-
Insert: client.insertMany,
889-
NotifyNonTxJobInsert: client.notifyProducerWithoutListenerJobFetch,
890-
WorkerMetadata: workerMetadata,
888+
Insert: client.insertMany,
889+
NotifyNonTxJobInsert: client.notifyProducerWithoutListenerJobFetch,
890+
ProducerReportInterval: producerReportIntervalDefault,
891+
WorkerMetadata: workerMetadata,
891892
}).Validate())
892893
pluginPilot, _ := client.pilot.(pilotPlugin)
893894

@@ -960,6 +961,7 @@ func NewClient[TTx any](driver riverdriver.Driver[TTx], config *Config) (*Client
960961
{
961962
jobRescuer := maintenance.NewRescuer(archetype, &maintenance.JobRescuerConfig{
962963
ClientRetryPolicy: config.RetryPolicy,
964+
Pilot: client.pilot,
963965
RescueAfter: config.RescueStuckJobsAfter,
964966
Schema: config.Schema,
965967
WorkUnitFactoryFunc: func(kind string) workunit.WorkUnitFactory {

client_pilot_test.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ type pilotSpyTestSignals struct {
3939
PeriodicJobGetAll testsignal.TestSignal[struct{}]
4040
PeriodicJobKeepAlive testsignal.TestSignal[struct{}]
4141
PeriodicJobUpsertMany testsignal.TestSignal[struct{}]
42-
PilotInit testsignal.TestSignal[struct{}]
42+
PilotInit testsignal.TestSignal[*riverpilot.PilotInitParams]
4343
ProducerInit testsignal.TestSignal[struct{}]
4444
ProducerKeepAlive testsignal.TestSignal[struct{}]
4545
ProducerShutdown testsignal.TestSignal[struct{}]
@@ -106,7 +106,7 @@ func (p *pilotSpy) PeriodicJobUpsertMany(ctx context.Context, exec riverdriver.E
106106

107107
func (p *pilotSpy) PilotInit(archetype *baseservice.Archetype, params *riverpilot.PilotInitParams) {
108108
p.pilotInitCalls.Add(1)
109-
p.testSignals.PilotInit.Signal(struct{}{})
109+
p.testSignals.PilotInit.Signal(params)
110110
p.StandardPilot.PilotInit(archetype, params)
111111
}
112112

@@ -150,6 +150,7 @@ func Test_Client_PilotUsage(t *testing.T) {
150150
}
151151

152152
pilot := &pilotSpy{}
153+
pilot.testSignals.Init(t)
153154
pluginDriver := newDriverWithPlugin(t, dbPool)
154155
pluginDriver.pilot = pilot
155156

@@ -187,6 +188,8 @@ func Test_Client_PilotUsage(t *testing.T) {
187188
require.NotNil(t, client)
188189
require.Equal(t, int64(1), pilot.jobCleanerQueuesExcludedCalls.Load())
189190
require.Equal(t, int64(1), pilot.pilotInitCalls.Load())
191+
pilotInitParams := pilot.testSignals.PilotInit.WaitOrTimeout()
192+
require.Equal(t, producerReportIntervalDefault, pilotInitParams.ProducerReportInterval)
190193
})
191194

192195
t.Run("JobCancelUsesPilot", func(t *testing.T) {

internal/maintenance/job_rescuer.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/riverqueue/river/riverdriver"
1515
"github.com/riverqueue/river/rivershared/baseservice"
1616
"github.com/riverqueue/river/rivershared/circuitbreaker"
17+
"github.com/riverqueue/river/rivershared/riverpilot"
1718
"github.com/riverqueue/river/rivershared/riversharedmaintenance"
1819
"github.com/riverqueue/river/rivershared/startstop"
1920
"github.com/riverqueue/river/rivershared/testsignal"
@@ -50,6 +51,9 @@ type JobRescuerConfig struct {
5051
// Interval is the amount of time to wait between runs of the rescuer.
5152
Interval time.Duration
5253

54+
// Pilot controls driver-level behavior that can be customized by plugins.
55+
Pilot riverpilot.Pilot
56+
5357
// RescueAfter is the amount of time for a job to be active before it is
5458
// considered stuck and should be rescued.
5559
RescueAfter time.Duration
@@ -70,6 +74,9 @@ func (c *JobRescuerConfig) mustValidate() *JobRescuerConfig {
7074
if c.Interval <= 0 {
7175
panic("RescuerConfig.Interval must be above zero")
7276
}
77+
if c.Pilot == nil {
78+
panic("RescuerConfig.Pilot must be set")
79+
}
7380
if c.RescueAfter <= 0 {
7481
panic("RescuerConfig.JobDuration must be above zero")
7582
}
@@ -103,12 +110,17 @@ type JobRescuer struct {
103110

104111
func NewRescuer(archetype *baseservice.Archetype, config *JobRescuerConfig, exec riverdriver.Executor) *JobRescuer {
105112
batchSizes := config.WithDefaults()
113+
pilot := config.Pilot
114+
if pilot == nil {
115+
pilot = &riverpilot.StandardPilot{}
116+
}
106117

107118
return baseservice.Init(archetype, &JobRescuer{
108119
Config: (&JobRescuerConfig{
109120
BatchSizes: batchSizes,
110121
ClientRetryPolicy: config.ClientRetryPolicy,
111122
Interval: cmp.Or(config.Interval, JobRescuerIntervalDefault),
123+
Pilot: pilot,
112124
RescueAfter: cmp.Or(config.RescueAfter, JobRescuerRescueAfterDefault),
113125
Schema: config.Schema,
114126
WorkUnitFactoryFunc: config.WorkUnitFactoryFunc,
@@ -253,7 +265,7 @@ func (s *JobRescuer) runOnce(ctx context.Context) (*rescuerRunOnceResult, error)
253265
}
254266

255267
if len(rescueManyParams.ID) > 0 {
256-
_, err = s.exec.JobRescueMany(ctx, &rescueManyParams)
268+
_, err = s.Config.Pilot.JobRescueMany(ctx, s.exec, &rescueManyParams)
257269
if err != nil {
258270
return nil, fmt.Errorf("error rescuing stuck jobs: %w", err)
259271
}

internal/maintenance/job_rescuer_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"math"
7+
"sync/atomic"
78
"testing"
89
"time"
910

@@ -14,6 +15,7 @@ import (
1415
"github.com/riverqueue/river/riverdbtest"
1516
"github.com/riverqueue/river/riverdriver"
1617
"github.com/riverqueue/river/riverdriver/riverpgxv5"
18+
"github.com/riverqueue/river/rivershared/riverpilot"
1719
"github.com/riverqueue/river/rivershared/riversharedmaintenance"
1820
"github.com/riverqueue/river/rivershared/riversharedtest"
1921
"github.com/riverqueue/river/rivershared/startstoptest"
@@ -57,6 +59,19 @@ func (p *SimpleClientRetryPolicy) NextRetry(job *rivertype.JobRow) time.Time {
5759
return job.AttemptedAt.Add(timeutil.SecondsAsDuration(retrySeconds))
5860
}
5961

62+
type jobRescueManyPilotSpy struct {
63+
riverpilot.StandardPilot
64+
65+
calls atomic.Int64
66+
params *riverdriver.JobRescueManyParams
67+
}
68+
69+
func (p *jobRescueManyPilotSpy) JobRescueMany(ctx context.Context, exec riverdriver.Executor, params *riverdriver.JobRescueManyParams) (*struct{}, error) {
70+
p.calls.Add(1)
71+
p.params = params
72+
return p.StandardPilot.JobRescueMany(ctx, exec, params)
73+
}
74+
6075
func TestJobRescuer(t *testing.T) {
6176
t.Parallel()
6277

@@ -313,6 +328,22 @@ func TestJobRescuer(t *testing.T) {
313328
riversharedtest.WaitOrTimeout(t, stopped)
314329
})
315330

331+
t.Run("UsesPilotJobRescueMany", func(t *testing.T) {
332+
t.Parallel()
333+
334+
rescuer, bundle := setup(t)
335+
pilot := &jobRescueManyPilotSpy{}
336+
rescuer.Config.Pilot = pilot
337+
338+
job := testfactory.Job(ctx, t, bundle.exec, &testfactory.JobOpts{Kind: ptrutil.Ptr(rescuerJobKind), State: ptrutil.Ptr(rivertype.JobStateRunning), AttemptedAt: ptrutil.Ptr(bundle.rescueHorizon.Add(-1 * time.Hour)), MaxAttempts: ptrutil.Ptr(5)})
339+
340+
_, err := rescuer.runOnce(ctx)
341+
require.NoError(t, err)
342+
343+
require.Equal(t, int64(1), pilot.calls.Load())
344+
require.Equal(t, []int64{job.ID}, pilot.params.ID)
345+
})
346+
316347
t.Run("CanRunMultipleTimes", func(t *testing.T) {
317348
t.Parallel()
318349

rivershared/riverpilot/pilot.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ type Pilot interface {
4141
params *riverdriver.JobInsertFastManyParams,
4242
) ([]*riverdriver.JobInsertFastResult, error)
4343

44+
JobRescueMany(ctx context.Context, exec riverdriver.Executor, params *riverdriver.JobRescueManyParams) (*struct{}, error)
45+
4446
JobRetry(ctx context.Context, exec riverdriver.Executor, params *riverdriver.JobRetryParams) (*rivertype.JobRow, error)
4547

4648
JobSetStateIfRunningMany(ctx context.Context, exec riverdriver.Executor, params *riverdriver.JobSetStateIfRunningManyParams) ([]*rivertype.JobRow, error)
@@ -75,6 +77,10 @@ type PilotInitParams struct {
7577
// latency between job insert and when a job is worked.
7678
NotifyNonTxJobInsert func(ctx context.Context, res []*rivertype.JobInsertResult)
7779

80+
// ProducerReportInterval is the amount of time between periodic reports of
81+
// producer status.
82+
ProducerReportInterval time.Duration
83+
7884
// WorkerMetadata is metadata about registered workers as received from the
7985
// client's worker bundle. Only available when a client will work jobs (i.e.
8086
// has Workers configured), so while it's safe to assume the presence of

rivershared/riverpilot/standard_pilot.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,10 @@ func (p *StandardPilot) JobInsertMany(
3939
return exec.JobInsertFastMany(ctx, params)
4040
}
4141

42+
func (p *StandardPilot) JobRescueMany(ctx context.Context, exec riverdriver.Executor, params *riverdriver.JobRescueManyParams) (*struct{}, error) {
43+
return exec.JobRescueMany(ctx, params)
44+
}
45+
4246
func (p *StandardPilot) JobRetry(ctx context.Context, exec riverdriver.Executor, params *riverdriver.JobRetryParams) (*rivertype.JobRow, error) {
4347
return exec.JobRetry(ctx, params)
4448
}

0 commit comments

Comments
 (0)