IRC logs of #tryton for Friday, 2013-10-25

chat.freenode.net #tryton log beginning Fri Oct 25 00:00:03 CEST 2013
giedriusrecently i've noticed a big performance problem on tryton. When I try to save a record with a single field modified (ex. field type == integer), Tryton's ORM makes validation of all fields available on the record. So then the record has many relational fields, it becomes very slow12:16
giedriusto be more specific, the problematic part is in ModelStorage._validation, where it validates domains of relational fields12:19
cedkgiedrius: this validation exists since 1.012:19
giedriusi have an idea how to improve it12:19
giedriusbasically we need to check if modified field has any impact in validation of relational field. If no, then no need to make any query to db12:20
cedkgiedrius: except the target could also have changed12:21
giedriusbut it doesn't matter, because these fields are not touched12:23
cedkgiedrius: it matters12:23
cedkgiedrius: it is about data integrity12:23
giedriusi'll tell you my case12:23
giedriusi need to update unit_price for >200 StockMoves. Now if i save every record it takes 40 secs (single record ~0,2 sec.). This is not acceptable for the user.12:26
giedriuscedk: the target must be responsible for integrity12:27
cedkgiedrius: no because the link depends on the target value12:29
giedriuscedk: i mean, if changing target makes impact on other objects, it must make some kind of validation, because if it doesn't, then these object will be invalid till someone change them (until _validate is triggered)12:30
cedkgiedrius: your problem will be solved with the write multi12:30
cedkgiedrius: yes it is not prefect the other validation way should be managed by the target12:31
giedriuscedk: and changing things like single integer field, it should not matter on this12:31
cedkgiedrius: it could matter12:31
giedriuscedk: yes it could, only i case then the changed field is used in relation field domain12:32
giedriusonly in case*12:32
giedriusso my suggestion is to check if changed field has any impact on record's relation fields and if so, only then query the db12:33
cedkgiedrius: for me, it is the wrong answer to your performance issue12:34
giedriuscedk: it would be answer not only for this issue, but also would increase performance of all other modules12:36
giedriuscedk: also i think this kind of validation is wrong, because user may not understand why he cannot save the record. He cannot know that things changed in other objects12:37
giedriuscedk: ex. user change date of invoice, but he cannot save the record and gets the error: invalid invoice lines. He cannot understand what happens, because he only changed the date.12:39
cedkgiedrius: yes but at least the bug is discovered otherwise it could never been12:39
cedkgiedrius: and you will get the same behavior with your optimisation12:40
giedriuscedk: no, because if the date field is not used in domain or state of invoice lines, then invoice lines are not validated at all.12:41
cedkgiedrius: yes but if it is, you still have an error message that user doesn't understand12:42
giedriuscedk: hmm, i don't understand what you mean. What kind of error?12:42
cedkgiedrius: change date -> raise line is not valid12:43
cedkgiedrius: for the usee pov, it is the same strange error12:44
cedkgiedrius: by the way, have you metrics?12:45
giedriuscedk: do you mean change date -> raise line is not valid, then the date is in the domain of invoice line?12:45
cedkgiedrius: yes12:45
cedkgiedrius: indeed, I will could agree on such optimisation if it is the only possible one12:46
cedkgiedrius: so I think some metrics are needed here12:46
cedkgiedrius: like with target search takes time with which domain etc.12:46
giedriuscedk: yeah, it such case no workaround for this - user must be warned12:47
giedriusin*12:47
cedkgiedrius: because I just see 5 Many2One on stock.move so you are just removing 5 queries out of I guess ~20012:47
giedriuscedk: saving StockMove does not make up to 200 queries, its only about 5-612:49
cedkgiedrius: needs metrics12:49
giedriuscedk: i'm pretty sure that in some cases improvements will be >100x (ex. in mine)12:50
cedkgiedrius: I think you are under-estimate12:50
giedriuscedk: ok, i'll try to implement this12:50
giedriuscedk: could be, but why not try12:51
cedkgiedrius: that what I say we need metrics12:52
giedriuscedk: sure, not problem for this. I just was wondering if this would not make any problem with functionality12:54
cedkgiedrius: hard to say but we have tests for that12:55
cedkgiedrius: by the way, I think you change should do this:13:09
cedk- change: _validate(cls, records, fields=None)13:09
cedk- on the loop over fields, add test field_name in fields or any field.depends in fields13:10
giedriuscedk: i was already started thinking how to parse the domain to get these fields. Cool, field.depends makes this easy :)13:18
cedkgiedrius: of course this will depend on the good definition but there is a default unittest to check13:20
-!- ykarmouta(~ykarmouta@LNeuilly-152-23-15-185.w193-252.abo.wanadoo.fr) has left #tryton15:36
-!- vcardon(~vcardon@LNeuilly-152-23-15-185.w193-252.abo.wanadoo.fr) has left #tryton19:00

Generated by irclog2html.py 2.11.0 by Marius Gedminas - find it at mg.pov.lt!