Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transactions with 'db.useTransaction' when multiple (same) connections are around. #65

Closed
AxelG1 opened this issue Dec 12, 2019 · 7 comments
Closed
Labels
bug

Comments

@AxelG1
Copy link

@AxelG1 AxelG1 commented Dec 12, 2019

Hello.

I had a situation in which I had multiple Database s with the same connection parameters around. Rollback had no effect when I used the transaction scope as a method db.useTransaction { ... }, but succeeded whenever I used the function useTransaction, i.e. on the global instance.

I stepped through the code. rollback SQL is called reliably but in the case described above has no effect.

As soon as I reduced the multiple connections to one, both ways of employing the transaction scope worked.

Maybe having multiple same connections is against the concept? Then of course it can also be correct undefined behaviour.

Great work by the way!

@vincentlauvlwj

This comment has been minimized.

Copy link
Owner

@vincentlauvlwj vincentlauvlwj commented Dec 12, 2019

How can I reproduce that? Can you show me your code?

@AxelG1

This comment has been minimized.

Copy link
Author

@AxelG1 AxelG1 commented Dec 12, 2019

I condensed it to this:

// inside DBUtils:
fun ktormConnect(config: JsonObject): Database {
    val host = config.getString("host")
    val port = config.getInteger("port")
    val db = config.getString("db")
    val u = config.getString("user")
    val p = config.getString("pass")
    val dbUrl = "jdbc:mysql://%s:%d/%s".format(host, port, db)
    val driver = "com.mysql.cj.jdbc.Driver"
    val dataBase = Database.connect(dbUrl, driver, u, p)
    return dataBase
}
...
// then:
val dbConfig = config.getJsonObject("db")
val db = DbUtils.ktormConnect(dbConfig)
val db2 = DbUtils.ktormConnect(dbConfig)    // this line makes the difference, if it's active rollback has no effect
...
// somewhere else I use a function copied from transaction tests:
fun testTransaction() {
    class DummyException : Exception()

    try {
        //useTransaction {
        //Database.global.useTransaction {
        db.useTransaction {
            PendingConfirmations.insert {
                it.token to "administration"
                it.type to 23
                it.email to "Hong Kong"
                it.time to TimeUtils.shared.now()
            }
            assert(PendingConfirmations.count() == 2)
            throw DummyException()
        }
    } catch (e: DummyException) {
        assert(PendingConfirmations.count() == 1)
    }
}

I'm still slow formating code here...

@vincentlauvlwj vincentlauvlwj added the bug label Dec 12, 2019
@vincentlauvlwj

This comment has been minimized.

Copy link
Owner

@vincentlauvlwj vincentlauvlwj commented Dec 12, 2019

Related issues are: #47, #41, #27

@vincentlauvlwj

This comment has been minimized.

Copy link
Owner

@vincentlauvlwj vincentlauvlwj commented Dec 12, 2019

Thank you for your feedback. I reproduced this bug with your help.

This is caused by the design of Database.global, which seems to be a mistake now.

After we call connect function, Ktorm will save the latest created database instance to Database.global automatically, then obtain it when executing a SQL.

Departments.insert, Departments.count or any other functions always use the global instance Database.global to execute their generated SQL. That's why the transaction doesn't work here, we are opening an transaction with db, but executing our SQL commands with db2.

fun main() {
    val db = Database.connect("jdbc:mysql://127.0.0.1:3306/ktorm", driver = "com.mysql.jdbc.Driver", user = "root")
    val db2 = Database.connect("jdbc:mysql://127.0.0.1:3306/ktorm", driver = "com.mysql.jdbc.Driver", user = "root")

    class DummyException : Exception()

    try {
        db.useTransaction {                 // open transaction with db
            Departments.insert {            // insert a record with db2 (Database.global)
                it.name to "test"
                it.location to "New York"
            }

            println(Departments.count())    // query the record count with db2 (Database.global)
            throw DummyException()
        }
    } catch (e: DummyException) {
        println(Departments.count())        // query the record count with db2 (Database.global)
    }
}
@vincentlauvlwj

This comment has been minimized.

Copy link
Owner

@vincentlauvlwj vincentlauvlwj commented Dec 12, 2019

I noticed the problem of Database.global months ago. I've been working on deprecating it for long and it's just finished now. I will release it as Ktorm version 2.7 after updating the documents.

There is a workaround before 2.7 released. db { } can change the value Database.global in the scope of the closure. You can use it temporally:

fun main() {
    val db = Database.connect("jdbc:mysql://127.0.0.1:3306/ktorm", driver = "com.mysql.jdbc.Driver", user = "root")
    val db2 = Database.connect("jdbc:mysql://127.0.0.1:3306/ktorm", driver = "com.mysql.jdbc.Driver", user = "root")

    class DummyException : Exception()

    db {                                        // Database.global will change to db in the scope of the closure
        try {
            db.useTransaction {                 // open transaction with db
                Departments.insert {            // insert a record with db (Database.global)
                    it.name to "test"
                    it.location to "New York"
                }

                println(Departments.count())    // query the record count with db (Database.global)
                throw DummyException()
            }
        } catch (e: DummyException) {
            println(Departments.count())        // query the record count with db (Database.global)
        }
    }
}
@AxelG1

This comment has been minimized.

Copy link
Author

@AxelG1 AxelG1 commented Dec 13, 2019

Thank you!
For now I changed my connection management, but I'm looking forward to the next release. Cheers!

@vincentlauvlwj

This comment has been minimized.

Copy link
Owner

@vincentlauvlwj vincentlauvlwj commented Feb 2, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.