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

Feature/3.0.0 beta1 #25

Merged
merged 12 commits into from
Sep 4, 2022
Merged

Feature/3.0.0 beta1 #25

merged 12 commits into from
Sep 4, 2022

Conversation

m-barthelemy
Copy link
Owner

@m-barthelemy m-barthelemy commented Sep 3, 2022

QueuesFluentDriver 3.0

Breaking changes

  • When upgrading from previous versions, the existing jobs DB storage table will not be used anymore. This new version cannot process pending jobs stored by the previous releases of this package.
    In fact, after successfully upgrading, you can delete the table created by the 1.x releases of QueuesFluentDriver.
  • The ability to list the existing jobs via the (non-standard) list() method has been removed. Not sure if anyone was really using this feature; can be added back upon request.
  • This version now assumes that the jobs will always have a unique ID. When you specify a custom JobIdentifier, it is now your responsibility to ensure that you don't create another job with the same ID as a job that still exists in the QueuesFluentDriver DB tables.

    Generally, we don't recommend using custom JobIdentifiers with this package (unless you are absolutely certain that your custom values will be unique).
  • Jobs that are completed are now deleted from the database tables, instead of using Fluent's soft delete feature.
    You can opt in for the previous behavior by setting app.queues.use(.fluent(useSoftDeletes: true))

Other internal changes:

  • The format of the storage for the Queues jobs information has changed. This will ensure the storage of jobs is less error prone no matter the database (Postgres, Mysql) and the job payload/parameters.
  • The job payload is now saved to the DB as [UInt8] and skips the intermediate JSON encoding previously used; this should have a small but positive impact on performance.

@m-barthelemy m-barthelemy marked this pull request as ready for review September 4, 2022 03:45
@m-barthelemy
Copy link
Owner Author

Working on both Postgres and Mysql.

@m-barthelemy m-barthelemy merged commit e2ce677 into master Sep 4, 2022
@tonyarnold
Copy link
Contributor

The job payload is now saved to the DB as [UInt8] and skips the intermediate JSON encoding previously used; this should have a small but positive impact on performance.

This is a shame. Queued items aren't debuggable or inspectable when stored this way - I'm not sure that using JSONB all the way down was the right answer either, but at least it was easy to see details of what was queued (although this broke down when you hit the inner payload, which obviously is also stored as an array of raw integers).

@m-barthelemy
Copy link
Owner Author

The job payload is now saved to the DB as [UInt8] and skips the intermediate JSON encoding previously used; this should have a small but positive impact on performance.

This is a shame. Queued items aren't debuggable or inspectable when stored this way - I'm not sure that using JSONB all the way down was the right answer either, but at least it was easy to see details of what was queued (although this broke down when you hit the inner payload, which obviously is also stored as an array of raw integers).

Every property is now stored in a dedicated DB field, which might help if you need to see some details about a queued task, it makes them very easily queryable.
The infamous uint8 payload now also has its dedicated field, and if you use the psql cli, you might notice that its content is actually quite easy to read, even if it's not "pure JSON" 😉

@grahamburgsma
Copy link

I'm with @tonyarnold on this one. Why can't the payload be a proper jsonb type? It makes debugging very difficult even with the other properties now in dedicated columns.

@m-barthelemy
Copy link
Owner Author

I'm with @tonyarnold on this one. Why can't the payload be a proper jsonb type? It makes debugging very difficult even with the other properties now in dedicated columns.

What works with Postgres breaks on Mysql. Not sure if this is due the underlying DB type that fluent uses, or some Fluent driver issue...
The current approach used in this 3.0.0 beta has the merit of working with both databases.

If you think there could be a solution that works with at least these 2 database engines, I'm happy to hold off on this new release for a few days.

@grahamburgsma
Copy link

@m-barthelemy I just did a brief look and see why you're limited here. Queues itself only provides it as [UInt8] and you can't assume it's even an Encodable or json type. So splitting out the other properties of JobData is about all that can be done.

@tonyarnold
Copy link
Contributor

This seems much better, @m-barthelemy.

@grahamburgsma I really feel like the [UInt8] is a horrid misuse of the database types in Queues.

@tonyarnold
Copy link
Contributor

I've just suggested upstream that Data might not be the best way for Queues to offer payloads to drivers: vapor/queues#116

@m-barthelemy
Copy link
Owner Author

I don't see any easy short term fix, does that sound good to you if we release 3.0.0 as is? @tonyarnold @grahamburgsma

@tonyarnold
Copy link
Contributor

Yeah, that's unrelated to anything going on here. Postico actually shows the payload column as char*[] 😂 so that's something.

I think if you're happy with this new approach, it works for my needs.

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

Successfully merging this pull request may close these issues.

3 participants