Tryton - Issues

 

Issue9594

Title Improve speed of products by quantities
Priority feature Status resolved
Superseder Nosy List albertca, ced, pokoli, reviewbot, roundup-bot
Type performance Components stock
Assigned To ced Keywords review
Reviews 323981002,323991002
View: 323981002, 323991002

Created on 2020-09-11.18:18:12 by ced, last changed by roundup-bot.

Messages
New changeset 31c71b5439ff by Cédric Krier in branch 'default':
Use query clause when searching quantity for 1 location
https://hg.tryton.org/tryton-env/rev/31c71b5439ff
New changeset 7e581e7b9674 by Cédric Krier in branch 'default':
Use query clause when searching quantity for 1 location
https://hg.tryton.org/modules/stock/rev/7e581e7b9674
msg60123 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-09-15.10:03:34
Indeed, it is possible to have an optimization for a single location (which is the common case). Here is review323991002
msg60122 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2020-09-15.00:04:56
Missatge de Cédric Krier <bugs@tryton.org> del dia dl., 14 de set. 2020 a
les 18:06:

>
> Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
> Last version limit the list of products (and lots) to only those there
> were stored once in the selected locations.
> I think this solve the concern of msg60071.
> About the performance, the query used to compute the stored in locations,
> is faster than the computation of product quantity by location. Also it is
> using select distinct which can be improved with loose indexscan (when
> implemented in Postgresql:
> https://wiki.postgresql.org/wiki/Loose_indexscan).
>

I appreciate the effort but to be honest in my opinion it is better to list
all products and let the user search afterwards than show a shorter list of
"random" products. It makes no sense to list a product with quantity = 0
just because 5 years ago the product was there for a week.

However, I think that we can make an optimization that will allow to
compute the quantity efficiently with a single SQL query.

In http://hg.tryton.org/modules/stock/file/tip/move.py#l1471 we check if
we're computing the quantity for a single location (even if it has
children) so that we don't need to propagate quantities but simply sum the
result returned by the SQL query.

