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

Avatica server should expire stale connections/statements

Details

    • Improvement
    • Status: Closed
    • Major
    • Resolution: Fixed
    • None
    • 1.2.0-incubating
    • None

    Description

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

      Attachments

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

        Issue Links

          Activity

            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.

            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.
            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.

            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.
            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.

            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.
            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.

            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.
            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.

            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.
            ndimiduk Nick Dimiduk added a comment -

            Clean up tests and checkstyle issues.

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

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

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

            People

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

              Dates

                Created:
                Updated:
                Resolved: