Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-640

Avatica server should expire stale connections/statements

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2.0-incubating
    • Component/s: None
    • Labels:

      Description

      To avoid resource leaks in a long-running server process, we should be expiring our connections and statement handles after some configurable period.

      1. 640.txt
        14 kB
        Nick Dimiduk
      2. CALCITE-640.00.patch
        19 kB
        Nick Dimiduk

        Activity

        Hide
        julianhyde Julian Hyde added a comment -

        That "configurable period" should be the connection/statement timeout requested by the JDBC client.

        A server should have sensible default timeouts. And maybe as a policy a server might disallow timeouts that are too long.

        Show
        julianhyde Julian Hyde added a comment - That "configurable period" should be the connection/statement timeout requested by the JDBC client. A server should have sensible default timeouts. And maybe as a policy a server might disallow timeouts that are too long.
        Hide
        ndimiduk Nick Dimiduk added a comment -

        Initial patch using Guava cache for both connections and statements. Reads global configuration properties based on properties provided at connection creation. Tell me if you think the defaults look sensible.

        Will now look into threading connection/statement parameters through.

        Show
        ndimiduk Nick Dimiduk added a comment - Initial patch using Guava cache for both connections and statements. Reads global configuration properties based on properties provided at connection creation. Tell me if you think the defaults look sensible. Will now look into threading connection/statement parameters through.
        Hide
        ndimiduk Nick Dimiduk added a comment - - edited

        Okay, how would the client request a timeout? Right now we implicitly create connection objects the first time a client requests data. This is in order to avoid RPCs. I see no timeout methods on the java.sql.Connection API. java.sql.Statement has a setQueryTimeout method, but that's a timeout for query execution, not for the lifetime of a statement preparation.

        This random article has some guidelines for different settings and different database drivers. There's apparently not a consistent way to do things.

        Show
        ndimiduk Nick Dimiduk added a comment - - edited Okay, how would the client request a timeout? Right now we implicitly create connection objects the first time a client requests data. This is in order to avoid RPCs. I see no timeout methods on the java.sql.Connection API. java.sql.Statement has a setQueryTimeout method, but that's a timeout for query execution, not for the lifetime of a statement preparation. This random article has some guidelines for different settings and different database drivers. There's apparently not a consistent way to do things.
        Hide
        julianhyde Julian Hyde added a comment -

        Can you add connect-string parameters (in enum BuiltInConnectionProperty) for connectionTimeout and statementTimeout with sensible defaults. Make them strings so that people can write "10s" or "10000ms" if they want, and they mean the same as "10".

        In their doc, specify their time unit and note the fact that they are for inactivity (on the part of both the server and client). If a connection has a timeout of 1h and creates or closes a statement every 59m it will never time out.

        Show
        julianhyde Julian Hyde added a comment - Can you add connect-string parameters (in enum BuiltInConnectionProperty) for connectionTimeout and statementTimeout with sensible defaults. Make them strings so that people can write "10s" or "10000ms" if they want, and they mean the same as "10". In their doc, specify their time unit and note the fact that they are for inactivity (on the part of both the server and client). If a connection has a timeout of 1h and creates or closes a statement every 59m it will never time out.
        Hide
        ndimiduk Nick Dimiduk added a comment -

        Guava's Cache class doesn't let you override the expiration conditions for individual values. I think we'll have to extend/implement our own Cache to achieve this goal, or maybe add a daemon thread that trolls the cache on some interval, honoring custom key settings. For now, let's leaving the cache duration as a configuration setting specified by the operator at server launch time.

        Show
        ndimiduk Nick Dimiduk added a comment - Guava's Cache class doesn't let you override the expiration conditions for individual values. I think we'll have to extend/implement our own Cache to achieve this goal, or maybe add a daemon thread that trolls the cache on some interval, honoring custom key settings. For now, let's leaving the cache duration as a configuration setting specified by the operator at server launch time.
        Hide
        ndimiduk Nick Dimiduk added a comment -

        Clean up tests and checkstyle issues.

        Show
        ndimiduk Nick Dimiduk added a comment - Clean up tests and checkstyle issues.
        Hide
        ndimiduk Nick Dimiduk added a comment -
        Show
        ndimiduk Nick Dimiduk added a comment - This is on the tip of my branch https://github.com/ndimiduk/incubator-calcite/tree/avatica-to-prod
        Show
        julianhyde Julian Hyde added a comment - Fixed in http://git-wip-us.apache.org/repos/asf/incubator-calcite/commit/0f824f81 .
        Hide
        julianhyde Julian Hyde added a comment -

        Resolved in release 1.2.0-incubating (2015-04-16)

        Show
        julianhyde Julian Hyde added a comment - Resolved in release 1.2.0-incubating (2015-04-16)

          People

          • Assignee:
            julianhyde Julian Hyde
            Reporter:
            ndimiduk Nick Dimiduk
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development