sbp commented on issue #93: URL: https://github.com/apache/tooling-trusted-release/issues/93#issuecomment-2901293871
What we are trying to prevent here is the read counter, increment, and write counter antipattern. In SQLAlchemy, `before_insert` event hooks do not receive any kind of special transactional treatment. This means that, for example, there is no lock obtained which encompasses running the hook and inserting the row. The changes in 9edc6f2b95e9954c4b1938b20e1561cf40f76aee should fix this problem. SQLAlchemy runs `before_insert` event hooks [when `Session.flush` is called](https://docs.sqlalchemy.org/en/20/orm/session_events.html#mapper-level-flush-events), which includes when `Session.commit` is called: > It is important to note that these events apply only to the [session flush operation](https://docs.sqlalchemy.org/en/20/orm/session_basics.html#session-flushing), and not to the ORM-level INSERT/UPDATE/DELETE functionality But due to the changes in 9edc6f2b95e9954c4b1938b20e1561cf40f76aee, we now issue `BEGIN IMMEDIATE` prior to the `Session.commit`. [As the SQLite documentation says](https://sqlite.org/lang_transaction.html): > IMMEDIATE causes the database connection to start a new write immediately, without waiting for a write statement. The BEGIN IMMEDIATE might fail with [SQLITE_BUSY](https://sqlite.org/rescode.html#busy) if another write transaction is already active on another database connection. We set a 30 second timeout in `create_async_engine`, which means that all ATR code should retry upon `BEGIN IMMEDIATE` failing with `SQLITE_BUSY`. This is necessary because the SQLite write lock is global for the whole database across all connections. Previously, if there were a race condition in generating a new Revision number, this would be detected by the integrity checker which disallows duplicate values. But because we now lock using `BEGIN IMMEDIATE`, this should wait and then allocate safely as long as queued allocations do not take longer than 30 seconds. A more elegant fix would be to use the `ON CONFLICT ... DO UPDATE ... RETURNING` syntax that was [added to SQLite 3.35.0](https://www.sqlite.org/releaselog/3_35_0.html) on 2021-03-12. This would allow us to have a counter table from which we can automatically allocate a safe, atomically incremented, new counter value. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@tooling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tooling.apache.org For additional commands, e-mail: dev-h...@tooling.apache.org
