refactor: improve snapshot generation performance

[seanmcgary]

Makes all snapshot tables long-lived with proper unique constraints.

Previously, snapshot tables were dropped (if they existed) and re-generated for a given snapshot date. This results in a lot of data being generated and thrown out for no reason. It also meant that we couldnt put proper indexes on these tables since they would disappear when the table was dropped and re-created.

[jbrower95]

generally looks good. I need to spend some more time reading up on the sidecar for further reviews.

[jbrower95]

Since you're changing queries, or insert logic -- will this be re-audited?

[seanmcgary]

@jbrower95

Since you're changing queries, or insert logic -- will this be re-audited?

functionally it should behave exactly the same (in fact, this is actually much safer than how it is now since it has proper unique constraints in place...). Im going to regression test it with a full mainnet sync to ensure thats the case before merging it. If it can sync from scratch and reconstitute every root, that's good enough for me.

@anupsv thoughts on this?

[anupsv]

net sync to ensure thats the case before merging it. If it can sync from scratch and reconstitute every root, that's good enough for me.

I was thinking if we could include this small change in the rewards 2.1 audit thats going on to be on the safer side. But yes, we definitely need (if we don't have) tests to cover the invariants that should hold for this table. Since a regression test is being done its good. But also @seanmcgary do we have an e2e test where it tries to ingest the latest database and see if all the roots match up ?

[seanmcgary]

Yeah I can see if the OpenBlocks guys can take a pass at this particular change as a sanity check.

do we have an e2e test where it tries to ingest the latest database and see if all the roots match up ?

Not an automated one yet (it's on my list to do...). For large-ish changes like this I just manually start up a new mainnet sidecar and sync from EL genesis. We do have a test suite for rewards generation that you can run locally, but it only does the first like 5 roots from mainnet, so its decent as a sanity check, but not 100% comprehensive like re-syncing from scratch.

[anupsv]

Yeah I can see if the OpenBlocks guys can take a pass at this particular change as a sanity check.

do we have an e2e test where it tries to ingest the latest database and see if all the roots match up ?

Not an automated one yet (it's on my list to do...). For large-ish changes like this I just manually start up a new mainnet sidecar and sync from EL genesis. We do have a test suite for rewards generation that you can run locally, but it only does the first like 5 roots from mainnet, so its decent as a sanity check, but not 100% comprehensive like re-syncing from scratch.

I see. Can we add the automated test in like the next PR or so? With more changes coming in, it'll be really helpful in getting confidence that the changes didn't brick things.

[seanmcgary]

Id like to get it in before the slashing release...Im like halfway through setting up self-hosted github runners to handle it (more horsepower & flexibility with creating the correct test environment).