RFC: Back osbuild-composer with a PostgreSQL database

[larskarlitski]

RFC: Back osbuild-composer with a PostgreSQL database

Motivation

osbuild-composer currently uses the file system to persist its state (using package fsjobqueue). This has served it well for its main use-case of running on a single machine with local access via the weldr API. It's easy to inspect the state on disk and it doesn't require running any additional services.

It has drawbacks when deploying it, though. Attaching volumes to stateless VMs or containers is cumbersome and error-prone. It also means that there can only be one instance of osbuild-composer accessing the state at the same time, and that no other services can access the data directly (e.g., for monitoring or reporting). Most importantly, it doesn't scale beyond a few hundred jobs.

Those are all problems that various databases have been solving for ages. osbuild-composer persisting its state to one of them would enable people using it at scale, first of all our own Image Builder service.

Release Note

This is what the release note could say if this feature is implemented

osbuild-composer's job queue can now optionally be backed by a PostgreSQL database, allowing it to scale to many more jobs and running multiple instances against the same state. README.md contains documentation about how to setup a database and configure osbuild-composer to access it.

Approach

Write a PostgreSQL backend to the JobQueue interface, using its built-in SQL extensions to implement a queue that can be concurrently accessed from multiple services.

PostgreSQL has a reputation of being very stable, while also offering extensions that make it more useful than standard SQL. In particular, it makes it possible to implement a queue without resorting to an additional queuing service. Thus, only supporting this one database is worthwhile. PostgreSQL is available in all of osbuild-composer's target environments: RHEL, Fedora, and Amazon's RDS service.

Note that solely implementing JobQueue is not enough to back all of weldr's state by a database (it serializes and additional state.json). It's possible to write this to a database as well, but considerably more work for a questionable amount of benefit - the local use case is served quite well with everything on the filesystem.

Schema Versioning

The aim is to keep all state for the Image Builder service in this database, with many stateless services accessing it. At first, these services will be multiple instances of osbuild-composer, potentially at different versions.

Thus, the database schema has to be treated with the same API stability considerations as other APIs, because it is not private to (and versioned with) a single service. In particular, it needs to stay backwards-compatible with all services that are still accessing it. For a start, only allowing accretive, non-breaking changes is enough, as long as there's a way to allow for breaking changes in the future.

Testing

To ensure osbuild-composer is not regressing, move the tests for fsjobqueue that are really testing properties of the JobQueue interface into a shared package, from where they can be run against both fsjobqueue and the new database-backed queue.

Ideally, all integration tests also run against a version of osbuild-composer that is configured with a database backend. Most importantly, the ones exercising the composer and composer-koji APIs should.

Drawbacks

Having two backends to JobQueue, thus giving users an additional choice to make, complicates deploying Image Builder for them. However, the ease of use of the filesystem-based backend in the local (weldr) scenario might make up for this.

Relatedly, this means new features (or any changes) have to be implemented in both backends. This might not be that large of an issue, because the interface is meant to be fairly stable (with all logic in the jobs themselves).

Alternatives

1. Do nothing and stick with the filesystem-based state. (Arguments against
   this are in section "Motivation".)
2. Use another database, possibly in combination with a dedicated event queue.
   (Arguments for using PostgreSQL are in section "Approach".)

Prototype

An early prototype of this work is at larskarlitski/osbuild-composer/tree/db.

[gicmo]

Ideally, all integration tests also run against a version of osbuild-composer that is configured with a database backend.

Quick thought: If there is a comprehensive test suite for the interface both backends are implementing (JobQueue?), is that really necessary?

[larskarlitski]

Yeah, JobQueue. I wouldn't call it "comprehensive", but there is a test suite for fsjobqueue. The prototype branch moves that to a shared package and also runs the tests against new dbjobqueue.

[major]

@larskarlitski Good write up on this. Adding a database backend makes sense, especially as we hope to increase redundancy and availability. Having a (mostly) stateless application also makes it much easier to manage in OpenShift.

[achilleas-k]

Drawbacks

Having two backends to JobQueue, thus giving users an additional choice to make, complicates deploying Image Builder for them. However, the ease of use of the filesystem-based backend in the local (weldr) scenario might make up for this.

