conn.close() idempotence

[Daniele Varrazzo-2]

Hello,

in a discussion on the psycopg mailing list, an user has been
surprised by the fact that .close() called on a closed connection
raised error. I thought this was an implementation accident in the
driver and a more robust semantics should allow for close to be called
on a closed object without effect. But going to code a forgiving
double close, I found a test failing in the Stuart Bishop DBAPI unit
test,

        # connection.close should raise an Error if called more than once
        self.assertRaises(self.driver.Error,con.close)

I see no reason to require this, and it actually creates some problem.
For example, if the connection is dropped for a network problem, the
driver may detect the invalid connection and put the connection in
closed status. But at this point any guard such as:

    conn = connection()
    try:
        # work
    finally:
        conn.close()

risks to become a problem, with the close() that may raise an error
because the connection has been implicitly closed by a communication
error. Note that close() in itself often is a quite safe operation,
not involving database communication, so it is not expected to fail
and may not be well guarded.

The DBAPI says about conn.close():

    The connection will be unusable from this point
    forward; an Error (or subclass) exception will be raised
    if any operation is attempted with the connection.

now, is conn.close() an "operation" on the connection? Even if it
doesn't involve server communication? I don't think an idempotent
close() violates this rule, so I'm really asking about an
interpretation of the DBAPI, not a change.

Another user in the ML pointed out that an idempotent close is also
the behaviour of the Python file objects.

My opinion is that the DBAPI test suite can be relaxed and
conn.close() should have no effect on a closed connection. What do you
think?

-- Daniele
_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig

[Michael Bayer]

On Oct 18, 2011, at 8:53 PM, Daniele Varrazzo wrote:

>
> The DBAPI says about conn.close():
>
>    The connection will be unusable from this point
>    forward; an Error (or subclass) exception will be raised
>    if any operation is attempted with the connection.
>
> now, is conn.close() an "operation" on the connection? Even if it
> doesn't involve server communication? I don't think an idempotent
> close() violates this rule, so I'm really asking about an
> interpretation of the DBAPI, not a change.
>
> Another user in the ML pointed out that an idempotent close is also
> the behaviour of the Python file objects.
>
> My opinion is that the DBAPI test suite can be relaxed and
> conn.close() should have no effect on a closed connection. What do you
> think?

I'm +1 for close() on a closed connection being harmless.  rollback() already has this behavior, as the spec specifies "rolls back the start of any pending transaction", thus allowing for no activity if no transaction is present.

_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig

[M.-A. Lemburg]

Daniele Varrazzo wrote:

> Hello,
>
> in a discussion on the psycopg mailing list, an user has been
> surprised by the fact that .close() called on a closed connection
> raised error. I thought this was an implementation accident in the
> driver and a more robust semantics should allow for close to be called
> on a closed object without effect. But going to code a forgiving
> double close, I found a test failing in the Stuart Bishop DBAPI unit
> test,
>
>         # connection.close should raise an Error if called more than once
>         self.assertRaises(self.driver.Error,con.close)

This is a consequence of making the connection object unusable
after .close() was called for the first time.

> I see no reason to require this, and it actually creates some problem.
> For example, if the connection is dropped for a network problem, the
> driver may detect the invalid connection and put the connection in
> closed status. But at this point any guard such as:
>
>     conn = connection()
>     try:
>         # work
>     finally:
>         conn.close()
>
> risks to become a problem, with the close() that may raise an error
> because the connection has been implicitly closed by a communication
> error. Note that close() in itself often is a quite safe operation,
> not involving database communication, so it is not expected to fail
> and may not be well guarded.

That last sentence is not quite correct: .close() issues an implicit
.rollback() and usually also tells the database backend to free up
resources maintained for the connection, so it does require communication
with the backend.

There are a few situations which can result in an exception:

 * the network has gone down or is temporarily interrupted
 * an operation is still pending completion on the connection,
   e.g. a two-phase commit or an asynchronously running statement
 * there's not enough memory available to complete the operation

Note that in the above cases, .close() will not complete and thus
also not necessarily put the connection in an unusable state. The
exact semantics depend on the database backend.

Example: You may have a temporary network error, so .close() issues
an exception, but still allows to call .close() again after the
issue has been resolved.

> The DBAPI says about conn.close():
>
>     The connection will be unusable from this point
>     forward; an Error (or subclass) exception will be raised
>     if any operation is attempted with the connection.
>
> now, is conn.close() an "operation" on the connection? Even if it
> doesn't involve server communication? I don't think an idempotent
> close() violates this rule, so I'm really asking about an
> interpretation of the DBAPI, not a change.

See above. .close() does imply a few operations that can fail.
You are basically suggesting to silence and ignore those exceptions.
That's generally not a good idea, since the application may want
to report those errors to the user or take some other action.

