balena-io-modules / mahler

A automated task composer and HTN based planner for building autonomous system agents.
Apache License 2.0
7 stars 1 forks source link

Actions are not atomic #45

Closed pipex closed 11 months ago

pipex commented 11 months ago

The changes of an action on the system state should only be committed to the agent memory if the action runs to completion. Right now, changes are stored in memory as soon as they take place within the action body (thanks to the observe function), however, this risks leaving the agent in an inconsistent state which will mess up with planning (or the system state in the worst case scenario). Here is a simple replication

const plusOne = Task.from<number>({
    condition: (state, { target }) => state < target,
    effect: (state) => ++state._,
    action: async (state) => {
        ++state._;

        // This step could be the storage of the state to disk
        throw new Error('action failed');
    },
    description: '+1',
});
const agent = Agent.from({
    initial: 0,
    opts: { logger: logger, maxRetries: 0 },
    tasks: [plusOne],
});

agent.seek(1);

const res = await agent.wait();
expect(res.success).to.be.false;

// The state should be reset
expect(agent.state).to.equal(0);
pipex commented 11 months ago

I can see two solutions for this,

Refactor the agent interface to report an AgentStatus object on subscribe. Subscribers would receive updates while the agent is running as different events. For changes, subscribers would receive a 'change' event, including the list of changes performed by the action. The observe function would only commit the changes to memory after the action has executed successfully.

// This is how the agent state object would look like
type AgentState<T> = {event: 'start', status: 'running', state: T} | {event: 'change', status: 'running', changes: Array<Operation<T, any>>}

agent.subscribe((state) => {
   if (state.change) {
      // patch changes in local copy
   }
});

This is of course a major change, but it would improve observabiliity, however it would also complicate the API even more

pipex commented 11 months ago

The second solution is to make operations reversible. When performing an update, we keep track of the changes to the state and store the inverse operation, e.g. for a delete/update/create we store the reverse create/update/delete operation. The observe function would perform the same work it does today, but on failure it would apply the inverse operations on the state.

This option is simpler as it doesn't change the interface, but potentially it would require storing more data on memory as we need to keep copies of every changed property, which could get to a relevant size if the action performs a lot of changes.

pipex commented 11 months ago

Fixed by PR #46