pietermartin / sqlg

TinkerPop graph over sql
MIT License
240 stars 51 forks source link

supportsTransactionalSchema cause metadata sync problem in multi jvms #509

Open lalicw opened 1 month ago

lalicw commented 1 month ago

when we add graph change notify support by kafka for database not postgre, we found that metadata of different nodes is not the same in multi jvm nodes senario.

we implement notifyChange by send kafka message, and all nodes listen the same topic to recieve change and notifyFromJson.

but we found TestMultipleThreadMultipleJvm.java we reproduce the problem. and the root cause is the code in Topology.java as below:

 this.sqlgGraph.tx().afterRollback(() -> {
            if (this.sqlgGraph.getSqlDialect().supportsTransactionalSchema()) {
                afterRollback();
            } else {
                afterCommit();
            }
        });

when the database not supportsTransactionalSchema, this code indicate that use commit instead of rollback, and this will cause the change is not be send.

lalicw commented 4 weeks ago

after more analyze, i found that if we invoke after Commit in afterRollback which not supportsTransactionalSchema. this will case the data in metadata table was rollback, but the schema created, then no change notification and other node cannot create schema anymore. That's why the metadata is not sync anymore.

pietermartin commented 4 weeks ago

when we add graph change notify support by kafka for database not postgre

I am not fully understanding what database you are running on?

lalicw commented 3 weeks ago

when we add graph change notify support by kafka for database not postgre

I am not fully understanding what database you are running on?

we use mariadb, and i found the root cause is that when we rollback, the data in V_log will rollback too, and we even send the notification but cannot load change anymore.

pietermartin commented 3 weeks ago

Ok, ddl statments are not transactional on MariaDb, so its strongly advised to run schema changes in its own transaction to reduce failures leaving the db in a undefined state.

Are you creating schema elements as part of a bigger transaction, or the failure/rollback happens because of the ddl/schema change statement itself?

Are you creating the schema using Sqlg's topology api? What version of Sqlg are you on?

lalicw commented 3 weeks ago

Ok, ddl statments are not transactional on MariaDb, so its strongly advised to run schema changes in its own transaction to reduce failures leaving the db in a undefined state.

Are you creating schema elements as part of a bigger transaction, or the failure/rollback happens because of the ddl/schema change statement itself?

Are you creating the schema using Sqlg's topology api? What version of Sqlg are you on?

actually most of the time we will not create schema user sqlg api, but we will create table use sqlg api. we just try to run the TestMultipleThreadMultipleJvm.java to make sure the notification is sent. so we cannot just use afterCommit instead of afterRollback sqlg version:3.0

pietermartin commented 3 weeks ago

Ok, I'd have to investigate it further,

after more analyze, i found that if we invoke after Commit in afterRollback which not supportsTransactionalSchema. this will case the data in metadata table was rollback, but the schema created, then no change notification and other node cannot create schema anymore. That's why the metadata is not sync anymore.

MariaDb does not support transactional schema, so afterCommit is already called this.sqlgGraph.tx().afterRollback(..)

Can you explain this more, where are you invoking afterCommit in afterRollback ?

lalicw commented 3 weeks ago

Ok, I'd have to investigate it further,

after more analyze, i found that if we invoke after Commit in afterRollback which not supportsTransactionalSchema. this will case the data in metadata table was rollback, but the schema created, then no change notification and other node cannot create schema anymore. That's why the metadata is not sync anymore.

MariaDb does not support transactional schema, so afterCommit is already called this.sqlgGraph.tx().afterRollback(..)

Can you explain this more, where are you invoking afterCommit in afterRollback ?

In Topology.java line 539

        this.sqlgGraph.tx().afterRollback(() -> {
            if (this.sqlgGraph.getSqlDialect().supportsTransactionalSchema()) {
                afterRollback();
            } else {
                afterCommit();
            }
        });