CORE-8695 Add new backchain batch size config

[nkovacsx]

Overview

We want to make sure that batch size in the backchain resolution flow is configurable. We'll use this config property to set the batch size.

[corda-jenkins-ci02]

Jenkins build for PR 1171 build 15

Build Successful:
Jar artifact version produced by this PR: 5.1.0.15-alpha-1692803901138

[kyriathar]

LGTM (introducing the initial config to the system wise)

[blsemo]

LGTM

[lankydan]

We'd need a new liquibase file for this since this is coming after 5.0? Can someone else confirm this?

[blsemo]

Good point - if it goes into 5.1, it needs a new changeset, so ideally a new file so a liquibase upgrade would work.

[kyriathar]

Good catch. @nkovacsx actually identified and pointed out the need for this to be added in ledger.utxo.1.1 instead, and I missed tha fact that it's coming in 5.1. I am not sure at all in terms of the versioning here. So it definitely needs to be in a new changeset. But where does that changeset fall under in terms of versioning? (This change seems to be backwards compatible, so maybe different file under same version?)

[nkovacsx]

@kyriathar suggested to consult with @LWogan @JamesHR3, what do you guys think?

[kyriathar]

What @JamesHR3 says below is right.

Please disregard my above answer where I managed to blend the database update with the config schema update.

[blsemo]

Please put db changes in separate liquibase change set.

[JamesHR3]

There's two interesting considerations here: the database upgrade and the config upgrade. Both schema files require some versioning.

For the DB side, I'd suggest that we use a separate changeset file per minor version increment - that makes it easier to track what changes were introduced in what version.

For the config side, I'd suggest the same thing. However, I think @nikinagy ran into a problem across upgrade whereby old versions of the config could get pushed, and so code requiring new versions of the config did not have a value it could use. I'm not sure where discussions got to around solving that.

[nkovacsx]

@JamesHR3 I understand we need to suffix the utxo-ledger-config-creation with v1.1 which makes sense. However, how would that be picked up? Do we need a new db.changelog-master.xml? Is there a plan on how these upgrade will work? I feel like there's no clear picture atm.

[nkovacsx]

I applied the suggestions made by @kyriathar and renamed the config XML to add-ledger-utxo-config.xml without a version and added it to db.changelog-master.xml.

[lankydan]

I applied the suggestions made by @kyriathar and renamed the config XML to add-ledger-utxo-config.xml without a version and added it to db.changelog-master.xml.

Based off the comments, I'm still not sure if the change you made is correct. Can @JamesHR3 @LWogan @kyriathar jump in here? Who officially owns the config part of the code, we should make sure this is documented because its not clear what is supposed to happen here.

[lankydan]

@blsemo @driessamyn Does anyone know who owns configuration? This PR has been blocked for a couple of weeks now and it is still not clear if @nkovacsx is doing the right thing or not (at least from my perspective).

We can merge our change as it is, but if it's wrong, it is unlikely to ever be corrected unless the owner of configuration knows to deal with it.

If we can't reach a decision on this in the coming week, then I suggest we close this PR and @nkovacsx can hardcode the appropriate default batch size directly into the code and then we'll raise a ticket to address the configuration part whenever we receive guidance on how to achieve this.

[driessamyn]

@blsemo @driessamyn Does anyone know who owns configuration? This PR has been blocked for a couple of weeks now and it is still not clear if @nkovacsx is doing the right thing or not (at least from my perspective).

We can merge our change as it is, but if it's wrong, it is unlikely to ever be corrected unless the owner of configuration knows to deal with it.

If we can't reach a decision on this in the coming week, then I suggest we close this PR and @nkovacsx can hardcode the appropriate default batch size directly into the code and then we'll raise a ticket to address the configuration part whenever we receive guidance on how to achieve this.

@lankydan , best to check with @simon-johnson-r3 about the implications on upgrade.
I think we have a todo item to publish some guidance.

[blsemo]

LGTM

[simon-johnson-r3]

Hi, sorry for the late reply, I've been trying to gather background on some of our file structures and reasoning behind them.

I've created a document here, it's a bit rushed right now, but it should detail the information you need on the database side and it will be our process going forward: https://r3-cev.atlassian.net/wiki/spaces/CB/pages/4562256029/Corda+5+Upgrade+Developer+Knowledge+Base+-+Making+Database+Schema+Changes

On the Config side, for now there is no document (it's WIP) however I believe what you have here is in line with our thinking. That is you can create new config, you must assign default values (such that old config from before any upgrade can be assigned a value) but cannot delete or alter existing config at all to maintain compatibility from previous versions.

So as far as this review goes, you will need to make a minor change on the db side only. Please refer to the doc on that - part of the reason I'm not spelling it out here is so you can let me know if the document doesn't make sense anywhere.

[simon-johnson-r3]

See general comment.