CERT-Polska/mwdb-core

IntegrityError due to non-repetable read made to ensure that operation was done by another transaction

psrok1 opened this issue · 1 comments

Environment information

  • MWDB version (from /about): v2.12.0
  • Installation method:
    • mwdb.cert.pl service
    • From PyPi (pip install mwdb-core)
    • From docker-compose
    • Other (please explain)
  • Plugins installed: not applicable

Behaviour the bug (what happened?)

MWDB Core uses wrong method to recover from potential IntegrityError due to concurrent transaction.

Example from add_tag method:

        db.session.begin_nested()
        try:
            db_tag = Tag(tag=tag_name, object_id=self.id)
            db.session.add(db_tag)
            db.session.commit()
            is_new = True
        except IntegrityError:
            db.session.rollback()
            db.session.refresh(self)
            if db_tag not in self.tags:
                raise

If the same tag was added by concurrent transaction, we're recovering to the savepoint before insertion (db.session.rollback of db.session.begin_nested) but after that we're doing refresh to check if tag was actually added.

Unfortunately, even on READ COMMITTED default isolation level, we don't have non-repetable read for regular SELECT.

When a transaction uses this isolation level, a SELECT query (without a FOR UPDATE/SHARE clause) sees only data committed before the query began; it never sees either uncommitted data or changes committed by concurrent transactions during the query's execution
UPDATE, DELETE, SELECT FOR UPDATE, and SELECT FOR SHARE commands behave the same as SELECT in terms of searching for target rows: they will only find target rows that were committed as of the command start time

Source: https://www.postgresql.org/docs/current/transaction-iso.html

So even if we can enforce our database to perform non-repetable read for data committed by concurrent transaction via SELECT FOR SHARE, we should not rely on that behavior.

Expected behaviour

Don't break operation with ISE 500 and try to recover from conflict in transaction

Ok I misunderstood the documentation.

changes committed by concurrent transactions during the query's execution

READ COMMITTED works as expected.