Sitemaps with i18n=True fetch the entire queryset of items before paginating

[Julien Palard]

Hi!

It's a bit like: https://code.djangoproject.com/ticket/11572

when enabling i18n on a sitemap, _items uses the following code:

            # Create (item, lang_code) tuples for all items and languages.
            # This is necessary to paginate with all languages already considered.
            items = [
                (item, lang_code)
                for item in self.items()
                for lang_code in self.get_languages_for_item(item)
            ]

The list comprehension loads the whole table times the number of languages in memory.

We tried with a table containing two millions elements and two languages: it does not fit in 16GB of memory and the process gets killed.

[Julien Palard]

While:

    def get_languages_for_item(self, item):
        """Languages for which this item is displayed."""
        return self._languages()

does not use item, something (non trivial) could be implemented to slice the queryset at the right spot (depending on the number of languages), and the total number of elements could be computed by a simple multiplication.

But as long as get_languages_for_item uses item all of this break.

[Jacob Walls]

Thanks for the report. #33662 introduced eager evaluation of self.items() (if i18n is in use) instead of letting self.items() (a queryset) remain unevaluated until paged by the paginator.

Tentatively accepting as a bug that we should at least respect self.limit.

Could be as simple as something like this (could limit even further by doing some language arithmetic):

django/contrib/sitemaps/__init__.py

@@ -185 +185 @@
 
     def items(self):
         # Make sure to return a clone; we don't want premature evaluation.
-        return self.queryset.filter()
+        return self.queryset[:self.limit]
 
     def lastmod(self, item):
         if self.date_field is not None:

---

I'm not seeing what you initially suggested about a cartesian product of items and languages -- if the order of the for loops in the comprehension were reversed, then I'd see the problem, but it's not, so it's the same item. Am I missing something?

(Pdb) items
[(<I18nTestModel: I18nTestModel object (1)>, 'en'), (<I18nTestModel: I18nTestModel object (1)>, 'pt')]
(Pdb) items[0][0] is items[1][0]
True

[Julien Palard]

I'm not seeing what you initially suggested about a cartesian product

You're right, it's not a cartesian product database-side, it's only a carthesian product Python-side, so 8 bytes per object, so ~15 MB per language for my 2 million objects, that's nothing compared to the 16GB+ that it takes currently.

This can't be as simple as return self.queryset[:self.limit] because the limit is not a limit overall but a limit per page, so putting the limit here imples a single page (the pagination occurs over the result of items: return paginator.Paginator(self._items(), self.limit)).

The path of doing arithmetics to be able to query the right slice of items depending on the requested page *and* the number of languages breaks if get_languages_for_item uses its item argument.

[Jacob Walls]

Right, okay. Maybe we can pursue checking if get_languages_for_item has been overridden and if not, avoid the materialization of the items before paginating.

[Julien Palard]

Thinking of while eating today, did not checked the code or tried an implementation, but:

The limit being just a limit, so in cases when an URL has no translation we could just not add it to the page, leaving some pages not comptely full, but this could be OK.

It would greatly ease pagination in case get_languages_for_item is overloaded.

The "risk" being: in a site with many languages, say 20, and a low limit, say 100 URLs per page, but very few URLs being translated, say less than 1%, then we would give only 5 URLs per page, leaving 95 « empty spots » for future translations. This could be considered as a bug.

[Varun Kasyap Pentamaraju]

Deassigned. Thanks

[Augusto Pontes]

​https://github.com/django/django/pull/20268