Invalidate model cache when model is registered/updated

[broody]

Torii stores model schemas in cache so it doesn't have to rebuild from database on every query. When a model is registered/updated, we should invalidate the corresponding cache entry.

Cache implementation: https://github.com/dojoengine/dojo/blob/b3f49977fe921564725c30b912126aea57859bbd/crates/torii/core/src/cache.rs#L17 Subscription event: https://github.com/dojoengine/dojo/blob/b3f49977fe921564725c30b912126aea57859bbd/crates/torii/core/src/sql.rs#L109

[CouldBeFree]

As far as I understand SimpleBroker::publish(model_registered); should call clear method from the ModelCache struct?

[broody]

As far as I understand SimpleBroker::publish(model_registered); should call clear method from the ModelCache struct?

So SimpleBroker will publish an event when a model is registered, we could subscribe to these events in ModelCache and just call update_schema to invalidate that cache entry. Lemme know if you wanna take a stab at it.

[CouldBeFree]

ModelCache should have a subscribe method which should call update_schema?

[tarrencev]

ModelCache should have a subscribe method which should call update_schema?

yes we should create a channel between the register_model processor and the cache. register_model should push a message onto the channel when it is triggered.

more generally, it might be useful to have a generalized pub sub system for processors to emit messages on using channels. we can pass it into each processor similar to the db. that way, processors can notify other parts of the code when they are triggered. we already do this adhoc for set_record and block notifications

[CouldBeFree]

I used publish-subscribe in js in this way.

class PubSub {
    constructor() {
        this.instance = null;
    }

    init() {
        const events = {};

        return {
            subscribe(evName, fn) {
                events[evName] = events[evName] || [];
                events[evName].push(fn);
            },
            unsubscribe(evName, fn) {
                if (events[evName]) {
                    events[evName] = events[evName].filter(f => f !== fn);
                }
            },
            publish(evName, data) {
                if (events[evName]) {
                    events[evName].forEach(f => {
                        f(data);
                    });
                }
            }
        };
    }

    getInstance() {
        if (!this.instance) {
            this.instance = this.init();
        }
        return this.instance;
    }
}

export default PubSub;

And it works in the following way. Some method subscribes to the some event name -> inst.subscribe('ev_name', someDataHandler); In another part of the application, I publish some data under the registered event name -> inst.publish('ev_name', someData); Do we need to implement pub sub in this way?

[loothero]

@broody I am trying to capture this issue with a test case but the current system doesn't appear to support updating an existing model via sql.rs -> register_model

I have tested three variants of updating a model:

1. Overwrite an existing model with an empty set of children Result: The model is unchanged in the db.

2. Update the type of an existing child of a model: Result: The child type is not updated in the db

3. Add a new child to an existing model: Result: database error "(code: 1) no such column: external_new_children"

This leads me to conclude that the current system does not provide the ability to update an existing model. Curious if we have any test cases or real world examples of an existing model being updated. If not and you think the above observation could be valid, I can create an issue for this and push my failing test cases.

[broody]

Hi @loothero!

Mmm u r right, I reproduced issue. Yes, please file an issue. I don't think we have test coverage for this from indexer side.

[gianalarcon]

Hi Team, I would like to try this one

[glihm]

@gianalarcon hey mate, is this still a valid issue?

[gianalarcon]

Hi sir. Yes it is. Me and @broody are still working on it

[CollinsC1O]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

Hi I'm a frontend and also a smart contract develop. i will love to work on this please assign

[Shubhammore71]

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I am smart contract developer ,I can try to solve this problem

[anihdev]

Would love to tackle this!

[LuisDi98]

As I understand the issue consists on invalidating the model cache whenever a model is registered or updated in the system. The ModelCache class should refresh its corresponding cache entries when these registration or update events occur.

My strategy requires setting up a subscription within the ModelCache structure to listen to model registration events. Currently, the system uses SimpleBroker::publish(model_registered) to broadcast these events so the ModelCache needs to subscribe to these events and react accordingly to the case.

Also I think it's important to create a channel between the register_model processor and ModelCache, so when a model is registered, the processor should push a message onto this channel. This means adding a method to ModelCache that listens to these events and calls update_schema to invalidate the related cache entry.

Based on the previous discussions, a generalized publish-subscribe system using channels could be useful. This would allow other processors to emit messages similarly. This approach mirrors the way ad-hoc notifications are handled for set_record and block notifications.

Steps to Address the Issue

1. Implement the Communication Channel: i think creating a channel within ModelCache to receive messages whenever a model is registered or updated can be useful to tackle it.
2. Subscribe to the model_registered Event: Add a subscription method to ModelCache that listens for the SimpleBroker::publish(model_registered) event.
3. Invalidate the Cache: Modify ModelCache so that it calls update_schema or a similar method when it receives the model_registered event, updating the cache entry.

Also I am part of Dojo Coding community looking to contribute to new open source projects, it would be nice to work with Dojo project.

[glihm]

Torii now overrides the cache by directly settings the data into the cache: https://github.com/dojoengine/dojo/blob/aa039c0c89ace7cdb9838116a1ab7b71269a920e/crates/torii/core/src/sql/mod.rs#L305-L310.

The channel suggestion is something interesting to keep for farthing improvement and decoupling.

[LuisDi98]

Ohh that's great, I'm gonna keep looking forward other issues and hope can colaborate later ;) Thanks man