test suites that make new engines for each test will blow up the LRU cache

Issue #4071 resolved
Mike Bayer repo owner created an issue

firstly, that those dialects aren't garbage collected as quickly, and then that we emit a warning, test suites that raise on warning will blow up. this is currently illustrated in the keystone test suite. pdb and listing out keys:

(<sqlalchemy.dialects.sqlite.pysqlite.SQLiteDialect_pysqlite object at 0x7f4d4725d290>, <sqlalchemy.sql.dml.Insert object at 0x7f4d481f6fd0>, ('description', 'domain_id', 'enabled', 'extra', 'id', 'is_domain', 'name', 'parent_id'), 0, False)
(<sqlalchemy.dialects.sqlite.pysqlite.SQLiteDialect_pysqlite object at 0x7f4d475b61d0>, <sqlalchemy.sql.dml.Insert object at 0x7f4d481f6fd0>, ('description', 'domain_id', 'enabled', 'extra', 'id', 'is_domain', 'name', 'parent_id'), 0, False)
(<sqlalchemy.dialects.sqlite.pysqlite.SQLiteDialect_pysqlite object at 0x7f4d47048550>, <sqlalchemy.sql.dml.Insert object at 0x7f4d481f6fd0>, ('description', 'domain_id', 'enabled', 'extra', 'id', 'is_domain', 'name', 'parent_id'), 0, False)
(<sqlalchemy.dialects.sqlite.pysqlite.SQLiteDialect_pysqlite object at 0x7f4d473bd990>, <sqlalchemy.sql.dml.Insert object at 0x7f4d481f6fd0>, ('description', 'domain_id', 'enabled', 'extra', 'id', 'is_domain', 'name', 'parent_id'), 0, False)
(<sqlalchemy.dialects.sqlite.pysqlite.SQLiteDialect_pysqlite object at 0x7f4d473b9cd0>, <sqlalchemy.sql.dml.Insert object at 0x7f4d481f6fd0>, ('description', 'domain_id', 'enabled', 'extra', 'id', 'is_domain', 'name', 'parent_id'), 0, False)
(<sqlalchemy.dialects.sqlite.pysqlite.SQLiteDialect_pysqlite object at 0x7f4d470acf10>, <sqlalchemy.sql.dml.Insert object at 0x7f4d481f6fd0>, ('description', 'domain_id', 'enabled', 'extra', 'id', 'is_domain', 'name', 'parent_id'), 0, False)

I'm pretty relieved this is all it is, was super worried the cache was failing in some weirder way. it might be time to restructure how the compiled cache works, it needs to be mapper local and dialect local which means we probably need another elaborate weakref scheme like the event system has.

Comments (3)

  1. Mike Bayer reporter

    in fact, these are ORM inserts, so this same situation was already occurring in keystone's suite for a long time :) we just weren't warning on it. mayyybeeee we just kill the warning :).

  2. Mike Bayer reporter

    Remove LRU warnings

    Removed the warnings that are emitted when the LRU caches employed by the mapper as well as loader srtategies reach their threshold; the purpose of this warning was at first a guard against excess cache keys being generated but became basically a check on the "creating many engines" antipattern. While this is still an antipattern, the presense of test suites which both create an engine per test as well as raise on all warnings will be an inconvenience; it should not be critical that such test suites change their architecture just for this warning (though engine-per-test suite is always better).

    Change-Id: I41ef8cd642d05a845f53119b196440f9d7879cd9 Fixes: #4071

    → <<cset e83024d3fa45>>

  3. Log in to comment