This doesn't seem like a big drawback to me. I can imagine it being simple to deploy a default setup with the fsjobqueue for local deployments while having the dbjobqueue for live deployments where extra setup steps are necessary.

A related drawback I can imagine is needing to maintain both backends. The requirements are rather simple now so it doesn't seem like much of an issue but it could become cumbersome. I'm trying to think of a scenario where there's a need for a feature that requires some not-so-straightforward storage implementation and we end up needing to solve it twice—we would need to consider different performance implications or even the feasibility for the two very different bakends.

[larskarlitski]

This doesn't seem like a big drawback to me. I can imagine it being simple to deploy a default setup with the fsjobqueue for local deployments while having the dbjobqueue for live deployments where extra setup steps are necessary.

Agreed. Just thought I'd mention it for completeness' sake.

A related drawback I can imagine is needing to maintain both backends. The requirements are rather simple now so it doesn't seem like much of an issue but it could become cumbersome. I'm trying to think of a scenario where there's a need for a feature that requires some not-so-straightforward storage implementation and we end up needing to solve it twice—we would need to consider different performance implications or even the feasibility for the two very different bakends.

Good point. I'll add that to the RFC. I think it's ok for now, because I hope to not have to make any foundational changes to how this interface in the medium term.

Thanks!

[ondrejbudai]

Love it, the drawbacks are very small in comparison with the benefits.

[achilleas-k]

Another potential issue I see now is that we rely on the order of certain things in the fsjobqueue, dependencies in particular. It's always assumed that the first dependency (deps[0]) of a Finalize Job is the Init Job and the rest are Build Jobs. With the database implementation, the dependencies are in a mapping table and there's no guaranteed ordering.

I think there might be a slightly similar issue (though not a DB issue exactly) with dequeueing. When the NOTIFY goes out for a new job, the listener unblocks from the WaitForNoticiation() call and then queries the jobs to dequeue an arbitrary available job. With the job queue though it should be simple enough to order by queued_at to get around this.
This looks like it's not an issue for fsjobqueue while it's running since the jobs are added to a Go channel, but it's possible the order gets shuffled across restarts when old jobs are loaded from disk.

[larskarlitski]

We talked out of band. Update for those who weren't there:

With the database implementation, the dependencies are in a mapping table and there's no guaranteed ordering.

Valid point. While it's always ordered correctly in fsjobqueue, the database doesn't guarantee it. We decided on specifying the dependencies-are-ordered requirement in JobQueue and implement it in dbjobqueue with a order (or `index) field in the dependencies table.

With the job queue though it should be simple enough to order by queued_at to get around this.

I think the db branch currently does this. But yeah, if it doesn't, it should.

This looks like it's not an issue for fsjobqueue while it's running since the jobs are added to a Go channel, but it's possible the order gets shuffled across restarts when old jobs are loaded from disk.

Indeed. Something to fix :)

[larskarlitski]

Note from @ondrejbudai: I suggest not using index as a column name for the dependency order. Index is already a heavily used term in the db world.

[bcl]

https://www.postgresql.org/docs/current/sql-keywords-appendix.html is the list of reserved words. Helps save debugging time when your schema fails ;)

[bcl]

It's not clear to me what the motivation is, probably because I don't have the current worker architecture loaded into my head. What other system would need access to the job queue and why? Is this for workers to pick up jobs? Assuming there is a good reason for it that I'm missing, I do like having 2 backends. Maintaining a file based job queue means it is easier for potential users to setup a system to explore without needing to build out infrastructure just to see if they like it until they are ready to scale up to using a database.

Using a SQL database instead of a key-value store is probably a good idea. It keeps the options open, even though it can be slower. Some other things to think about are: encrypted communications between the db and the clients. Client auth (setup, maintenance, removal of old accounts), overall database configuration (pgsql can be optimized in a wide variety of ways, some db admins are hands on, some are not), and it also means more moving parts to debug when something goes wrong. eg. unreliable network connections, dns issues, etc.

If you are looking for go libraries and only need to support PostgreSQL, take a look at pgx and pgxpool from - https://pkg.go.dev/github.com/jackc/ which have better support for postgresql features than the more generic database/sql interface.

[larskarlitski]

What other system would need access to the job queue and why? Is this for workers to pick up jobs?

Primarily I thought about reporting tools, so that osbuild-composer doesn't need to grow complex query capabilities. @teg was also thinking about splitting the worker api off osbuild-composer, so that it can run in a different security context.

Using a SQL database instead of a key-value store is probably a good idea. It keeps the options open, even though it can be slower. Some other things to think about are: encrypted communications between the db and the clients. Client auth (setup, maintenance, removal of old accounts), overall database configuration (pgsql can be optimized in a wide variety of ways, some db admins are hands on, some are not), and it also means more moving parts to debug when something goes wrong. eg. unreliable network connections, dns issues, etc.

Good points, thanks! We're hoping that some of these choices (particularly auth and config) have already been taken by the OpenShift environment we're running in. (For example, cloud.redhat.com has ready-made configurations for PostgreSQL).

If you are looking for go libraries and only need to support PostgreSQL, take a look at pgx and pgxpool from - https://pkg.go.dev/github.com/jackc/ which have better support for postgresql features than the more generic database/sql interface.

Good that you mention this one. It's the one I've been using in the prototype, following @Gundersanne, who did the research for image-builder.

[bcl]

Thanks for the clarification.

[croissanne]

So after reading up on psql notify/listen/unlisten and skip locked I like the queue and think it's a good approach.

I guess just as a note: currently for clouddot we'd only store an aws id not owned by us. However in future this might change if we can't find similar solutions for other cloud platforms we want to support. A database like this is slightly less ephemeral than some files on a filesystem inside a container, so at some point we might need to add support for tokenizing or something like that. Not sure it would/should happen on the level of the jobqueue though.

[larskarlitski]

Thanks! Good point. What exactly do you mean by "tokenizing"?

[croissanne]

Replacing it by a token where the mapping of token -> sensitive data is stored somewhere else. But maybe just removing that data once the job is complete would also work. But hey worries for later.

[dvdhrm]

You currently rely on jobs being manually cancelled, don't you? That is, you rely on the sqlCancelJob query to complete whenever a worker crashes (which won't happen, will it?). There are multiple ways to deal with that retrospectively, but it seems counter-intuitive to me. We always might end up with stale but claimed jobs in the queue (just imagine a worker-machine crashes).

Instead of tracking claimed entries, you can just use SQL transactions. That is, you can simply change SET started_at = now() to SET handled_at = now(), but DONT close the SQL transaction, but keep it alive. Then you perform your worker actions, and when eventually your worker finished, you commit the SQL transaction. This has the effect that the single database row stays locked for the entire transaction (parallel readers will skip it), but the UPDATE is only committed if the transaction finishes. If the transaction is aborted (either manually or due to a connection failure), the UPDATE is discarded and the locks are released.

[larskarlitski]

The only type of cancelation we currently support is explicit canceling from user input. There's no concept of canceling when a worker crashes or times out. I think the right thing to do in this case is to reschedule it on another worker, but that's not implemented yet.

Instead of tracking claimed entries, you can just use SQL transactions. [...]

This is really cool! Two notes:

1. Workers do not access the database directly, but via an API on osbuild-composer. It would need to do crash detection and timing workers out. Your point still stands though, when a composer node crashes.
2. Could the row be locked for writers only, so that some other process can read for reporting status?

[dvdhrm]

The only type of cancelation we currently support is explicit canceling from user input. There's no concept of canceling when a worker crashes or times out. I think the right thing to do in this case is to reschedule it on another worker, but that's not implemented yet.

Yeah, that was my point. In composer, you create one SQL connection per worker and keep a transaction open. Once a worker disappears, you simply drop that connection. Due to the REST design, it will be hard to track workers, though. Might require watchdogs, or hard timeouts. But I think regardless whether the tracking exists or not, this would at least make sure the underlying code supports cancellation.

Instead of tracking claimed entries, you can just use SQL transactions. [...]

This is really cool! Two notes:

1. Workers do not access the database directly, but via an API on _osbuild-composer_. It would need to do crash detection and timing workers out. Your point still stands though, when a composer node crashes.

2. Could the row be locked for writers only, so that some other process can read for reporting status?

Yes, depending on the lock-type, different accesses are possible. In particular, FOR UPDATE does not block independent SELECT statements.

One downside of this approach is that it is hard to list locked rows (you can do it via SKIP LOCKED and then creating a diff, but this would take a temporary lock on all rows, so be rather inefficient). So if this is a concern, this approach would not work.

[larskarlitski]

Yeah, that was my point. [...]

Ah sorry, now I understand. This is really clever.

One downside of this approach is that it is hard to list locked rows (you can do it via SKIP LOCKED and then creating a diff, but this would take a temporary lock on all rows, so be rather inefficient). So if this is a concern, this approach would not work.

I think it might be for allowing monitoring of workers (which are running what jobs). Needs a bit more thought.

[croissanne]

Sorry to revive this after so long but I think it warrants it. A couple of thoughts:

1. So imo monitoring of workers shouldn't be the queue's responsibility. I don't like the idea of relying on SQL transactions for this purpose either, the fs- and db- queue should be interchangeable.

2. Currently worker crashes aren't handled at all, and the queue gets into an 'unsanitary' state as well (many jobs are marked as pending in the instance for crc, and they're weeks old).

   1. The easiest way to handle this is just to have a hard timeout on jobs. Not sure if this should be implemented at the queue level or at the composer/worker server/monitoring level.
   2. Introduce keep-alives/heartbeats? Definitely belongs in the 'monitoring' part and not the queue level.

I think if we consider either of these as requirements (monitoring and queue hygiene) in this work, we should fix the fsqueue before introducing a 2nd queue. I think the goal of a new queue should be parity.

[teg]

Note that we already implement one half of the heartbeat logic. The workers regularly call into composer to check if their jobs have been cancelled. We could track these calls, and (dynamically?) consider any pending job that hasn't had a ping in more than X seconds failed.

[croissanne]

So after exploring it a little and implementing a heartbeat in the worker server, I realized that just a psql jobqueue isn't enough to scale composer. There's actually quite a bit of state in the workerserver itself with the map of running jobIds to generated tokens.

The latter is a problem in and of itself, for example:

1. worker requests job from composer instance A
2. worker tries to update job at the end but hits instance B and gets a 404

So we need to either move this indirection into the queue (thus defeating the point of the indirection I think), or remove the token->id indirection all together, but I'm not sure if that's possible? Is there a reason we can't upload to artifacts/tmp/$job_id and rename it rather than use the token?

Either way fixing the worker server first takes priority over the dbqueue imo.

[teg]

Thanks for looking into this!

I agree that any sort of synchronisation/locking/state needs to happen inside the job queue, so that in the db case it will move out of composer completely.

I think the idea behind the tokens was to loosen the coupling between the "job" and the "work", if that makes sense.

In principle anything from 0 to N workers could be actually working on a job (in case we allow rescheduling on timeouts), and this way the state of each is managed independently, even if at most one of them is tied to the job at any given point (the others are just gracefully handling the various failure states).

We were also discussing that having a token that only the worker that created the job knows we should avoid any accidental issues due to misbehaving workers, as the workers are more out of our control (mostly imagining a buggy worker that hasn't been updated, at least it could only work assigned to it).

@larskarlitski might have more input on this, I remember we discussed this at quite some length and I see that the above arguments aren't really that strong, mostly based on me liking the concept of "slots" to give us some sort of RAII semantics.

[croissanne]

Right, I do think the indirection has it's uses, but it has to move from the worker server to the queue itself.

We could move the token into the db, when a job gets requested the token is generated, and this token is used for the artifact urls etc... Say the job times out, the job should simply get the token removed again. If a worker then tries to upload to an expired token, it'll simply error out.

And we then index by both token and by id, and make sure that internally 1 token will always have 1 id; and 1 id will have 0 or 1 token.

[teg]

Yeah, totally agree. Sounds perfect!