Skip to content

Commit

Permalink
Error on batch insert if InsertOpts.UniqueOpts is specified
Browse files Browse the repository at this point in the history
As I was writing docs for batch insertion today [1], I remembered that
we don't support job uniqueness on batch inserts because the mechanism
uses PG advisory locks, and holding many locks at once could easily lead
to contention and deadlocks.

We may remove this limitation in the future, but I figure that in the
meantime, it might not be the worst idea to error if they're specified
on batch insert so as not to mislead the user that what they expected to
happen happened. That's what this change does.

[1] https://riverqueue.com/docs/batch-job-insertion
  • Loading branch information
brandur committed Nov 11, 2023
1 parent 250ce47 commit 3d679e4
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
24 changes: 24 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1047,6 +1047,10 @@ func (c *Client[TTx]) InsertMany(ctx context.Context, params []InsertManyParams)
return 0, err
}

if err := c.validateManyInsertOpts(param.InsertOpts); err != nil {
return 0, err
}

var err error
insertParams[i], err = insertParamsFromArgsAndOptions(param.Args, param.InsertOpts)
if err != nil {
Expand Down Expand Up @@ -1087,6 +1091,10 @@ func (c *Client[TTx]) InsertManyTx(ctx context.Context, tx TTx, params []InsertM
return 0, err
}

if err := c.validateManyInsertOpts(param.InsertOpts); err != nil {
return 0, err
}

var err error
insertParams[i], err = insertParamsFromArgsAndOptions(param.Args, param.InsertOpts)
if err != nil {
Expand All @@ -1113,6 +1121,22 @@ func (c *Client[TTx]) validateJobArgs(args JobArgs) error {
return nil
}

// Validates insertion options for to insertion, but for batch insertions only.
// Currently, verifies only that UniqueOpts aren't used for batch inserts.
// They're not supported because they use PG advisory locks to work, and taking
// many locks simultaneously could easily lead to contention and deadlocks.
func (c *Client[TTx]) validateManyInsertOpts(insertOpts *InsertOpts) error {
if insertOpts == nil {
return nil
}

if !insertOpts.UniqueOpts.isEmpty() {
return errors.New("UniqueOpts are not supported for batch inserts")
}

return nil
}

var nameRegex = regexp.MustCompile(`^(?:[a-z0-9])+(?:_?[a-z0-9]+)*$`)

func validateQueueName(queueName string) error {
Expand Down
24 changes: 24 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,18 @@ func Test_Client_InsertMany(t *testing.T) {
})
require.NoError(t, err)
})

t.Run("ErrorsOnInsertOptsUniqueOpts", func(t *testing.T) {
t.Parallel()

client, _ := setup(t)

count, err := client.InsertMany(ctx, []InsertManyParams{
{Args: noOpArgs{}, InsertOpts: &InsertOpts{UniqueOpts: UniqueOpts{ByArgs: true}}},
})
require.EqualError(t, err, "UniqueOpts are not supported for batch inserts")
require.Equal(t, int64(0), count)
})
}

func Test_Client_InsertManyTx(t *testing.T) {
Expand Down Expand Up @@ -809,6 +821,18 @@ func Test_Client_InsertManyTx(t *testing.T) {
})
require.NoError(t, err)
})

t.Run("ErrorsOnInsertOptsUniqueOpts", func(t *testing.T) {
t.Parallel()

client, bundle := setup(t)

count, err := client.InsertManyTx(ctx, bundle.tx, []InsertManyParams{
{Args: noOpArgs{}, InsertOpts: &InsertOpts{UniqueOpts: UniqueOpts{ByArgs: true}}},
})
require.EqualError(t, err, "UniqueOpts are not supported for batch inserts")
require.Equal(t, int64(0), count)
})
}

func Test_Client_ErrorHandler(t *testing.T) {
Expand Down

0 comments on commit 3d679e4

Please sign in to comment.