types migration #2478
types migration #2478
Conversation
| self.migrate_anchor_leaf().await?; | ||
| self.migrate_da_proposals().await?; | ||
| self.migrate_vid_shares().await?; | ||
| self.migrate_undecided_state().await?; | ||
| self.migrate_quorum_proposals().await?; | ||
| self.migrate_quorum_certificates().await |
There was a problem hiding this comment.
Can we gate this by versions(and the other parts we start storing the new types) so we can merge this and not worry about deploying it
types/src/v0/traits.rs
Outdated
| async fn migrate_anchor_leaf(&self) -> anyhow::Result<()> { | ||
| Ok(()) | ||
| } | ||
| async fn migrate_da_proposals(&self) -> anyhow::Result<()> { | ||
| Ok(()) | ||
| } | ||
| async fn migrate_vid_shares(&self) -> anyhow::Result<()> { | ||
| Ok(()) | ||
| } | ||
| async fn migrate_undecided_state(&self) -> anyhow::Result<()> { | ||
| Ok(()) | ||
| } | ||
| async fn migrate_quorum_proposals(&self) -> anyhow::Result<()> { | ||
| Ok(()) | ||
| } | ||
| async fn migrate_quorum_certificates(&self) -> anyhow::Result<()> { | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
I think all of these should be fully implemented in the same PR
There was a problem hiding this comment.
this is default implementation. I have it implemented for SQL anf fs. I can remove this default implmentation to avoid confusion
| drop(tx); | ||
|
|
||
| if rows.is_empty() { | ||
| tracing::info!("no leaf1 rows found"); |
There was a problem hiding this comment.
Is there a weird race condition here? Say we migrate exactly limit rows and it fails during the query that sets leaf_migration.completed=true. Then when we start up we will always try to perform the migration again?
There was a problem hiding this comment.
yeah it should break here like we do in consensus migration
| let (count,) = query_as::<(i64,)>("SELECT COUNT(*) from leaf2") | ||
| .fetch_one(tx.as_mut()) | ||
| .await | ||
| .unwrap(); |
There was a problem hiding this comment.
Instead of checking the number of rows it would be better to check that the data we migrated is as expected.
There was a problem hiding this comment.
in my todo list
| let mut offset = 0; | ||
| let limit = 10000; | ||
|
|
||
| loop { |
There was a problem hiding this comment.
Lets' add some logs
- Inform about start of migration
- Inform about progress.
- Inform about completion.
|
|
||
|
|
||
| CREATE TABLE undecided_state2 ( | ||
| -- The ID is always set to 0. Setting it explicitly allows us to enforce with every insert or |
There was a problem hiding this comment.
Did you consider adding a constraint for only allowing the ID to be 0?
There was a problem hiding this comment.
can add, these tables are just copy of older ones
imabdulbasit
force-pushed
the
ab/leaf2-migration
branch
from
8f63f7f to
d8d460c
Compare
TODO: