How to do Housekeeping

[da-kami]

We have some cases where CFDs should get cleaned up because their state is causing trouble.

Cases that require cleanup because the CFDs remain in "the taker UI forever" (and remains in cfds table forever):

1. CFD never made it into contact setup (we fail before the initial event is appended) - Cfd stale in Pending Setup if taker has Insufficient Funds error  #2127
2. CFD with ContractSetupStarted but no failed event (but the contract setup failed) - Testnet position remains in ContractSetup "forever" #1987

Cases that could be cleaned up because we are not in a proper state:

3. CFD with RolloverStarted but no failed event (but the rollover failed)
   • Are currently "cleaned up" by appending a failed event if we fail to start a rollover.
4. CFD with CollaborativeSettlementStarted but no failed event (but the collab settlement failed)
   • Currently problematic because the protocol is not symmetric, i.e. the taker considers the collab settlement successful after the maker accepted regardless of the transaction publication.

Note that special cases where CFDs are not in a correct state are not handled here but in separate tickets. This discussion concerns cases where we started something, but did not finish in a state that the application can handle properly.

General recommendations:

• Rollout symmetric collab settlement protocol before adding cleanup to collab settlement
• Add a creation_timestamp to the cfds (and the closed/failed cfd tables) before adding cleanup to CFDs that don't have any events

[da-kami]

There are patch solutions for some individual problems, but it would be great to have a more general solution that ensures our current state is sane given the history of the CFD.

One general solution was outlined by #1317

The idea is to go over CFD states and decide to add a xxxFailed event if the last event recorded is not sane or the CFD has not events.
If we do this upon startup it is easy, because at startup we can always assume that a protocol that was started cannot be finished anymore.

However, it would be great to be able to run this task periodically while the application is running and not just upon startup. This requires time boundaries to decide if a CFD is in a problematic state or not. We have to decide how much time a CFD that was just created or a protocol that was just started should have until we consider it failed and add a corresponding event.

Constraints:

• CFDs that have no events attached requires the CFD to have a creation timestamp. Otherwise we cannot make a time based decision if a CFD is already considered failed. This was outlined in Cleanup CFDs without events when handling failed CFDs #1997
• Each protocol needs a time interval after which we consider the protocol to be failed.

[luckysori]

• I like the idea of appending xxxFailed where needed at an interval, as proposed here in ADR016.
• I'm okay with adding a creation timestamp to cfds to be able to remove trivial CFDs periodically rather than just on startup.
• I'm not sure I understand what you mean by "general solution". I think running all these housekeeping tasks separately (like we do with cull_old_dlcs) should be acceptable.

[luckysori]

I created #2134 to showcase how we can delete trivial CFDs on startup without changes to the DB tables. I propose to merge that now and iterate on it based on the outcome of this discussion.

[da-kami]

[snip]

* I'm not sure I understand what you mean by "general solution". I think running all these housekeeping tasks separately (like we do with `cull_old_dlcs`) should be acceptable.

What I mean is, that I would like to have an overall strategy how to deal with problematic CFDs instead if having all kinds of "cleanup tasks" for different cases. On that note I am not sure I agree with #2134 because we would be able to just handle it with appending ContractSetupFailed or a different xxxFailed event. But we could argue that deleting is actually what we want, not sure.

[luckysori]

Maybe I'm being difficult, but I just can't anticipate all possible undesirable states that a CFD could be in. So it's hard for me to imagine a solution that tackles all these problems at once.

If you already envision a more general strategy that is simple enough then we can prioritise that. But I am quite happy introducing small modules that perform highly specific tasks when it comes to the topic of housekeeping.

[da-kami]

Fair enough! I saw the solution of adding a xxxFailed event (with the potential to add more xxxFailed events in the future) as a somewhat general solution. Based on a xxxFailed event we can then take different decisions in e.g. moving CFDs to failed cfds.
I think we can apply this solution to CFDs without events as well - on startup or in a periodic task. But I am not 100% sure about this pattern of adding xxxFailed events.
If we agree that this is a good solution I could write out the solution for this in a ticket. Then we could potentially close #2134 and move forward with this more general solution. The decisions when to add a xxxFailed event can change and there can be additional xxxFailed events in the future that we can add (which adds the flexibility to define more cases).

[da-kami]

We could go one step further and question the pattern to start and "finish" protocols (where finish can be a successful completion or a failed event).

We introduced the started event mainly to be able to decide:

1. If protocol A is ongoing we cannot start protocol A (i.e. we detect that a protocol is ongoing so we don't start it twice)
2. If protocol A is ongoing protocol B cannot be started (i.e. we detect that a protocol is ongoing so we don't start another protocol that is not allowed then)

We could try to re-define these constraints in a different way, but I don't have an overall better solution in my head yet.

[da-kami]

For me there is too much unknowns around changing this pattern, so I think we are stuck with it for now.

[bonomat]

I think this is the solution we should aim for: we should not have "unfinished" events in the db.

• we should check in memory if a protocol is currently running. This can also be automatically cleaned up using a timer
• we should store successful protocol executions in the db
• we can store failed protocol executions if we want to (might be useful for our statistics)

Without going into a technical solution, it sounds too simple to be possible.
But I believe with this approach we should not need any housekeeping.

[da-kami]

The initial design when we started transitioning to the event sourcing model did not intend to have these xxxStarted events, but only store actual state changes, i.e. data: DLC, attestation, timelock information (...). Only when we encountered the problems listed above we started introducing this pattern.

Here is some more thoughts on how we could go away from this pattern:

1. If protocol A is ongoing we cannot start protocol A (i.e. we detect that a protocol is ongoing so we don't start it twice)
2. If protocol A is ongoing protocol B cannot be started (i.e. we detect that a protocol is ongoing so we don't start another protocol that is not allowed then)

ad. (1.): We should be able to solve this without saving into the database as @bonomat outlined. In memory state should be enough.
ad. (2.): It might be harded to achieve this, but we could actually add a mechanism to ask the new protocol actors (we don't have multiple ephemeral protocol actors in libp2p) for ongoing protocol executions. Besides that, we may be able to define the protocols in a way so that there is no hard rule that B cannot run if A is not finished.

Additionally, there is one other problem:

3. Feedback for the user interface.

ad. (3.): At the moment the projection solely relies on loading the CFD from the database. We would need additional information about a CFD from protocol execution actors to have feedback in the UI.
Note: We could still save this information in the database, but separate it from the CFD. We could just have tables that record protocol execution stats. Not sure that is necessary though. Even better would be if we can keep that in memory only.

I am happy to invest more time into aiming for this solution, but removing the started/finished pattern will take some more work.

[luckysori]

There should be a way to keep the same state design and only persist the events once the corresponding protocol has finished. It could be an implementation detail of the database layer i.e. to keep certain (all?) xxxStarted events in memory until we process their matching xxxCompleted event.

I don't see a good alternative to ADR009, so I would not try to get rid of xxxStarted events until we find one.

[klochowicz]

This was the initial idea with event sourcing and it has caused us numerous troubles, because suddenly we had not a single source of truth - especially when we want to be able to enforce policies such as "don't do protocol B if protocol A is in-flight". It makes it much harder to convey states to the UI, which e.g. can show some label / notification or disable a button during a protocol execution.

I would caution against this path.