#2299 hub: query_buildroots have to return ASAP
Merged 2 years ago by mikem. Opened 2 years ago by tkopecek.
tkopecek/koji issue2298  into  master

file modified
+6
@@ -5381,6 +5381,8 @@ 

                                 values=locals())

          result = set(query.execute())

          candidate_buildroot_ids = result

+         if not candidate_buildroot_ids:

+             return []

  

      if archiveID is not None:

          joins.insert(0, 'buildroot_archives ON buildroot.id = buildroot_archives.buildroot_id')
@@ -5393,6 +5395,8 @@ 

              candidate_buildroot_ids &= result

          else:

              candidate_buildroot_ids = result

+         if not candidate_buildroot_ids:

+             return []

  

      if taskID is not None:

          clauses.append('standard_buildroot.task_id = %(taskID)i')
@@ -5404,6 +5408,8 @@ 

              candidate_buildroot_ids &= result

          else:

              candidate_buildroot_ids = result

+         if not candidate_buildroot_ids:

+             return []

  

      if candidate_buildroot_ids:

          candidate_buildroot_ids = list(candidate_buildroot_ids)

If candidate_buildroot_ids are pruned to zero in some point,
query_buildroots have to return. Otherwise big working queryset will be
created eating resources and returning empty list anyway.

Fixes: https://pagure.io/koji/issue/2298

This changes the behavior in the case where both an rpmID and an archiveID is given. Before these conditions were effectively ORed, because the second stanza would add to candidate_buildroot_ids. With this change, if rpmID is given and that first stanza yields no buildroots, then we return None, even if the archiveID param might yield some.

Granted, I'm not 100% what we should be doing when both are given, or if we even really want to support that.

Nevermind, I think I'm reading this wrong

Looking at this, I realize that the original optimization from #2074 leaves the original joins and clauses in place for the rpmID and archiveID cases, even though they are essentially obsoleted.

This means two things,

  • In the "not used in buildroot" case this PR is addressing, the behavior should be no worse than before #2074
  • In these cases, we are still performing these joins, which seems like a potential performance hit.

Granted, we do need to perform the join to return the is_update field in the rpmID case (though I wonder if we actually need that field). Otoh, the join in the archiveID case seems entirely redundant.

I guess we do not have to address the above here, though. This fix looks fine. We just might want to follow up.

:thumbsup:

Commit 9fa07f6 fixes this pull-request

Pull-Request has been merged by mikem

2 years ago

I've filed #2300 for the follow up

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

2 years ago

Metadata Update from @mfilip:
- Pull-request tagged with: testing-done

2 years ago