m-barthelemy / vapor-queues-fluent-driver

A Fluent implementation for https://github.com/vapor/queues (Vapor4)
MIT License
32 stars 16 forks source link

Feature/3.0.0 beta1 #25

Closed m-barthelemy closed 2 years ago

m-barthelemy commented 2 years ago

QueuesFluentDriver 3.0

Breaking changes

Other internal changes:

m-barthelemy commented 2 years ago

Working on both Postgres and Mysql.

tonyarnold commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

This seems much better, @m-barthelemy.

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

tonyarnold commented 2 years ago

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

m-barthelemy commented 2 years ago

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 commented 2 years ago

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.