Actually fix race condition when creating sidetags in Koji

Signed-off-by: Nikola Forró <[email protected]>
packit · Nov 11, 2024 · 7d4df20 · 7d4df20
1 parent bea3f49
commit 7d4df20
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 20 deletions.
diff --git a/packit_service/models.py b/packit_service/models.py
@@ -3974,18 +3974,24 @@ class SidetagModel(Base):
     @contextmanager
     def get_or_create_for_updating(
         cls,
-        group: "SidetagGroupModel",
+        group_name: str,
         target: str,
     ) -> Generator["SidetagModel", None, None]:
         """
         Context manager that gets or creates a sidetag upon entering the context,
         provides a corresponding instance of `SidetagModel` to be updated within the context
         and commits the changes upon exiting the context, all within a single transaction.
         """
-        session = singleton_session or Session()
-        session.begin()
+        with sa_session_transaction(commit=True) as session:
+            # lock the sidetag group in the DB by using SELECT ... FOR UPDATE
+            # all other workers accessing this group will block here until the context is exited
+            group = (
+                session.query(SidetagGroupModel)
+                .filter_by(name=group_name)
+                .with_for_update()
+                .first()
+            )
 
-        try:
             sidetag = session.query(cls).filter_by(sidetag_group_id=group.id, target=target).first()
             if not sidetag:
                 sidetag = cls()
@@ -3994,24 +4000,9 @@ def get_or_create_for_updating(
 
                 group.sidetags.append(sidetag)
                 session.add(group)
-        except Exception as ex:
-            logger.warning(f"Exception while working with database: {ex!r}")
-            session.rollback()
-            raise
 
-        try:
             yield sidetag
-        except Exception:
-            session.rollback()
-            raise
-
-        try:
             session.add(sidetag)
-            session.commit()
-        except Exception as ex:
-            logger.warning(f"Exception while working with database: {ex!r}")
-            session.rollback()
-            raise
 
     @classmethod
     def get_by_koji_name(cls, koji_name: str) -> Optional["SidetagModel"]:

diff --git a/packit_service/worker/helpers/sidetag.py b/packit_service/worker/helpers/sidetag.py
@@ -150,7 +150,7 @@ def get_or_create_sidetag(cls, sidetag_group: str, dist_git_branch: str) -> Side
             PackitException if the sidetag failed to be created in Koji.
         """
         group = SidetagGroupModel.get_or_create(sidetag_group)
-        with SidetagModel.get_or_create_for_updating(group, dist_git_branch) as sidetag:
+        with SidetagModel.get_or_create_for_updating(group.name, dist_git_branch) as sidetag:
             if not sidetag.koji_name or not cls.koji_helper.get_tag_info(
                 sidetag.koji_name,
             ):