Skip to content

Commit accaa41

Browse files
committed
preserve internal queue metadata
Rate-limit rollups need durable queue-scoped storage without exposing runtime state through River's public metadata APIs or allowing ordinary queue updates to erase it. Reserve top-level `river:*` metadata keys, preserve them atomically across user metadata replacement, and filter them from public reads, notifications, and producer polling. Wrap non-object user metadata only when runtime keys must coexist with it so the public representation remains unchanged. Retain queues while a rollup has unexpired debt, lock cleanup candidates before deletion, and use one database-relative timestamp for each cleanup pass. Pass River's existing producer stale horizon through the pilot boundary so Pro can apply the same liveness definition to concurrency and retained rate debt. Expose shared transaction advisory locks through the driver seam so unlimited fetches can inspect configuration concurrently while producer lifecycle changes and limited fetches coordinate through the exclusive counterpart.
1 parent 4aa884e commit accaa41

23 files changed

Lines changed: 587 additions & 45 deletions

client.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,13 @@ const (
4848
FetchPollIntervalDefault = 1 * time.Second
4949
FetchPollIntervalMin = 1 * time.Millisecond
5050

51-
JobStuckThresholdDefault = 10 * time.Second
52-
JobTimeoutDefault = 1 * time.Minute
53-
MaxAttemptsDefault = rivercommon.MaxAttemptsDefault
54-
PriorityDefault = rivercommon.PriorityDefault
55-
QueueDefault = rivercommon.QueueDefault
56-
QueueNumWorkersMax = 10_000
51+
JobStuckThresholdDefault = 10 * time.Second
52+
JobTimeoutDefault = 1 * time.Minute
53+
MaxAttemptsDefault = rivercommon.MaxAttemptsDefault
54+
PriorityDefault = rivercommon.PriorityDefault
55+
producerStaleRetentionPeriodDefault = 5 * time.Minute
56+
QueueDefault = rivercommon.QueueDefault
57+
QueueNumWorkersMax = 10_000
5758
)
5859

