Splitstore Review

[Stebalien]

Reviewing #6474

Unfortunately, github makes it difficult to review rewrites because the patches are nasty. So I'm just adding comments to the code directly.

To respond, please review this PR. I'll continue to push new comments inline.

[Stebalien]

Have we tried exercising this code by syncing the chain? I assume most of our "realistic" tests have been one offs so it would be nice to have a test that rapidly and repeatedly purges.

[vyzo]

Have we tried exercising this code by syncing the chain? I assume most of our "realistic" tests have been one offs so it would be nice to have a test that rapidly and repeatedly purges.

Yes, I have two live nodes running latest code for about a week now, one with discard and the other with a coldstore. Both are syncing fine and exhibit no ill effects, with block validation time unaffected by a running compaction.

However, as we've been discussing with @raulk, we do need a harness test so that we can reliably and reproducibly do automated testing. In my opinion, this is critical and necessary before we declare the code production ready. I will create follow up issue as it is unlikely we'll get to it before code freeze for v1.11.1.

[vyzo]

Addressed review comments in 0c5e336

And fixed a bug a lint complaint made me see in df9670c

[vyzo]

Created follow up issue for testing in #6725
It is not urgent, but we definitely need to resolve this in the next few months.

[Stebalien]

Ok, final final review.

[jacobheun]

Closing this as #6474 was merged.