twitter / finagle

A fault tolerant, protocol-agnostic RPC system
https://twitter.github.io/finagle
Apache License 2.0
8.78k stars 1.45k forks source link

Nested transactions are stuck #925

Open RicoGit opened 2 years ago

RicoGit commented 2 years ago

I have a test like this:

  test("Finagle StdClient is stuck on nested transaction") {
    val finagleClient: Client with Transactions = getUnderlyingFinagleClient
    val future = finagleClient.transaction { tx1 =>
      for {
        _ <- tx1.execute[Unit](sql"select * from nodes limit 1")
        _ <- finagleClient.transaction { tx2 =>
          tx2.execute(sql"select * from nodes limit 1")
        }
      } yield ()
    }

    intercept[TimeoutException] {
      Await.result(future, Duration.fromSeconds(42)) // always throws timeout exception 
    }
  }

This is minimal example, but it reflects what I actually've faced. I knows that I'm shouldn't do that in general. But in large code base with a bunch of Dao and Repositories is really hard to catch this case. Is this usage of client valid?Is it a bug?

joybestourous commented 2 years ago

Nested transactions aren't permitted in mysql: https://dev.mysql.com/doc/refman/5.7/en/implicit-commit.html

RicoGit commented 2 years ago

The question is about API of finagle-mysql that allows to make stuff like that.

I'm figured out why it's stuck. The reason is session pool with max size 1. If increase max size of session poll up to N then N number of nested transaction are allowed. In fact all transactions are just parallel and don't share any state.

The second problem is deadlocks. If you update row in parent transaction and then try to update the same row in child transaction they will be wait each other.

I understand that it's difficult to ban expression like above in compile time. But maybe better if nested transactions will be raise some exception in runtime?

joybestourous commented 2 years ago

gotcha, yeah we can make more clear that we don't support nested transactions via the api. to clarify, when you made those changes, you were able to get nested transactions working?