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

chat.freenode.net #tryton log beginning Fri Oct 25 00:00:03 CEST 2013
2013-10-25 12:16 <giedrius> recently 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 slow
2013-10-25 12:19 <giedrius> to be more specific, the problematic part is in ModelStorage._validation, where it validates domains of relational fields
2013-10-25 12:19 <cedk> giedrius: this validation exists since 1.0
2013-10-25 12:19 <giedrius> i have an idea how to improve it
2013-10-25 12:20 <giedrius> basically 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 db
2013-10-25 12:21 <cedk> giedrius: except the target could also have changed
2013-10-25 12:23 <giedrius> but it doesn't matter, because these fields are not touched
2013-10-25 12:23 <cedk> giedrius: it matters
2013-10-25 12:23 <cedk> giedrius: it is about data integrity
2013-10-25 12:23 <giedrius> i'll tell you my case
2013-10-25 12:26 <giedrius> i 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.
2013-10-25 12:27 <giedrius> cedk: the target must be responsible for integrity
2013-10-25 12:29 <cedk> giedrius: no because the link depends on the target value
2013-10-25 12:30 <giedrius> cedk: 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)
2013-10-25 12:30 <cedk> giedrius: your problem will be solved with the write multi
2013-10-25 12:31 <cedk> giedrius: yes it is not prefect the other validation way should be managed by the target
2013-10-25 12:31 <giedrius> cedk: and changing things like single integer field, it should not matter on this
2013-10-25 12:31 <cedk> giedrius: it could matter
2013-10-25 12:32 <giedrius> cedk: yes it could, only i case then the changed field is used in relation field domain
2013-10-25 12:32 <giedrius> only in case*
2013-10-25 12:33 <giedrius> so my suggestion is to check if changed field has any impact on record's relation fields and if so, only then query the db
2013-10-25 12:34 <cedk> giedrius: for me, it is the wrong answer to your performance issue
2013-10-25 12:36 <giedrius> cedk: it would be answer not only for this issue, but also would increase performance of all other modules
2013-10-25 12:37 <giedrius> cedk: 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 objects
2013-10-25 12:39 <giedrius> cedk: 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.
2013-10-25 12:39 <cedk> giedrius: yes but at least the bug is discovered otherwise it could never been
2013-10-25 12:40 <cedk> giedrius: and you will get the same behavior with your optimisation
2013-10-25 12:41 <giedrius> cedk: no, because if the date field is not used in domain or state of invoice lines, then invoice lines are not validated at all.
2013-10-25 12:42 <cedk> giedrius: yes but if it is, you still have an error message that user doesn't understand
2013-10-25 12:42 <giedrius> cedk: hmm, i don't understand what you mean. What kind of error?
2013-10-25 12:43 <cedk> giedrius: change date -> raise line is not valid
2013-10-25 12:44 <cedk> giedrius: for the usee pov, it is the same strange error
2013-10-25 12:45 <cedk> giedrius: by the way, have you metrics?
2013-10-25 12:45 <giedrius> cedk: do you mean change date -> raise line is not valid, then the date is in the domain of invoice line?
2013-10-25 12:45 <cedk> giedrius: yes
2013-10-25 12:46 <cedk> giedrius: indeed, I will could agree on such optimisation if it is the only possible one
2013-10-25 12:46 <cedk> giedrius: so I think some metrics are needed here
2013-10-25 12:46 <cedk> giedrius: like with target search takes time with which domain etc.
2013-10-25 12:47 <giedrius> cedk: yeah, it such case no workaround for this - user must be warned
2013-10-25 12:47 <giedrius> in*
2013-10-25 12:47 <cedk> giedrius: because I just see 5 Many2One on stock.move so you are just removing 5 queries out of I guess ~200
2013-10-25 12:49 <giedrius> cedk: saving StockMove does not make up to 200 queries, its only about 5-6
2013-10-25 12:49 <cedk> giedrius: needs metrics
2013-10-25 12:50 <giedrius> cedk: i'm pretty sure that in some cases improvements will be >100x (ex. in mine)
2013-10-25 12:50 <cedk> giedrius: I think you are under-estimate
2013-10-25 12:50 <giedrius> cedk: ok, i'll try to implement this
2013-10-25 12:51 <giedrius> cedk: could be, but why not try
2013-10-25 12:52 <cedk> giedrius: that what I say we need metrics
2013-10-25 12:54 <giedrius> cedk: sure, not problem for this. I just was wondering if this would not make any problem with functionality
2013-10-25 12:55 <cedk> giedrius: hard to say but we have tests for that
2013-10-25 13:09 <cedk> giedrius: by the way, I think you change should do this:
2013-10-25 13:09 <cedk> - change: _validate(cls, records, fields=None)
2013-10-25 13:10 <cedk> - on the loop over fields, add test field_name in fields or any field.depends in fields
2013-10-25 13:18 <giedrius> cedk: i was already started thinking how to parse the domain to get these fields. Cool, field.depends makes this easy :)
2013-10-25 13:20 <cedk> giedrius: of course this will depend on the good definition but there is a default unittest to check
2013-10-25 15:36 -!- ykarmouta(~ykarmouta@LNeuilly-152-23-15-185.w193-252.abo.wanadoo.fr) has left #tryton
2013-10-25 19:00 -!- vcardon(~vcardon@LNeuilly-152-23-15-185.w193-252.abo.wanadoo.fr) has left #tryton

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!