Commons Dbcp
  1. Commons Dbcp
  2. DBCP-71

[dbcp] Docs inaccurately describe default value of testOnBorrow

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Operating System: other
      Platform: Other

      Description

      On the page http://jakarta.apache.org/commons/dbcp/configuration.html, the
      Default column for the testOnBorrow parameter says that testOnBorrow is set by
      default to "true". I observe from the behaviour of my application that this
      seems to be true only if validationQuery is not set to null. Further, I find
      this code in the BasicDataSource.java file, createDataSource method:
      // Can't test without a validationQuery
      if (validationQuery == null)

      { setTestOnBorrow(false); setTestOnReturn(false); setTestWhileIdle(false); }

      Users may think that DBCP uses a default validationQuery such as "SELECT 1" if
      you don't set one up explicitly, and that therefore setting testOnBorrow
      without setting validationQuery is legitimate. This is what I naively thought -
      although on reflection I can see why you don't, for reasons of portability.

      It would be very helpful if the documentation explained this, with text like
      this:
      "The testOnBorrow, testOnReturn, and testWhileIdle parameters are defaulted to
      true if and only if you supply a non-null value for the validationQuery
      parameter."

      It would be even more helpful if BasicDataSource noticed that testOnBorrow was
      set to true without a valid validationQuery setting, and logged something like
      "Disabling testOnBorrow, testOnReturn, and testWhileIdle because no
      valiationQuery was supplied. To enable these settings you must also supply a
      validationQuery, typically a very simple query like 'SELECT 1'".

        Activity

        Hide
        Douglas Squirrel added a comment -

        I just realised that my suggested text was wrong on two fronts:
        1. testOnReturn and testWhileIdle are not set by default.
        2. Even if you explicitly set any of these three parameters, they will be
        ignored if you don't also set a validationQuery.

        So I would suggest adding this text to each of the three parameters instead,
        preferably with bold type or a bright colour:
        "NOTE: This parameter will be ignored if validationQuery is not set to a non-
        null string."

        Show
        Douglas Squirrel added a comment - I just realised that my suggested text was wrong on two fronts: 1. testOnReturn and testWhileIdle are not set by default. 2. Even if you explicitly set any of these three parameters, they will be ignored if you don't also set a validationQuery. So I would suggest adding this text to each of the three parameters instead, preferably with bold type or a bright colour: "NOTE: This parameter will be ignored if validationQuery is not set to a non- null string."
        Hide
        Phil Steitz added a comment -

        Site docs and method javadocs have been updated to clarify this issue. Thanks!

        Show
        Phil Steitz added a comment - Site docs and method javadocs have been updated to clarify this issue. Thanks!

          People

          • Assignee:
            Unassigned
            Reporter:
            Douglas Squirrel
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development