The same could be done at the query level and simply avoid the grouping by
location in this case (
http://hg.tryton.org/modules/stock/file/tip/move.py#l1433), thus simply
doing GROUP BY product + HAVING SUM(internal_quantity) <> 0.

Or am I missing something?

Also performance will probably be improved once we only compute the total
number of records when the user requests it as discussed here:
https://discuss.tryton.org/t/counters-performance/1206.
review323981002 updated at https://codereview.tryton.org/323981002/#ps298561002
msg60118 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-09-14.18:06:16
Last version limit the list of products (and lots) to only those there were stored once in the selected locations.
I think this solve the concern of msg60071.
About the performance, the query used to compute the stored in locations, is faster than the computation of product quantity by location. Also it is using select distinct which can be improved with loose indexscan (when implemented in Postgresql: https://wiki.postgresql.org/wiki/Loose_indexscan).
review323981002 updated at https://codereview.tryton.org/323981002/#ps300401002
review323981002 updated at https://codereview.tryton.org/323981002/#ps312281002
msg60071 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2020-09-12.01:47:00
Missatge de Cédric Krier <bugs@tryton.org> del dia ds., 12 de set. 2020 a
les 1:37:

>
> Cédric Krier <cedric.krier@b2ck.com> added the comment:
>
> On 2020-09-12 01:21, Albert Cervera i Areny wrote:
> > Missatge de Cédric Krier <bugs@tryton.org> del dia dv., 11 de set. 2020
> a
> > les 18:18: <cedric.krier@b2ck.com>
> >
> > > I think that it is not really needed to filter by quantities for this
> view
> > > and instead we can just filter product of type service.
> > >
> >
> > I don't agree at 100% with this sentence. I think that users usually only
> > want to see products that have quantity != 0.
>
> For me it does not apply to any normal scenario.
> There are two cases, DB with small number of product so there is no
> problem showing them. Or DB with large number of product so user will
> have any way to filter for what he is looking for. And what user may be
> looking for is that there is no quantity for a product.
>

I agree with those two scenarios but I was thinking more on a third one:

DB with say 1000 products but with few products on each location.

>
> > I see a couple of alternatives (maybe both are desirable):
> >
> > - Open a couple of tabs. The default one shows all products and the other
> > one applies the existing filter.
>
> User who wants such slow filter can bookmark it. There is no point to
> make tab domain.
>
> > - Try to optimize the searcher which it seems to me it will eventually be
> > necessary. We should probably give CTE a chance and see if we can make it
> > faster.
>
> Recursive CTE are not of any order faster. The only saving is the
> communication. I already proved that previously for the MPTT. So I will
> not try this dead end again myself.
>

Understood.
msg60068 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-09-12.01:37:03
On 2020-09-12 01:21, Albert Cervera i Areny wrote:
> Missatge de Cédric Krier <bugs@tryton.org> del dia dv., 11 de set. 2020 a
> les 18:18: <cedric.krier@b2ck.com>
> 
> > I think that it is not really needed to filter by quantities for this view
> > and instead we can just filter product of type service.
> >
> 
> I don't agree at 100% with this sentence. I think that users usually only
> want to see products that have quantity != 0.

For me it does not apply to any normal scenario.
There are two cases, DB with small number of product so there is no
problem showing them. Or DB with large number of product so user will
have any way to filter for what he is looking for. And what user may be
looking for is that there is no quantity for a product.

> I see a couple of alternatives (maybe both are desirable):
> 
> - Open a couple of tabs. The default one shows all products and the other
> one applies the existing filter.

User who wants such slow filter can bookmark it. There is no point to
make tab domain.

> - Try to optimize the searcher which it seems to me it will eventually be
> necessary. We should probably give CTE a chance and see if we can make it
> faster.

Recursive CTE are not of any order faster. The only saving is the
communication. I already proved that previously for the MPTT. So I will
not try this dead end again myself.
msg60065 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2020-09-12.01:21:31
Missatge de Cédric Krier <bugs@tryton.org> del dia dv., 11 de set. 2020 a
les 18:18: <cedric.krier@b2ck.com>

> I think that it is not really needed to filter by quantities for this view
> and instead we can just filter product of type service.
>

I don't agree at 100% with this sentence. I think that users usually only
want to see products that have quantity != 0.

With the proposed change, the user will have to manually add that filter
which is difficult for most users.

However, I understand that if the user goes into that view to see only some
products (wants to filter once the view is opened) it is annoying to have
this filter because it slows things down.

I see a couple of alternatives (maybe both are desirable):

- Open a couple of tabs. The default one shows all products and the other
one applies the existing filter.
- Try to optimize the searcher which it seems to me it will eventually be
necessary. We should probably give CTE a chance and see if we can make it
faster.
review323981002 updated at https://codereview.tryton.org/323981002/#ps296231002
msg60061 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-09-11.18:18:12
When opening a location to the list of products, there is a domain to show only product with at least quantity or forecast quantity not null. This was added by issue939.
The problem is that the implementation of the searcher needs to compute the quantities for all the products (see issue6605). So on database with a lot of products, this can become a serious bottleneck because it is O(l * p) (l=number of location and p number of product).
I think that it is not really needed to filter by quantities for this view and instead we can just filter product of type service.

We have a similar filter on location when opening a product. But I think we can keep it because the number of locations is limited and in case of very large number flat locations will be used (and it is of O(l)).
History
Date User Action Args
2020-09-27 18:20:59roundup-botsetmessages: + msg60431
2020-09-27 18:20:55roundup-botsetstatus: testing -> resolved
nosy: + roundup-bot
messages: + msg60430
2020-09-15 10:15:52cedsettitle: Do not filter products by quantities -> Improve speed of products by quantities
2020-09-15 10:03:35cedsetreviews: 323981002 -> 323981002,323991002
messages: + msg60123
2020-09-15 00:04:56albertcasetmessages: + msg60122
2020-09-14 18:17:45reviewbotsetmessages: + msg60119
2020-09-14 18:06:16cedsetmessages: + msg60118
2020-09-14 17:49:26reviewbotsetmessages: + msg60117
2020-09-12 19:30:02pokolisetnosy: + pokoli
2020-09-12 10:32:18reviewbotsetmessages: + msg60078

Showing 10 items. Show all history (warning: this could be VERY long)