High Availability - sync over DB, multiple process instances support #718
Conversation
| @@ -0,0 +1,25 @@ | |||
| -- Create ENUM for prover type | |||
| CREATE TYPE batch_type_enum AS ENUM ('Insertion', 'Deletion'); | |||
There was a problem hiding this comment.
Postgres enums are tricky as you can't add a variant to them later down the line.
I would suggest changing it to a number or string.
Another option is a table with allowed values but that's a bit overkill imo.
There was a problem hiding this comment.
I have seen we were using it for "prover_type" (file 008_prover_type.sql). What we prefer? Do it like for prover or just use String field and parse on rust side?
There was a problem hiding this comment.
Let's use string here I think. There's a very real chance we'll add an Update batch type in the future and that's a breaking change if we're using enums.
| async fn get_next_batch(self, prev_root: &Hash) -> Result<Option<BatchEntry>, Error> { | ||
| let res = sqlx::query_as::<_, BatchEntry>( | ||
| r#" | ||
| SELECT * FROM batches WHERE prev_root = $1 |
There was a problem hiding this comment.
Consider explicitly listing all fetched columns here.
I think the * might break if we're deploying a new version of a sequencer with a DB schema update as the older version will try to fetch columns that it doesn't know how to parse.
By listing all columns explicitly we'll be able to add columns without breaking previous sequencer versions.
There was a problem hiding this comment.
Also applies to other usages of *
Motivation
It is required to run signup-sequencer with HA.
Solution
Sync workers over DB.
PR Checklist