> Another user in the ML pointed out that an idempotent close is also
> the behaviour of the Python file objects.
>
> My opinion is that the DBAPI test suite can be relaxed and
> conn.close() should have no effect on a closed connection. What do you
> think?

In summary, I'd rather not have .close() silence exceptions.

What we could do is expose an attribute connection.closed which
gets set to True by a successful run of connection.close().
That way you can test for the "unusable" state on a connection,
e.g.

if not connection.closed:
     connection.close()

--
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Oct 19 2011)
>>> Python/Zope Consulting and Support ...        http://www.egenix.com/
>>> mxODBC.Zope.Database.Adapter ...             http://zope.egenix.com/
>>> mxODBC, mxDateTime, mxTextTools ...        http://python.egenix.com/
________________________________________________________________________

::: Try our new mxODBC.Connect Python Database Interface for free ! ::::


   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
    D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
           Registered at Amtsgericht Duesseldorf: HRB 46611
               http://www.egenix.com/company/contact/
_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig

[Federico Di Gregorio]

On 19/10/11 09:59, M.-A. Lemburg wrote:
[snip]
>> risks to become a problem, with the close() that may raise an error
>> > because the connection has been implicitly closed by a communication
>> > error. Note that close() in itself often is a quite safe operation,
>> > not involving database communication, so it is not expected to fail
>> > and may not be well guarded.
> That last sentence is not quite correct: .close() issues an implicit
> .rollback() and usually also tells the database backend to free up
> resources maintained for the connection, so it does require communication
> with the backend.

Here is the problem. Does .close() always issue an implicit .rollback()?
The DBAPI says yes but, as noted previously, the driver can choose to
NOT send a rollback. In fact on the second .close() the driver SHOULD
NOT send a rollback because there is no transaction in progress.

I'd vote for idempotent .close() interpretation too.

federico

--
Federico Di Gregorio                                       [hidden email]
              Una nazionale senza neanche una nazione. -- macchinavapore


_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig

[M.-A. Lemburg]

Federico Di Gregorio wrote:

> On 19/10/11 09:59, M.-A. Lemburg wrote:
> [snip]
>>> risks to become a problem, with the close() that may raise an error
>>>> because the connection has been implicitly closed by a communication
>>>> error. Note that close() in itself often is a quite safe operation,
>>>> not involving database communication, so it is not expected to fail
>>>> and may not be well guarded.
>> That last sentence is not quite correct: .close() issues an implicit
>> .rollback() and usually also tells the database backend to free up
>> resources maintained for the connection, so it does require communication
>> with the backend.
>
> Here is the problem. Does .close() always issue an implicit .rollback()?

Yes.

> The DBAPI says yes but, as noted previously, the driver can choose to
> NOT send a rollback.

Not if it complies to the DB-API.

> In fact on the second .close() the driver SHOULD
> NOT send a rollback because there is no transaction in progress.

The second .close() would in that case raise an exception
as per the definition in the database, since it has become
unusable :-)

BTW: Regardless of whether the driver explicitly issues a rollback
or not, the database backend will roll back the connection if the
connection closes and there's a pending transaction.

> I'd vote for idempotent .close() interpretation too.

Please read my full reply: the implicit rollback is not
the only operation that can fail. Silencing errors is not
a good idea.

--
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Oct 19 2011)
>>> Python/Zope Consulting and Support ...        http://www.egenix.com/
>>> mxODBC.Zope.Database.Adapter ...             http://zope.egenix.com/
>>> mxODBC, mxDateTime, mxTextTools ...        http://python.egenix.com/
________________________________________________________________________

::: Try our new mxODBC.Connect Python Database Interface for free ! ::::


   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
    D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
           Registered at Amtsgericht Duesseldorf: HRB 46611
               http://www.egenix.com/company/contact/
_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig

[Daniele Varrazzo-2]

On Wed, Oct 19, 2011 at 8:59 AM, M.-A. Lemburg <[hidden email]> wrote:

>> risks to become a problem, with the close() that may raise an error
>> because the connection has been implicitly closed by a communication
>> error. Note that close() in itself often is a quite safe operation,
>> not involving database communication, so it is not expected to fail
>> and may not be well guarded.
>
> That last sentence is not quite correct: .close() issues an implicit
> .rollback() and usually also tells the database backend to free up
> resources maintained for the connection, so it does require communication
> with the backend.

If the rollback is implicit in the backend instead of in the driver,
the driver can just cut the communication with the server and obtain
the dbapi semantics without issuing a new command. While this is just
a trick when close() is called explicitly, it is about necessary on
del: not only sending a rollback is a complex operation to be executed
by __del__ (just because: it can fail and del would swallow the
exception, thing we don't like): in complex environments (such as
nonblocking aynchronous communication) pushing the rollback to the
server and reading its response implies interaction between other
python code and the connection being destroyed, whose results are at
best undetermined.


>> now, is conn.close() an "operation" on the connection? Even if it
>> doesn't involve server communication? I don't think an idempotent
>> close() violates this rule, so I'm really asking about an
>> interpretation of the DBAPI, not a change.
>
> See above. .close() does imply a few operations that can fail.
> You are basically suggesting to silence and ignore those exceptions.
> That's generally not a good idea, since the application may want
> to report those errors to the user or take some other action.

No, I'm not suggesting to silence exception: I'm suggesting to not
enforce an exception throw when it is not necessary.


> What we could do is expose an attribute connection.closed which
> gets set to True by a successful run of connection.close().
> That way you can test for the "unusable" state on a connection,
> e.g.
>
> if not connection.closed:
>     connection.close()

We already have such attribute as an extension. I am basically
suggesting this pattern to be implicit in the close() instead of
having people to code it everywhere.


-- Daniele
_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig

[M.-A. Lemburg]

Thinking about this some more ...

Perhaps what you're really after is the following and I was just
misunderstanding the original proposal:

class Connection:

    closed = False

    def close(self):
        if self.closed:
            return
        # close the connection
        ...
        self.closed = True

In other words, you don't silence any errors, but instead use
a flag to store the successful close operation and then simply
ignore all future calls to the method without raising an
exception.

M.-A. Lemburg wrote:

> Federico Di Gregorio wrote:
>> On 19/10/11 09:59, M.-A. Lemburg wrote:
>> [snip]
>>>> risks to become a problem, with the close() that may raise an error
>>>>> because the connection has been implicitly closed by a communication
>>>>> error. Note that close() in itself often is a quite safe operation,
>>>>> not involving database communication, so it is not expected to fail
>>>>> and may not be well guarded.
>>> That last sentence is not quite correct: .close() issues an implicit
>>> .rollback() and usually also tells the database backend to free up
>>> resources maintained for the connection, so it does require communication
>>> with the backend.
>>
>> Here is the problem. Does .close() always issue an implicit .rollback()?
>
> Yes.
>
>> The DBAPI says yes but, as noted previously, the driver can choose to
>> NOT send a rollback.
>
> Not if it complies to the DB-API.
>
>> In fact on the second .close() the driver SHOULD
>> NOT send a rollback because there is no transaction in progress.
>
> The second .close() would in that case raise an exception
> as per the definition in the database, since it has become
> unusable :-)
>
> BTW: Regardless of whether the driver explicitly issues a rollback
> or not, the database backend will roll back the connection if the
> connection closes and there's a pending transaction.
>
>> I'd vote for idempotent .close() interpretation too.
>
> Please read my full reply: the implicit rollback is not
> the only operation that can fail. Silencing errors is not
> a good idea.
>

--
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Oct 19 2011)
>>> Python/Zope Consulting and Support ...        http://www.egenix.com/
>>> mxODBC.Zope.Database.Adapter ...             http://zope.egenix.com/
>>> mxODBC, mxDateTime, mxTextTools ...        http://python.egenix.com/
________________________________________________________________________

::: Try our new mxODBC.Connect Python Database Interface for free ! ::::


   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
    D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
           Registered at Amtsgericht Duesseldorf: HRB 46611
               http://www.egenix.com/company/contact/
_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig

[Daniele Varrazzo-2]

On Wed, Oct 19, 2011 at 9:49 AM, M.-A. Lemburg <[hidden email]> wrote:

> Thinking about this some more ...
>
> Perhaps what you're really after is the following and I was just
> misunderstanding the original proposal:
>
> class Connection:
>
>    closed = False
>
>    def close(self):
>        if self.closed:
>            return
>        # close the connection
>        ...
>        self.closed = True
>
> In other words, you don't silence any errors, but instead use
> a flag to store the successful close operation and then simply
> ignore all future calls to the method without raising an
> exception.

Yep, the idea is just this :) Seems a reasonable behaviour but it
currently raises an exception in the DBAPI unit test, which I feel a
little bit on the strict side.

-- Daniele
_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig

[James William Pye-6]

On Oct 18, 2011, at 10:36 PM, Michael Bayer wrote:
> I'm +1 for close() on a closed connection being harmless.  rollback() already has this behavior, as the spec specifies "rolls back the start of any pending transaction", thus allowing for no activity if no transaction is present.

yep.. +1

cheers, jwp
_______________________________________________
DB-SIG maillist  -  [hidden email]
http://mail.python.org/mailman/listinfo/db-sig