clarify server-side vs. client-side isolation level, as there's no way to retrieve the AUTOCOMMIT setting from a connection w/o using default

Issue #3357 new
Timothy Cardenas created an issue

Simply create a engine with isolation_level set to autocommit, connect, and compare the return value from get_isolation_level from the value returned from the psycopg2 connection. They do not match.

Example:

url='postgresql+psycopg2://me:pass@host/db'
engine = create_engine(
            url,
            encoding='utf8',
            pool_size=5,
            poolclass=QueuePool,
            isolation_level="AUTOCOMMIT"
        )

with engine.connect() as connection:
  print(connection.get_isolation_level())  # => READ COMMITED
  print(connection.connection.isolation_level)  # => 0 (psycopg2.extensions.ISOLATION_LEVEL_AUTOCOMMIT)

Comments (11)

  1. Timothy Cardenas reporter

    Might be related to this but i found that if I opened a transaction with isolation_level='autocommit' i was able work within the transaction and commit successfully but was unable to read any of the changes i made while within the transaction like one would expect. If i changed the isolation level right before going into the transaction to READ COMMITTED i noticed that my ability to read changes within a transaction was restored.

  2. Mike Bayer repo owner

    I'm not sure where to go on this because get_isolation_level() is documented as reading the level using a SQL query, and that is what it's set to on the database side:

    This attribute will typically perform a live SQL operation in order to procure the current isolation level, so the value returned is the actual level on the underlying DBAPI connection regardless of how this state was set.

    "AUTOCOMMIT" is a client-side setting only; there's no such transaction isolation level as "AUTOCOMMIT". It's just because the way the DBAPI has no begin(), the DBAPI vendors all seem to want to bundle "AUTOCOMMIT" into this setting as well.

    Note there is already default_isolation_level which will give you the Python-level setting and should return AUTOCOMMIT here:http://docs.sqlalchemy.org/en/rel_0_9/core/connections.html?highlight=get_isolation_level#sqlalchemy.engine.Connection.default_isolation_level.

    in this case, clarifying the methods would mean that get_isolation_level() would need to be broken into get_client_isolation_level() and/or get_server_isolation_level(). At the very least, the notes on get_isolation_level() should be expanded here to note that it currently is a server-side isolation level.

  3. Timothy Cardenas reporter

    Thats a really good point about DBAPI client vs DB Server isolation level issues. I would expect most people when they call get_isolation_level() would want what the DB Server is reporting as the isolation level and not some autocommit isolation setting that psycopg2 stuffed into the isolation_level accessor.

    However then i probably shouldn't be able to specify AUTOCOMMIT as a isolation_level at the create_engine() call for consistency. I think it makes sense that a user of sqlalchemy should not have to worry about the underlying dbapi driver and simply state isolation_levels as they exist in the DB and as a completely separate option have the ability to enable/disable autocommit at the dbapi driver level.

    To future readers: My paragraph below is incorrect, see Mike and my follow ups

    I believe in postgresql you can't even enter into a transaction when your connection is in dbapi autocommit mode so the isolation level settings are worthless if you turn on autocommit. This means that you could have a nice clean interface/accessor-set for isolation_levels and a toggle for "true" autocommit at the dbapi level. The concerns seem to separate themselves pretty nicely right? (is there something i am missing?)

  4. Mike Bayer repo owner

    However then i probably shouldn't be able to specify AUTOCOMMIT as a isolation_level at the create_engine() call for consistency.

    sure, but we just added that feature within the last couple of years since that's what everyone was trying to do anyway, since that's how all the DBAPIs are doing it, so I don't think that can change now.

    I think it makes sense that a user of sqlalchemy should not have to worry about the underlying dbapi driver and simply state isolation_levels as they exist in the DB and as a completely separate option have the ability to enable/disable autocommit at the dbapi driver level.

    it's too late now to take away "AUTOCOMMIT" as an isolation level setting, though we can potentially add a separate flag to supersede it (though we've changed so much in this area recently I sort of don't want to rush ahead with more changes right now).

    I believe in postgresql you can't even enter into a transaction when your connection is in dbapi autocommit mode so the isolation level settings are worthless if you turn on autocommit.

    yeah you can. Just emit the SQL command "BEGIN". psycopg2 makes this area very difficult because you explicitly cannot set isolation levels through SQL if the connection isn't in autocommit, b.c. psycopg2 explicitly sends out its isolation level setting to the connection every time. This is sort of why they have AUTOCOMMIT as an isolation level setting; it's more like, "how should psycopg2 start a transaction?". They also have a "connection.autocommit" flag anyway. Neither mode is explained as "deprecated", so it's not immediately obvious which one is preferred, however at http://initd.org/psycopg/docs/usage.html#transactions-control they make it clear: "you can use the autocommit property (set_isolation_level() in older versions).". So they've flipped on this a bit, but if in fact the ".autocommit" flag is how they think it should go, then we'd keep our own get_isolation_level() as the server side level and start to move away from "AUTOCOMMIT" as an isolation level. But like I said this keeps changing a lot so I'm not in a hurry to keep flipping it around...more documentation and notes would be best for the moment.

  5. Timothy Cardenas reporter

    Summary

    I think its fine if you just want to update the docs for practical concerns noted above. However i wanted to go deeper and get a better understanding. I learned that autocommit and isolation_levels are orthogonal. You can be in autocommit and have any of the isolation_levels apply at the same time. I believe that overloading isolation_level with a autocommit setting is a mistake and should be avoided. Perhaps in some future version of Sqlalchemy and probably psycopg2 they should break those concepts apart.

    Research

    Here are some (what i believe to be) facts from the research i did.

    • Postgresql doesn't let you disable autocommit mode. You can enter into a transaction but as soon as it commits/rollsback you are right back into autocommit mode. (source: 1)

    • When you are in autocommit mode (default for postgresql) the db is performing begin/commit pairs (aka transactions) on each statement implicitly. (source: 2)

    • After reading through all the isolation levels (source: 3) there are significant implications from changing the "default isolation level" even when you are in autocommit (because after all serialization is still an issue if you are not in an explicit transaction).

    • psycopg2 acts in a silly manner if i am understanding their code (source: 4). If you select the autocommit isolation level it will set the session to the "default" which i believe is the one setup in your postgresql config file.(source: 5) (note: i am not 100% confident it pulls from just the postgresql config file, but please feel free to check me)

    To check the implications of these findings i did a very simple test with two open psql terminals to see if transaction levels affect autocommit statements. TLDR they do. (see test at end of comment)

    A potential result from all this testing is that overloading isolation_level with autocommit doesn't seem to make sense.

    Sources

    1. http://stackoverflow.com/questions/17927821/postgresql-trigger-disable-auto-commit-and-set-isolation-level

    2. http://www.postgresql.org/docs/9.1/static/sql-begin.html

    3. http://www.postgresql.org/docs/9.1/static/transaction-iso.html

    4. https://github.com/psycopg/psycopg2/blob/e7fc7f31b9c1c405b57c83b46475eb8618a66362/psycopg/connection_int.c#L1116-1122

    5. http://www.postgresql.org/docs/9.1/static/runtime-config-client.html#GUC-DEFAULT-TRANSACTION-ISOLATION

    Quick PSQL Test

    Terminal 1

    SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL REPEATABLE READ;
    BEGIN;
    UPDATE my_table SET my_column = 'X' where id = 1;
    
    # hold here and execute command in terminal 2
    
    COMMIT;
    

    Terminal 2

    SET SESSION CHARACTERISTICS AS TRANSACTION ISOLATION LEVEL REPEATABLE READ;
    
    # Autocommit statement
    UPDATE my_table SET my_column = 'Y' where id = 1;
    
    # Will block here until first terminal commits.
    
    ERROR:  could not serialize access due to concurrent update
    
  6. Timothy Cardenas reporter

    Hey Mike, I was going back and refactoring some of the code i had working that was retrieving the current db client isolation_level and noticed that when i switched to your suggestion above: use default_isolation_level it still was not retrieving the correct isolation level. It seems like just not possible to get the correct isolation level for the db client via SqlAlchemy right now. Could you check it out and see if i am right?

  7. Mike Bayer repo owner

    OK, same issue then, we get the default_isolation_level from the SQL query

    diff --git a/lib/sqlalchemy/dialects/postgresql/psycopg2.py b/lib/sqlalchemy/dialects/postgresql/psycopg2.py
    index f66ba96..c22f9f5 100644
    --- a/lib/sqlalchemy/dialects/postgresql/psycopg2.py
    +++ b/lib/sqlalchemy/dialects/postgresql/psycopg2.py
    @@ -599,6 +599,7 @@ class PGDialect_psycopg2(PGDialect):
    
             if self.isolation_level is not None:
                 def on_connect(conn):
    +                print "NOW WE'RE SETTING IT !!", self.isolation_level
                     self.set_isolation_level(conn, self.isolation_level)
                 fns.append(on_connect)
    
    diff --git a/lib/sqlalchemy/engine/default.py b/lib/sqlalchemy/engine/default.py
    index 763e85f..486bd75 100644
    --- a/lib/sqlalchemy/engine/default.py
    +++ b/lib/sqlalchemy/engine/default.py
    @@ -239,6 +239,7 @@ class DefaultDialect(interfaces.Dialect):
             try:
                 self.default_isolation_level = \
                     self.get_isolation_level(connection.connection)
    +            print "GET DEFAULT LEVEL!!", self.default_isolation_level
             except NotImplementedError:
                 self.default_isolation_level = None
    
    from sqlalchemy import create_engine
    e = create_engine("postgresql+psycopg2://scott:tiger@localhost/test", isolation_level="AUTOCOMMIT")
    c = e.connect()
    
    
    from sqlalchemy import create_engine
    e = create_engine("postgresql+psycopg2://scott:tiger@localhost/test", isolation_level="REPEATABLE READ")
    c = e.connect()
    
    #!
    
    
    $ python test.py
    NOW WE'RE SETTING IT !! AUTOCOMMIT
    GET DEFAULT LEVEL!! READ COMMITTED
    NOW WE'RE SETTING IT !! AUTOCOMMIT
    NOW WE'RE SETTING IT !! REPEATABLE READ
    GET DEFAULT LEVEL!! REPEATABLE READ
    NOW WE'RE SETTING IT !! REPEATABLE READ
    

    all pretty broken but nothing is happening here as far as altering existing functionality until 1.1 (new things that are part of a comprehensive featureset but have no impact on existing APIs can be added, however). If you'd like, start opening tickets on that milestone with how the complete API should look from front to back. It might be as simple as just adding an "autocommit" accessor.

  8. Timothy Cardenas reporter

    Yeah! There we go. I think the test output shows the problem we are having at SurveyMonkey. I wrote a wrapper around the sqlalchemy connection for postgresql that turned out to basically be just a "autocommit" accessor. Honestly like my "report" went into you can have a bunch of different isolation levels in the session that affect commits even while in autocommit mode so i think that makes sense.

    There is at least one gotcha associated with the accessor though. Because of pep249 (which i think is a bit silly and short sighted) when you go request the isolation_level you open a connection which under normal (and even with the sqlalchemy autocommit setting) puts you in a transaction where you can't adjust the autocommit setting without committing.

    So when using the autocommit accessor you have to be able to enforce you are not in a transaction if you are trying to enable it.

    Do you have a doc on how you like your feature requests to be submitted otherwise i will just go dump some loose requirements?

  9. Mike Bayer repo owner

    this is all kind of one big ball so if you just write up how the API would look and what the impact would be on exsiting use of the API, I think just opening a ticket here would be fine to start.

  10. Log in to comment