Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

publishing in a transaction #199

Closed
ibash opened this issue Oct 15, 2020 · 9 comments · May be fixed by #515
Closed

publishing in a transaction #199

ibash opened this issue Oct 15, 2020 · 9 comments · May be fixed by #515

Comments

@ibash
Copy link

ibash commented Oct 15, 2020

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
Copy link
Owner

timgit commented Oct 15, 2020

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

@gavacho
Copy link

gavacho commented Oct 16, 2020

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
Copy link
Author

ibash commented Oct 16, 2020

@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
Copy link

gavacho commented Oct 16, 2020

@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
Copy link
Author

ibash commented Oct 16, 2020

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
Copy link
Author

ibash commented Oct 16, 2020

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
Copy link

gavacho commented Oct 16, 2020

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

@ibash
Copy link
Author

ibash commented Oct 17, 2020

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

@brianmcd
Copy link
Contributor

brianmcd commented Mar 3, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants