Moodle
  1. Moodle
  2. MDL-37734

POLICY: sqlsrv driver, to be or not to be

    Details

    • Database:
      Microsoft SQL
    • Workaround:
      Hide

      (use FreeTDS instead, while this gets fixed)

      Show
      (use FreeTDS instead, while this gets fixed)
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Rank:
      5128

      Description

      This issue is about to describe the problem happening with current sqllrv moodle driver (using the MS driver for PHP) to use Moodle under SQL*Server databases.

      Along the last months, it has been reported in various issues, in different places, and it's affecting more and more, while we add more transactions to code base.

      Basically, this is the history:

      1) By default SQL*Server does not allow to have more than one recordset opened at the same time. Aka, any code doing this, will fail:

      • open recordset
      • open recordset

      2) To solve that limitation MS invented the "Multiple Active Result Sets" mode (named MARS, good name), so the combination above started to work (apparently by introducing a noticeable slowdown, but works).

      3) Obviously, in the Moodle driver, we are using that mode on all connections.

      4) The MARS mode, has some severe limitations about what can be done and what cannot. One of them is, literally, killing us. Cannot use transactions if there is any recordset open. Aka, any code doing this, will fail:

      • open recordset
      • start transaction

      5) While in Moodle 2.0 (when the driver was created), we weren't using transactions in many places, it has been a long walk since then, and everyday we are using transactions more and more. All those places are breaking when the sqlsrv driver is used.

      So, it's time to solve this problem. Possible solutions are:

      A) Drop support of the driver and turn everybody to use the FreeTDS alternative. Somehow those guys manage to workaround that problem and, with their driver, all tests are passing (note we added some specific tests at MDL-34130). I read somewhere, time ago, that they read the whole recordset in memory, just to later simulate iteration from there. Inefficient but works and that way they workaround both the problem exposed in 1) and 4) above.

      B) Solve that in the Moodle sqlsrv driver, so each time a transaction is going to be stated, we read ALL the opened recodsets and put them in memory (and closing them), emulating iteration from there (similar to the FreeTDS solution), only applied for 4). Hopefully, memory won't grow a lot (if opened recordsets aren't too big) and speed should not get much impact. It seems this is pretty doable (in fact we have already one candidate patch for it, thanks Petr!).

      C) Ask to MS to introduce some mode into their MS driver for PHP ("Will Transactions Finally Work under MARS, aka WTF-W-MARS"). If it can be implemented by us @ B) in our driver), surely it's also implementable by them into their driver. That would fix, once and for all, one of the major drawbacks of SQL*Server trying to run on pair with other, more "standard" RDBMS where the limitations commented above in 1) and 4) are, simply, ridiculous.

      And that is all, ciao

        Issue Links

          Activity

          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          My vote goes to implement B) ASAP and also ask for C) to MS.

          Show
          Eloy Lafuente (stronk7) added a comment - - edited My vote goes to implement B) ASAP and also ask for C) to MS.
          Hide
          Petr Škoda added a comment -

          I have linked a sample patch, I am going to improve the comments if we agree to use it.

          Show
          Petr Škoda added a comment - I have linked a sample patch, I am going to improve the comments if we agree to use it.
          Hide
          Martin Dougiamas added a comment -

          Great description of the problem Eloy, really makes this easier to understand. I agree with B + C.

          Show
          Martin Dougiamas added a comment - Great description of the problem Eloy, really makes this easier to understand. I agree with B + C.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Note that B (MDL-37748, subtask of this), has met upstream (23, 24 and master). So we need to pass the ball to MS (C) and see if they can do something in their side.

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Note that B ( MDL-37748 , subtask of this), has met upstream (23, 24 and master). So we need to pass the ball to MS (C) and see if they can do something in their side.
          Hide
          Luis de Vasconcelos added a comment -

          Has this problem been reported to Microsoft? I looked on https://sqlsrvphp.codeplex.com/workitem/list/basic but there doesn't seem to be anything there.

          Show
          Luis de Vasconcelos added a comment - Has this problem been reported to Microsoft? I looked on https://sqlsrvphp.codeplex.com/workitem/list/basic but there doesn't seem to be anything there.
          Hide
          Martin Dougiamas added a comment -

          Eloy can you report this? Give it a shot.

          Show
          Martin Dougiamas added a comment - Eloy can you report this? Give it a shot.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Mail has been sent to MS contact informing about the issue, and our solution, asking for alternatives @ sqlsrv database/driver.

          So I'm closing this. Any change, if meaningful, will be reported into different issue.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Mail has been sent to MS contact informing about the issue, and our solution, asking for alternatives @ sqlsrv database/driver. So I'm closing this. Any change, if meaningful, will be reported into different issue. Ciao
          Hide
          Luis de Vasconcelos added a comment -

          Did Microsoft reply?

          Show
          Luis de Vasconcelos added a comment - Did Microsoft reply?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Not yet. I have been CC-ed about the mail being forwarded to new ppl in charge of PHP/Drivers/SQL*Server (I don't know the exact area), but not reply yet.

          Show
          Eloy Lafuente (stronk7) added a comment - Not yet. I have been CC-ed about the mail being forwarded to new ppl in charge of PHP/Drivers/SQL*Server (I don't know the exact area), but not reply yet.
          Hide
          Luis de Vasconcelos added a comment -

          The notes above indicate that one of the problems with the Microsoft PHP driver is that it does not support MARS. I just noticed in the FreeTDS documentation that, apparently, FreeTDS does not support MARS either: http://www.freetds.org/userguide/choosingtdsprotocol.htm

          In point three above Eloy (I presume) says:
          "3) Obviously, in the Moodle driver, we are using that mode on all connections"

          So, if FreeTDS does not support MARS either then why are there problems using MARS with the MS driver, but not the FreeTDS driver?

          Show
          Luis de Vasconcelos added a comment - The notes above indicate that one of the problems with the Microsoft PHP driver is that it does not support MARS. I just noticed in the FreeTDS documentation that, apparently, FreeTDS does not support MARS either: http://www.freetds.org/userguide/choosingtdsprotocol.htm In point three above Eloy (I presume) says: "3) Obviously, in the Moodle driver, we are using that mode on all connections" So, if FreeTDS does not support MARS either then why are there problems using MARS with the MS driver, but not the FreeTDS driver?
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Hi Luis,

          simple. Because the FreeTDS driver does implement since ages ago (natively, without needing any php code in our mssql driver) exactly the same solution that we did for the Microsoft drivers by hacking our php sqlsrv driver:

          To read the whole recordset to memory (and close it in the DB server) before opening the next one.

          That way MARS is not needed because the DB server will have ZERO opened recordsets when opening the next one (or when starting a transaction). And FreeTDS is the one "owning" those records and allowing PHP to continue iterating over them. Basically what I explained in point A) in the main description.

          Hope it explains it. Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Hi Luis, simple. Because the FreeTDS driver does implement since ages ago (natively, without needing any php code in our mssql driver) exactly the same solution that we did for the Microsoft drivers by hacking our php sqlsrv driver: To read the whole recordset to memory (and close it in the DB server) before opening the next one. That way MARS is not needed because the DB server will have ZERO opened recordsets when opening the next one (or when starting a transaction). And FreeTDS is the one "owning" those records and allowing PHP to continue iterating over them. Basically what I explained in point A) in the main description. Hope it explains it. Ciao

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: