ocaml-community / ocaml-mariadb

OCaml bindings to MariaDB, supporting the nonblocking API
55 stars 18 forks source link

"Commands out of sync" error from MariaDB when executing statements in parallel with Lwt #12

Closed donut closed 7 years ago

donut commented 7 years ago

When I have multiple queries executed in parallel using something like Lwt.join I run into this error:

Fatal error: exception Failure("prepare: (2014) Commands out of sync; you can't run this command now")

I've created a gist to reproduce the problem: https://gist.github.com/donut/080ce76cf0dab97f5dc6e933d3f58daf

Key code:

let execute_query conn query values yield =
  Lwt_io.printlf "Executing query %s" query >>= fun () ->
  Mdb.prepare conn query >>= or_die "prepare" >>= fun stmt ->
  Lwt_io.printlf "prepared query" >>= fun () ->
  Mdb.Stmt.execute stmt values >>= or_die "exec" >>= fun result ->
  Lwt_io.printlf "executed query, yielding" >>= fun () ->
  yield result >>= fun return -> 
  Lwt_io.printlf "yielded, closing statement" >>= fun () ->
  Mdb.Stmt.close stmt >>= or_die "stmt close" >>= fun () ->
  Lwt_io.printlf "returning value from yield" >>= fun () ->
  Lwt.return return

let test db_conn index =
  let query =
    "SELECT id FROM url WHERE id = ? ORDER BY id ASC LIMIT 1" in

  execute_query db_conn query [| `Int 1 |] (function
  | None -> Lwt_io.printlf "[%d] nothing found" index
  | Some result -> stream result >>= stream_next_opt >>= function
    | None -> Lwt_io.printlf "[%d] no rows returned" index
    | Some row -> Lwt_io.printlf "[%d] row found" index
  )

let main () =
  let db_connect =
    Mdb.connect ~host:"localhost" ~user:"root" ~pass:"" ~db:"rtmDOTtv" in
  db_connect () >>= or_die "connect" >>= fun db_conn ->

  let mkt index = test db_conn index in
  Lwt.join [ mkt 0; mkt 1; mkt 2; ] >>= fun () ->

  Mdb.close db_conn

let () =
  Lwt_main.run @@ main ()

Am I doing something wrong? I was hoping to use this library for a web app, but I'm guessing parallel requests would run into this issue as well.

donut commented 7 years ago

It looks like I've been doing it wrong. Gotta run, but here's the updated version of the code posted above:

let execute_query db_connect query values yield =
  db_connect () >>= fun conn ->
  Lwt_io.printlf "Executing query %s" query >>= fun () ->
  Mdb.prepare conn query >>= or_die "prepare" >>= fun stmt ->
  Lwt_io.printlf "prepared query" >>= fun () ->
  Mdb.Stmt.execute stmt values >>= or_die "exec" >>= fun result ->
  Lwt_io.printlf "executed query, yielding" >>= fun () ->
  yield result >>= fun return -> 
  Lwt_io.printlf "yielded, closing statement" >>= fun () ->
  Mdb.Stmt.close stmt >>= or_die "stmt close" >>= fun () ->
  Lwt_io.printlf "closign connection" >>= fun () ->
  Mdb.close conn >>= fun () ->
  Lwt_io.printlf "returning value from yield" >>= fun () ->
  Lwt.return return

let test db_conn index =
  let query =
    "SELECT id FROM url WHERE id = ? ORDER BY id ASC LIMIT 1" in

  execute_query db_conn query [| `Int 1 |] (function
  | None -> Lwt_io.printlf "[%d] nothing found" index
  | Some result -> stream result >>= stream_next_opt >>= function
    | None -> Lwt_io.printlf "[%d] no rows returned" index
    | Some row -> Lwt_io.printlf "[%d] row found" index
  )

let main () =
  let db_connect () =
    Mdb.connect ~host:"localhost" ~user:"root" ~pass:"" ~db:"rtmDOTtv" ()
    >>= or_die "connect" in

  let mkt index = test db_connect index in
  Lwt.join [ mkt 0; mkt 1; mkt 2; ]

let () =
  Lwt_main.run @@ main ()

Basically, I need to make a new connection for each query. I'll verify this solution later today and close the issue if it checks out.

andrenth commented 7 years ago

This shouldn't be necessary, so it looks like a real bug. I haven't had time to test the code in your gist (busy week at work) but I'll try to fix this ASAP.

On Tue, Oct 31, 2017 at 12:32 PM, Donovan Mueller notifications@github.com wrote:

It looks like I've been doing it wrong. Gotta run, but here's the updated version of the code posted above:

let execute_query db_connect query values yield = db_connect () >>= fun conn -> Lwt_io.printlf "Executing query %s" query >>= fun () -> Mdb.prepare conn query >>= or_die "prepare" >>= fun stmt -> Lwt_io.printlf "prepared query" >>= fun () -> Mdb.Stmt.execute stmt values >>= or_die "exec" >>= fun result -> Lwt_io.printlf "executed query, yielding" >>= fun () -> yield result >>= fun return -> Lwt_io.printlf "yielded, closing statement" >>= fun () -> Mdb.Stmt.close stmt >>= or_die "stmt close" >>= fun () -> Lwt_io.printlf "closign connection" >>= fun () -> Mdb.close conn >>= fun () -> Lwt_io.printlf "returning value from yield" >>= fun () -> Lwt.return return

let test db_conn index = let query = "SELECT id FROM url WHERE id = ? ORDER BY id ASC LIMIT 1" in

execute_query db_conn query [| `Int 1 |] (function | None -> Lwt_io.printlf "[%d] nothing found" index | Some result -> stream result >>= stream_next_opt >>= function | None -> Lwt_io.printlf "[%d] no rows returned" index | Some row -> Lwt_io.printlf "[%d] row found" index ) let main () = let db_connect () = Mdb.connect ~host:"localhost" ~user:"root" ~pass:"" ~db:"rtmDOTtv" ()

= or_die "connect" >>= Lwt.return in

let mkt index = test db_connect index in Lwt.join [ mkt 0; mkt 1; mkt 2; ] let () = Lwt_main.run @@ main ()

Basically, I need to make a new connection for each query. I'll verify this solution later today and close the issue if it checks out.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/andrenth/ocaml-mariadb/issues/12#issuecomment-340780569, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB2BVBYESyd4tY1OCoCS3GcIssmOZmUks5sxy9mgaJpZM4QL5ge .

donut commented 7 years ago

Unfortunately, the multi-connection solution I suggested only works when new connections are made before any of the others have closed. I tried changing main to this:

let main () =
  let db_connect () =
    Mdb.connect ~host:"localhost" ~user:"root" ~pass:"" ~db:"rtmDOTtv" ()
    >>= or_die "connect" in

  let mkt index = test db_connect index in
  mkt 0 >>= fun () -> mkt 1 (* <-- changed from `Lwt.join [ mkt 0; ... ]` *)

Ran into this error:

Fatal error: exception Failure("connect: (2059) Plugin pvio_socket could not be loaded: not initialized")

Details on common conditions this happens under.

donut commented 7 years ago

@andrenth Do you think you'll be able to get to this issue soon? Not trying to pressure, just trying to gauge whether or not I should move to a different DB lib as I near the completion of my project. I completely understand that this is something you do in your free time. This lib has been great and I'd love to stick with it, but it will soon be blocking my project from being releasable.

I'd look at fixing the issue myself, but I'm brand new to OCaml and have basically no experience with C (which I'm guessing I'd need knowledge).

andrenth commented 7 years ago

Hi

I’ll try to look into it tomorrow morning. I haven’t been doing much OCaml lately so it’s been hard to find the time.

Sorry for the delay.

Em 20 de nov de 2017, à(s) 14:38, Donovan Mueller notifications@github.com escreveu:

@andrenth Do you think you'll be able to get to this issue soon? Not trying to pressure, just trying to gauge whether or not I should move to a different DB lib as I near the completion of my project. I completely understand that this is something you're doing in your free time. This lib has been great and I'd love to stick with it, but it is blocking my project from being releasable.

I'd look at fixing the issue myself, but I'm brand new to OCaml and have very little experience with compiled languages, especially not C (which I'm guessing I'd need knowledge).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

andrenth commented 7 years ago

According to this test and the comment in the docs for mysql_thread_init, it seems that you actually do need a separate handle for each thread, and it's likely this extends to Lwt or Async where you would be interleaving usage of a shared handle.

I'll try to check the second error you found.

andrenth commented 7 years ago

So, using MariaDB's libmysqlclient, your gist works for me with the following mkt function:

let mkt index = 
  db_connect () >>= or_die "connect" >>= fun db_conn ->
  test db_conn index >>= fun () ->
  Mdb.close db_conn

which I can call either with Lwt.join [ mkt 0; mkt 1; mkt 2; ] or mkt 0 >>= fun () -> mkt 1 >>= fun () -> mkt 2.

Now, if I compile ocaml-mariadb with MariaDB's Connector/C instead of libmysqlclient, the second form also works, but when using Lwt.join I get unknown: debugger aborting because missing DBUG_RETURN or DBUG_VOID_RETURN macro in function "vio_read".

I'll try to update the Connector/C and see if this error persists.

andrenth commented 7 years ago

With the latest Connector/C the Lwt.join version works, but the sequential execution one fails with the same error you got about pvio_socket.

andrenth commented 7 years ago

Found the explanation here: https://jira.mariadb.org/browse/CONC-277

I call mysql_library_end every time after calling mysql_close, which works fine when using libmysqlclient, but not with the Connector/C.

I'll release a version that doesn't call mysql_library_end automatically. So from this version on, ocaml-mariadb users will have to call the function themselves.

donut commented 7 years ago

Thank you so much for looking into this. Would you mind updating the readme or giving an example here of when and how one could call mysql_library_end? Would it simply be Ffi_bindings.Bindings.mysql_library_end () after closing the connection for the last time in the application?

andrenth commented 7 years ago

I'll add a new function to wrap the binding, so it should be just Mdb.library_end () in your case.

andrenth commented 7 years ago

Fixed in 78eb270. I'll publish a new version tomorrow.

andrenth commented 7 years ago

Version 0.9.0 was tagged and submitted to OPAM.