5960
var (
@@ -895,9 +896,10 @@ func NewClient[TTx any](driver riverdriver.Driver[TTx], config *Config) (*Client
895896
client.pilot = &riverpilot.StandardPilot{}
896897
}
897898
client.pilot.PilotInit(archetype, (&riverpilot.PilotInitParams{
898-
Insert: client.insertMany,
899-
NotifyNonTxJobInsert: client.notifyProducerWithoutListenerJobFetch,
900-
WorkerMetadata: workerMetadata,
899+
Insert: client.insertMany,
900+
NotifyNonTxJobInsert: client.notifyProducerWithoutListenerJobFetch,
901+
ProducerStaleRetentionPeriod: producerStaleRetentionPeriodDefault,
902+
WorkerMetadata: workerMetadata,
901903
}).Validate())
902904
pluginPilot, _ := client.pilot.(pilotPlugin)
903905

@@ -2301,7 +2303,7 @@ func (c *Client[TTx]) producerAdd(queueName string, queueConfig QueueConfig) (*p
23012303
RetryPolicy: c.config.RetryPolicy,
23022304
SchedulerInterval: c.config.schedulerInterval,
23032305
Schema: c.config.Schema,
2304-
StaleProducerRetentionPeriod: 5 * time.Minute,
2306+
StaleProducerRetentionPeriod: producerStaleRetentionPeriodDefault,
23052307
Workers: c.config.Workers,
23062308
})
23072309
c.producersByQueueName[queueName] = producer
@@ -2890,7 +2892,7 @@ func (c *Client[TTx]) queueUpdate(ctx context.Context, executorTx riverdriver.Ex
28902892

28912893
controlEvent := &controlEventPayload{
28922894
Action: controlActionMetadataChanged,
2893-
Metadata: params.Metadata,
2895+
Metadata: queue.Metadata,
28942896
Queue: queue.Name,
28952897
}
28962898

client_pilot_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ type pilotSpy struct {
2929
jobInsertManyCalls atomic.Int64
3030
jobRetryCalls atomic.Int64
3131
pilotInitCalls atomic.Int64
32+
pilotInitParams *riverpilot.PilotInitParams
3233

3334
testSignals pilotSpyTestSignals
3435
}
@@ -106,6 +107,7 @@ func (p *pilotSpy) PeriodicJobUpsertMany(ctx context.Context, exec riverdriver.E
106107

107108
func (p *pilotSpy) PilotInit(archetype *baseservice.Archetype, params *riverpilot.PilotInitParams) {
108109
p.pilotInitCalls.Add(1)
110+
p.pilotInitParams = params
109111
p.testSignals.PilotInit.Signal(struct{}{})
110112
p.StandardPilot.PilotInit(archetype, params)
111113
}
@@ -325,6 +327,7 @@ func Test_Client_PilotUsage(t *testing.T) {
325327
require.NotZero(t, insertRes.Job.ID)
326328

327329
pilot.testSignals.JobGetAvailable.WaitOrTimeout()
330+
require.Equal(t, 5*time.Minute, pilot.pilotInitParams.ProducerStaleRetentionPeriod)
328331
pilot.testSignals.JobSetStateIfRunningMany.WaitOrTimeout()
329332
pilot.testSignals.ProducerInit.WaitOrTimeout()
330333
pilot.testSignals.ProducerKeepAlive.WaitOrTimeout()

client_test.go

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6348,6 +6348,32 @@ func Test_Client_QueueGet(t *testing.T) {
63486348
require.Nil(t, queueRes.PausedAt)
63496349
})
63506350

6351+
t.Run("HidesReservedMetadata", func(t *testing.T) {
6352+
t.Parallel()
6353+
6354+
client, bundle := setup(t)
6355+
exec := client.driver.GetExecutor()
6356+
6357+
queue := testfactory.Queue(ctx, t, exec, &testfactory.QueueOpts{
6358+
Metadata: []byte(`{"foo":"bar"}`),
6359+
Schema: bundle.schema,
6360+
})
6361+
err := exec.QueryRow(ctx,
6362+
`UPDATE `+dbutil.SafeIdentifier(bundle.schema)+`.river_queue SET metadata = metadata || '{"river:rate_limit_rollup":{"version":1}}' WHERE name = $1 RETURNING 1`,
6363+
queue.Name,
6364+
).Scan(new(int))
6365+
require.NoError(t, err)
6366+
6367+
queueRes, err := client.QueueGet(ctx, queue.Name)
6368+
require.NoError(t, err)
6369+
require.JSONEq(t, `{"foo":"bar"}`, string(queueRes.Metadata))
6370+
6371+
listRes, err := client.QueueList(ctx, NewQueueListParams())
6372+
require.NoError(t, err)
6373+
require.Len(t, listRes.Queues, 1)
6374+
require.JSONEq(t, `{"foo":"bar"}`, string(listRes.Queues[0].Metadata))
6375+
})
6376+
63516377
t.Run("ReturnsErrNotFoundIfQueueDoesNotExist", func(t *testing.T) {
63526378
t.Parallel()
63536379

@@ -6606,9 +6632,14 @@ func Test_Client_QueueUpdate(t *testing.T) {
66066632

66076633
queue := testfactory.Queue(ctx, t, client.driver.GetExecutor(), &testfactory.QueueOpts{Schema: bundle.schema})
66086634
require.Equal(t, []byte(`{}`), queue.Metadata)
6635+
err = client.driver.GetExecutor().QueryRow(ctx,
6636+
`UPDATE `+dbutil.SafeIdentifier(bundle.schema)+`.river_queue SET metadata = metadata || '{"river:rate_limit_rollup":{"version":1}}' WHERE name = $1 RETURNING 1`,
6637+
queue.Name,
6638+
).Scan(new(int))
6639+
require.NoError(t, err)
66096640

66106641
queue, err = client.QueueUpdate(ctx, queue.Name, &QueueUpdateParams{
6611-
Metadata: []byte(`{"foo":"bar"}`),
6642+
Metadata: []byte(`{"foo":"bar","river:attempted_override":true}`),
66126643
})
66136644
require.NoError(t, err)
66146645
require.JSONEq(t, `{"foo":"bar"}`, string(queue.Metadata))
@@ -6621,6 +6652,18 @@ func Test_Client_QueueUpdate(t *testing.T) {
66216652
Queue: queue.Name,
66226653
}, notif.payload)
66236654

6655+
var (
6656+
attemptedOverrideIsNull bool
6657+
rollup []byte
6658+
)
6659+
err = client.driver.GetExecutor().QueryRow(ctx,
6660+
`SELECT metadata -> 'river:rate_limit_rollup', metadata -> 'river:attempted_override' IS NULL FROM `+dbutil.SafeIdentifier(bundle.schema)+`.river_queue WHERE name = $1`,
6661+
queue.Name,
6662+
).Scan(&rollup, &attemptedOverrideIsNull)
6663+
require.NoError(t, err)
6664+
require.JSONEq(t, `{"version":1}`, string(rollup))
6665+
require.True(t, attemptedOverrideIsNull)
6666+
66246667
queue, err = client.QueueUpdate(ctx, queue.Name, &QueueUpdateParams{
66256668
Metadata: []byte(`{"foo":"baz"}`),
66266669
})

internal/maintenance/queue_cleaner.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,12 @@ func (s *QueueCleaner) runOnce(ctx context.Context) (*queueCleanerRunOnceResult,
161161
ctx, cancelFunc := context.WithTimeout(ctx, riversharedmaintenance.TimeoutDefault)
162162
defer cancelFunc()
163163

164+
now := s.Time.Now()
164165
queuesDeleted, err := s.exec.QueueDeleteExpired(ctx, &riverdriver.QueueDeleteExpiredParams{
165166
Max: s.batchSize(),
167+
Now: &now,
166168
Schema: s.Config.Schema,
167-
UpdatedAtHorizon: time.Now().Add(-s.Config.RetentionPeriod),
169+
UpdatedAtHorizon: now.Add(-s.Config.RetentionPeriod),
168170
})
169171
if err != nil {
170172
return nil, fmt.Errorf("error deleting expired queues: %w", err)

internal/maintenance/queue_cleaner_test.go

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package maintenance
33
import (
44
"context"
55
"fmt"
6+
"sync/atomic"
67
"testing"
78
"time"
89

@@ -19,6 +20,36 @@ import (
1920
"github.com/riverqueue/river/rivertype"
2021
)
2122

23+
type queueCleanerExecutorMock struct {
24+
riverdriver.Executor
25+
26+
queueDeleteExpiredFunc func(ctx context.Context, params *riverdriver.QueueDeleteExpiredParams) ([]string, error)
27+
}
28+
29+
func (m *queueCleanerExecutorMock) QueueDeleteExpired(ctx context.Context, params *riverdriver.QueueDeleteExpiredParams) ([]string, error) {
30+
return m.queueDeleteExpiredFunc(ctx, params)
31+
}
32+
33+
type queueCleanerTimeGenerator struct {
34+
now time.Time
35+
nowCalls atomic.Int32
36+
}
37+
38+
func (g *queueCleanerTimeGenerator) Now() time.Time {
39+
g.nowCalls.Add(1)
40+
return g.now
41+
}
42+
43+
func (g *queueCleanerTimeGenerator) NowOrNil() *time.Time {
44+
now := g.Now()
45+
return &now
46+
}
47+
48+
func (g *queueCleanerTimeGenerator) StubNow(now time.Time) time.Time {
49+
g.now = now
50+
return now
51+
}
52+
2253
func TestQueueCleaner(t *testing.T) {
2354
t.Parallel()
2455

@@ -71,6 +102,34 @@ func TestQueueCleaner(t *testing.T) {
71102
startstoptest.Stress(ctx, t, cleaner)
72103
})
73104

105+
t.Run("UsesOneNowForExpiration", func(t *testing.T) {
106+
t.Parallel()
107+
108+
cleaner, bundle := setup(t)
109+
now := time.Date(2026, time.July, 10, 12, 0, 0, 0, time.UTC)
110+
timeGenerator := &queueCleanerTimeGenerator{now: now}
111+
112+
var capturedParams *riverdriver.QueueDeleteExpiredParams
113+
cleaner.Time = timeGenerator
114+
cleaner.exec = &queueCleanerExecutorMock{
115+
Executor: bundle.exec,
116+
queueDeleteExpiredFunc: func(ctx context.Context, params *riverdriver.QueueDeleteExpiredParams) ([]string, error) {
117+
paramsCopy := *params
118+
capturedParams = &paramsCopy
119+
return nil, nil
120+
},
121+
}
122+
123+
res, err := cleaner.runOnce(ctx)
124+
require.NoError(t, err)
125+
require.Empty(t, res.QueuesDeleted)
126+
require.Equal(t, int32(1), timeGenerator.nowCalls.Load())
127+
require.NotNil(t, capturedParams)
128+
require.NotNil(t, capturedParams.Now)
129+
require.Equal(t, now, *capturedParams.Now)
130+
require.Equal(t, now.Add(-cleaner.Config.RetentionPeriod), capturedParams.UpdatedAtHorizon)
131+
})
132+
74133
t.Run("DeletesExpiredQueues", func(t *testing.T) {
75134
t.Parallel()
76135

internal/rivercommon/river_common.go

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package rivercommon
22

33
import (
4+
"encoding/json"
45
"errors"
56
"regexp"
7+
"strings"
68
"time"
79
)
810

@@ -12,12 +14,48 @@ import (
1214
const (
1315
// AllQueuesString is a special string that can be used to indicate all
1416
// queues in some operations, particularly pause and resume.
15-
AllQueuesString = "*"
16-
MaxAttemptsDefault = 25
17-
PriorityDefault = 1
18-
QueueDefault = "default"
17+
AllQueuesString = "*"
18+
MaxAttemptsDefault = 25
19+
PriorityDefault = 1
20+
QueueDefault = "default"
21+
QueueMetadataKeyUserMetadata = "river:user_metadata"
1922
)
2023

24+
// QueueMetadataForUserWrite removes reserved keys from user-supplied queue
25+
// metadata while preserving every nonreserved value.
26+
func QueueMetadataForUserWrite(metadata []byte) []byte {
27+
var object map[string]json.RawMessage
28+
if err := json.Unmarshal(metadata, &object); err != nil {
29+
return metadata
30+
}
31+
32+
for key := range object {
33+
if strings.HasPrefix(key, "river:") {
34+
delete(object, key)
35+
}
36+
}
37+
38+
metadataWithoutReserved, err := json.Marshal(object)
39+
if err != nil {
40+
return metadata
41+
}
42+
return metadataWithoutReserved
43+
}
44+
45+
// QueueMetadataWithoutReserved removes River's reserved top-level metadata
46+
// keys before queue metadata crosses a public API boundary. A wrapped
47+
// non-object user value is restored to its original representation.
48+
func QueueMetadataWithoutReserved(metadata []byte) []byte {
49+
var object map[string]json.RawMessage
50+
if err := json.Unmarshal(metadata, &object); err != nil {
51+
return metadata
52+
}
53+
if userMetadata, ok := object[QueueMetadataKeyUserMetadata]; ok {
54+
return userMetadata
55+
}
56+
return QueueMetadataForUserWrite(metadata)
57+
}
58+
2159
// HotOperationTimeout attempts to standardize timeouts for some "hot"
2260
// operations like locking available jobs or completing finished jobs. It's
2361
// somewhat questionable whether it makes sense to share timing on these

internal/rivercommon/river_common_test.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,15 @@ func TestJobKindRE(t *testing.T) {
2525
require.NotRegexp(t, UserSpecifiedIDOrKindRE, "with,comma")
2626
require.NotRegexp(t, UserSpecifiedIDOrKindRE, ":no_leading_special_characters")
2727
}
28+
29+
func TestQueueMetadataWithoutReserved(t *testing.T) {
30+
t.Parallel()
31+
32+
require.JSONEq(t,
33+
`{"user":"value"}`,
34+
string(QueueMetadataWithoutReserved([]byte(`{"river:internal":{"state":1},"user":"value"}`))),
35+
)
36+
require.Equal(t, []byte(`not json`), QueueMetadataWithoutReserved([]byte(`not json`)))
37+
require.JSONEq(t, `[1,2]`, string(QueueMetadataWithoutReserved([]byte(`{"river:user_metadata":[1,2],"river:internal":{}}`))))
38+
require.JSONEq(t, `{"user":"value"}`, string(QueueMetadataForUserWrite([]byte(`{"river:internal":{},"user":"value"}`))))
39+
}

producer_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/riverqueue/river/rivershared/riversharedtest"
2727
"github.com/riverqueue/river/rivershared/startstoptest"
2828
"github.com/riverqueue/river/rivershared/testfactory"
29+
"github.com/riverqueue/river/rivershared/util/dbutil"
2930
"github.com/riverqueue/river/rivershared/util/ptrutil"
3031
"github.com/riverqueue/river/rivershared/util/randutil"
3132
"github.com/riverqueue/river/rivershared/util/testutil"
@@ -745,6 +746,22 @@ func testProducer(t *testing.T, makeProducer func(ctx context.Context, t *testin
745746
producer.config.QueuePollInterval = 50 * time.Millisecond
746747

747748
startProducer(t, ctx, ctx, producer)
749+
if producer.config.Notifier == nil {
750+
producer.testSignals.PolledQueueConfig.WaitOrTimeout()
751+
752+
schemaAndTable := "river_queue"
753+
if producer.config.Schema != "" {
754+
schemaAndTable = dbutil.SafeIdentifier(producer.config.Schema) + ".river_queue"
755+
}
756+
err := bundle.exec.QueryRow(ctx,
757+
`UPDATE `+schemaAndTable+` SET metadata = metadata || '{"river:rate_limit_rollup":{"version":1}}' WHERE name = $1 RETURNING 1`,
758+
producer.config.Queue,
759+
).Scan(new(int))
760+
require.NoError(t, err)
761+
762+
producer.testSignals.PolledQueueConfig.WaitOrTimeout()
763+
producer.testSignals.MetadataChanged.RequireEmpty()
764+
}
748765

749766
updateMetadata := func(newMetadata []byte) {
750767
t.Helper()

riverdriver/river_driver_interface.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,7 @@ type Executor interface {
274274

275275
NotifyMany(ctx context.Context, params *NotifyManyParams) error
276276
PGAdvisoryXactLock(ctx context.Context, key int64) (*struct{}, error)
277+
PGAdvisoryXactLockShared(ctx context.Context, key int64) (*struct{}, error)
277278

278279
QueueCreateOrSetUpdatedAt(ctx context.Context, params *QueueCreateOrSetUpdatedAtParams) (*rivertype.Queue, error)
279280
QueueDeleteExpired(ctx context.Context, params *QueueDeleteExpiredParams) ([]string, error)
@@ -814,6 +815,7 @@ type QueueCreateOrSetUpdatedAtParams struct {
814815

815816
type QueueDeleteExpiredParams struct {
816817
Max int
818+
Now *time.Time
817819
Schema string
818820
UpdatedAtHorizon time.Time
819821
}

riverdriver/riverdatabasesql/internal/dbsqlc/pg_misc.sql.go

Lines changed: 9 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)