timgit / pg-boss

Queueing jobs in Postgres from Node.js like a boss
MIT License
2.04k stars 157 forks source link

publishing in a transaction #199

Closed ibash closed 3 years ago

ibash commented 3 years ago

Hi.

It seems like it should be possible to publish jobs within a separate transaction, has anyone done this or are there examples of this?

This would be useful to do an all or nothing database insert (that is, insert a model in the database and also the job with it, or neither).

Is there a reason why this would be a bad idea?

From what I can tell all that would be needed is to allow passing a database connection as an option to publish.

timgit commented 3 years ago

Yes, this is possible, and your idea of passing a connection in an option would work

gavacho commented 3 years ago

My organization uses pg-boss's "bring your own database connection" feature described here. Our app uses slonik for its db pool and we've written an executeSql function which uses that pool

We would like to publish our jobs as part of a transaction as well.

ibash commented 3 years ago

@gavacho think the diff would just need to be. I haven't tested this yet:

diff --git a/src/manager.js b/src/manager.js
index 3c8c6fb..00dd480 100644
--- a/src/manager.js
+++ b/src/manager.js
@@ -170,6 +170,7 @@ class Manager extends EventEmitter {

   async createJob (name, data, options, singletonOffset = 0) {
     const {
+      connection,
       expireIn,
       priority,
       startAfter,
@@ -199,7 +200,7 @@ class Manager extends EventEmitter {
       keepUntil // 13
     ]

-    const result = await this.db.executeSql(this.insertJobCommand, values)
+    const result = await this.db.executeSql(this.insertJobCommand, values, { connection })

     if (result.rowCount === 1) {
       return result.rows[0].id
gavacho commented 3 years ago

@ibash I haven't tried implementing the feature but the sticky part seems to me, "what is the typescript type of the connection parameter to the publish function?"

ibash commented 3 years ago

hmm I guess it could be whatever we want, as it wouldn't used by the default pgboss db, and would only make sense when using "bring your own database connection".

I'm not super familiar with typescript, but I imagine it's an optional any for executeSql.

ibash commented 3 years ago
diff --git a/types.d.ts b/types.d.ts
index cf73c36..12c0446 100644
--- a/types.d.ts
+++ b/types.d.ts
@@ -1,6 +1,6 @@
 declare namespace PgBoss {
   interface Db {
-    executeSql(text: string, values: any[]): Promise<{ rows: any[]; rowCount: number }>;
+    executeSql(text: string, values: any[], options?: ExecutionOptions): Promise<{ rows: any[]; rowCount: number }>;
   }

   interface DatabaseOptions {
@@ -80,7 +80,11 @@ declare namespace PgBoss {
     singletonNextSlot?: boolean;
   }

-  type PublishOptions = JobOptions & ExpirationOptions & RetentionOptions & RetryOptions
+  interface ExecutionOptions {
+    connection?: any;
+  }
+
+  type PublishOptions = JobOptions & ExpirationOptions & RetentionOptions & RetryOptions & ExecutionOptions

   type ScheduleOptions = PublishOptions & { tz?: string }

I think?

gavacho commented 3 years ago

If the argument is typed as any or even unknown then my use-case is satisfied! :)

ibash commented 3 years ago

Put up a PR -- granted I haven't used it to encompass transactions yet.

brianmcd commented 3 years ago

For anyone that needs this functionality today, we've had success with just inserting jobs directly into the job table (which can be done inside of a transaction). The job table is easy to work with and has a good set of defaults, so we use our direct-insert code instead of .publish everywhere.