#1968 frontend: large project modification timeout fix
Merged 20 days ago by frostyx. Opened a month ago by praiskup.
copr/ praiskup/copr speedup-project-modify  into  main

@@ -22,7 +22,7 @@ 

  

  from werkzeug.urls import url_encode

  

- from copr_common.enums import EnumType

+ from copr_common.enums import EnumType, StatusEnum

  # TODO: don't import BuildSourceEnum from helpers, use copr_common.enum instead

  from copr_common.enums import BuildSourceEnum # pylint: disable=unused-import

  from copr_common.rpm import splitFilename
@@ -46,7 +46,8 @@ 

  PROJECT_RPMS_DL_STAT_FMT = "project_rpms_dl_stat:hset::{copr_user}@{copr_project_name}"

  

  

- FINISHED_STATUSES = ["succeeded", "forked", "canceled", "skipped", "failed"]

+ FINISHED_STATES = ["succeeded", "forked", "canceled", "skipped", "failed"]

+ FINISHED_STATUSES = [StatusEnum(s) for s in FINISHED_STATES]

  

  

  class WorkList:
@@ -646,16 +647,18 @@ 

      """

      By giving ``what`` string (e.g. "build") and ``items`` array, return string

      in either of those formats:

-     - "builds 1, 2, 3 and 4[ are]"

+     - "builds 1, 2, 3, and 4[ are]"

+     - "builds 1, 2, and others[ are]"

      - "builds 1 and 2[ are]"

      - "build 31[ is]"

      """

      if len(items) > 1:

-         return "{}s {} and {}{}".format(

-             what,

-             ', '.join(str(item) for item in items[:-1]),

-             str(items[-1]),

-             " are" if be_suffix else ""

+         return "{what}s {list}{comma} and {last}{be_suffix}".format(

+             what=what,

+             list=', '.join(str(item) for item in items[:-1]),

+             comma=',' if len(items) > 2 else "",

+             last=str(items[-1]),

+             be_suffix=" are" if be_suffix else "",

          )

      return "{} {}{}".format(

          what,

@@ -1020,21 +1020,32 @@ 

              # iteration

              to_remove.append(mock_chroot)

  

+         shortened = False

          running_builds = set()

          for mc in to_remove:

-             for bch in chroot_map[mc].build_chroots:

-                 if not bch.finished:

-                     running_builds.add(bch.build_id)

-                     continue

+             # We don't overwhelm  the user with too many build IDs in

+             # the error message.

+             max_items = 5

+             copr_chroot = chroot_map[mc]

+             query = CoprChrootsLogic.unfinished_buildchroot(copr_chroot).limit(

+                 max_items + 1)

+             for bch in query:

+                 if len(running_builds) >= max_items:

+                     shortened = True

+                     break

+                 running_builds.add(bch.build_id)

              cls.remove_copr_chroot(flask.g.user, chroot_map[mc])

  

+ 

          # reject the request when some build_chroots are not yet finished

          if running_builds:

+             builds = list(running_builds)

+             if shortened:

+                 builds.append("others")

              raise exceptions.ConflictingRequest(

                  "Can't drop chroot from project, related "

                  "{} still in progress".format(

-                     helpers.pluralize("build", list(running_builds),

-                                       be_suffix=True)))

+                     helpers.pluralize("build", builds, be_suffix=True)))

  

  

      @classmethod
@@ -1119,6 +1130,18 @@ 

          return query.filter(models.CoprChroot.delete_after

                              < datetime.datetime.now())

  

+     @classmethod

+     def unfinished_buildchroot(cls, copr_chroot):

+         """

+         Query returning list of unfinished BuildChroots assigned to

+         the given CoprChroot.

+         """

+         statuses = helpers.FINISHED_STATUSES

+         return (

+             models.BuildChroot.query

+             .filter(models.BuildChroot.copr_chroot==copr_chroot)

+             .filter(not_(models.BuildChroot.status.in_(statuses)))

+         )

  

  class CoprScoreLogic:

      """

@@ -1327,7 +1327,7 @@ 

          if self.finished_early:

              return True

          if not self.build_chroots:

-             return StatusEnum(self.source_status) in helpers.FINISHED_STATUSES

+             return StatusEnum(self.source_status) in helpers.FINISHED_STATES

          return all([chroot.finished for chroot in self.build_chroots])

  

      @property
@@ -1879,7 +1879,7 @@ 

      def finished(self):

          if self.build.finished_early:

              return True

-         return self.state in helpers.FINISHED_STATUSES

+         return self.state in helpers.FINISHED_STATES

  

      @property

      def task_id(self):

@@ -295,7 +295,8 @@ 

  def test_pluralize():

      """ test generic I/O for helpers.pluralize """

      # we don't explicitly re-order

-     assert pluralize("build", [2, 1, 3], be_suffix=True) == "builds 2, 1 and 3 are"

+     assert pluralize("build", [2, 1, 3], be_suffix=True) == "builds 2, 1, and 3 are"

+     assert pluralize("build", [2, 1, "others"], be_suffix=True) == "builds 2, 1, and others are"

      assert pluralize("action", [1], be_suffix=True) == "action 1 is"

      assert pluralize("sth", [1, 2], be_suffix=False) == "sths 1 and 2"

      assert pluralize("a", [2], be_suffix=False) == "a 2"

On @rubygems/rubygems in Fedora Copr, we faced timeouts when we tried to
disable mock chroots with too many BuildChroots. Turns out ORM lazy
loaded all the BuildChroot instances for disabled CoprChroot (which in
our case was more than 100k items).

Instead of this, prepare a custom query for related BuildChroots that
can be used with limit() safely.

For the purpose of the query, we need to use FINISHED_STATUSES variable
which was improperly using states, not statuses. So I'm fixing this
(status is the number, state is the string human-readable alternative).

Fixes: #1925

Build succeeded.

rebased onto 7adf903

a month ago

Build succeeded.

Please take a look at this one.

Just for the record the message for more then max_items looks like this

Can't drop chroot from project, related builds 2679387, 2679389, 2679390 and others are still in progress

I like the shortening.

Nitpick - there should be a comma before "and others". Also, I would reduce the max_items from 20 to 5 or so. But those are really minor things, it is +1 from me even as is.

rebased onto a36779c

a month ago

Thank you for the review. Updated, PTAL.

Build succeeded.

Thank you for the changes.

rebased onto 0c4c82c

20 days ago

Commit 0c4c82c fixes this pull-request

Pull-Request has been merged by frostyx

20 days ago

Pull-Request has been merged by frostyx

20 days ago

Build succeeded.