j.c.sackett has proposed merging lp:~jcsackett/launchpad/archivemirror-timeout-618400 into lp:launchpad.
Requested reviews: Launchpad code reviewers (launchpad-reviewers) Related bugs: #618400 Distribution:+archivemirrors timing out in 1% of requests https://bugs.launchpad.net/bugs/618400 For more details, see: https://code.launchpad.net/~jcsackett/launchpad/archivemirror-timeout-618400/+merge/46550 Summary ======= Distribution:+archivemirrors has a timeout caused by a combination of factors: 1) repeated country queries for each distributionmirror that get expensive in aggregate and 2) a very expensive mirror freshness query/calculation. This branch addresses the first cause; a second branch will need to be landed to address the second cause and close bug 618400. In the process there is a slight refactor to remove some repeated code. Proposed fix ============ When getting distribution mirrors, get the countries for the mirrors at the same time, rather than querying for them one by one later. Preimplementation ================= Spoke with Curtis Hovey and William Grant about the country query issue. Implementation ============== lib/lp/registry/interfaces/distribution.py lib/lp/registry/model/distribution.py ------------------------------------- Added a method, _getActiveMirrors, which can generate the resultset for either cdimages or archive mirrors, with or without the country join. Updated the archive_mirrors and cdimage_mirrors properties to use this method. Added an archive_mirrors_by_country and cdimage_mirrors_by_country property that also uses this method, and added them as not-exported CollectionFields to the interface. lib/lp/registry/browser/distribution.py --------------------------------------- Updated the +archivemirrors view and +cdmirrors view to use their respective _by_country property. Updated the parent view to use the returned data properly. lib/lp/registry/model/distributionmirror.py ------------------------------------------- Drive by reformat of the __all__ clause per style guidelines. lib/lp/registry/browser/tests/distribution-views.txt ---------------------------------------------------- Updated test to include archive mirrors alongside cdimage. Tests ===== bin/test -t distribution-views\.txt bin/test -t distributionmirror-views\.txt QA == Make certain that Distribution:+archivemirrors loads fine. Then, make certain it doesn't timeout and ceases to be in the timeout reports. The bug itself doesn't indicate how often the timeout occurs, which makes this a touch difficult to QA. Lint ==== = Launchpad lint = Checking for conflicts and issues in changed files. Linting changed files: lib/lp/registry/browser/distribution.py lib/lp/registry/browser/tests/distribution-views.txt lib/lp/registry/interfaces/distribution.py lib/lp/registry/model/distribution.py lib/lp/registry/model/distributionmirror.py ./lib/lp/registry/model/distributionmirror.py 594: E501 line too long (80 characters) 460: Line exceeds 78 characters. 478: Line exceeds 78 characters. 594: Line exceeds 78 characters. 812: Line exceeds 78 characters. 865: Line exceeds 78 characters. The above lint errors are from a number of queries (not in this branch) that I think are already hard enough to read without odd wrapping changes. -- https://code.launchpad.net/~jcsackett/launchpad/archivemirror-timeout-618400/+merge/46550 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/archivemirror-timeout-618400 into lp:launchpad. _______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : [email protected] Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
