| 00:00.15 | *** join/#storm wallflower (n=wallflow@ip-205-246-113-216.pool.grokthis.net) |
| 00:11.17 | *** join/#storm michelp (n=michelp@70.103.91.130) |
| 01:11.57 | *** join/#storm statik (n=emurphy@189.66.188.72.cfl.res.rr.com) |
| 01:38.51 | *** join/#storm lymxz (n=lymxz@123.116.112.42) |
| 01:42.38 | *** part/#storm lymxz (n=lymxz@123.116.112.42) |
| 01:42.59 | *** join/#storm lymxz (n=lymxz@123.116.112.42) |
| 04:10.47 | *** join/#storm bigdog (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 05:22.40 | *** part/#storm lymxz (n=lymxz@123.116.112.42) |
| 06:18.41 | *** join/#storm bigdog (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 06:45.06 | *** join/#storm jukart (n=jukart@194.183.146.181) |
| 07:03.25 | *** join/#storm michelp (n=michelp@70.103.91.130) |
| 07:24.29 | *** join/#storm dobee (n=dobeee@194.183.146.186) |
| 07:34.56 | *** join/#storm bozzo (n=bozzo@BSN-210-226-227.dial-up.dsl.siol.net) |
| 08:25.19 | *** join/#storm michelp (n=michelp@70.103.91.130) |
| 08:49.25 | *** join/#storm mcella (n=michele@ip-171-30.sn3.eutelia.it) |
| 10:55.53 | *** join/#storm goschtl (n=goschtl@p5495F96A.dip.t-dialin.net) |
| 11:44.04 | *** join/#storm niemeyer (n=niemeyer@201-10-91-193.ctame705.dsl.brasiltelecom.net.br) |
| 13:42.46 | *** join/#storm jukart_ (n=jukart@194.183.146.181) |
| 13:43.31 | dobee | hi niemeyer |
| 13:43.47 | niemeyer | dobee: Hey! |
| 13:43.55 | dobee | i have an odd problem |
| 13:44.05 | dobee | in trunk storm/store.py |
| 13:44.15 | dobee | line 646 |
| 13:44.36 | dobee | why are the values not set there |
| 13:44.42 | dobee | <PROTECTED> |
| 13:44.50 | dobee | should come after this line |
| 13:45.12 | *** join/#storm bigdog (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 13:45.27 | dobee | because the properties are lazy which will result in an unneeded database select |
| 13:45.47 | dobee | if the object_info is in cache but the weakref is gone |
| 13:46.31 | dobee | niemeyer: all tests are passing if i do this |
| 13:46.32 | dobee | === modified file 'storm/store.py' |
| 13:46.32 | dobee | *** storm/store.py2007-11-15 23:42:59 +0000 |
| 13:46.33 | dobee | --- storm/store.py2007-12-05 13:41:39 +0000 |
| 13:46.33 | dobee | *************** |
| 13:46.34 | dobee | *** 644,649 **** |
| 13:46.36 | dobee | --- 644,650 ---- |
| 13:46.38 | dobee | <PROTECTED> |
| 13:46.40 | dobee | <PROTECTED> |
| 13:46.42 | dobee | <PROTECTED> |
| 13:46.44 | dobee | + self._set_values(obj_info, cls_info.columns, result, values) |
| 13:46.46 | dobee | <PROTECTED> |
| 13:46.48 | dobee | <PROTECTED> |
| 13:46.50 | dobee | <PROTECTED> |
| 13:47.02 | dobee | and the unneeded selects are gone |
| 13:47.17 | dobee | do i miss something here? |
| 13:48.36 | niemeyer | dobee: Let me look at the code, hold on |
| 13:48.42 | dobee | k, thx |
| 13:49.55 | niemeyer | dobee: What's the problem you're seeing? |
| 13:50.21 | dobee | sek |
| 13:52.15 | dobee | niemeyer: http://paste.lisp.org/display/51940 |
| 13:52.42 | dobee | in the first transaction it does only the resultset's select |
| 13:52.56 | dobee | in the second transaction it does a select for each found item too |
| 13:53.29 | dobee | with my changes in store.py it does the same in the second transaction |
| 13:53.34 | dobee | as in the first |
| 13:55.24 | dobee | btw: forget about the rating stuff |
| 13:59.09 | niemeyer | dobee: Sorry, I still don't understand what's the actual bug |
| 13:59.44 | dobee | i do a store.find(...some.condition) |
| 13:59.47 | niemeyer | Ok |
| 13:59.50 | dobee | in the first transaction |
| 13:59.57 | niemeyer | Right |
| 14:00.06 | dobee | then i iterate over the objects returned and display some attributes |
| 14:00.12 | dobee | of the objects |
| 14:00.20 | niemeyer | Ok |
| 14:00.22 | dobee | in the first transaction the cache is empty |
| 14:00.26 | niemeyer | Right |
| 14:00.41 | dobee | and there is no select for the values of the single objects |
| 14:00.49 | dobee | as you see in the log excerpt |
| 14:01.01 | dobee | now commit() |
| 14:01.05 | niemeyer | dobee: How do you mean? |
| 14:01.08 | dobee | store.invalidate() is called |
| 14:01.28 | niemeyer | dobee: The second line is selecting for values of a single object, isn't it? |
| 14:01.32 | dobee | in my paste line 2 |
| 14:01.41 | dobee | exactly |
| 14:01.50 | dobee | that is ok |
| 14:01.51 | niemeyer | Right |
| 14:01.55 | niemeyer | <dobee> and there is no select for the values of the single objects |
| 14:01.56 | dobee | but in the second transaction |
| 14:02.05 | niemeyer | So there is one select, ok.. |
| 14:02.15 | niemeyer | Ok |
| 14:02.29 | dobee | niemeyer: look at line 11 |
| 14:02.39 | dobee | this select is not needed |
| 14:02.49 | niemeyer | Why not? |
| 14:02.58 | dobee | also line 9 is not needed |
| 14:03.03 | niemeyer | Why not? |
| 14:03.13 | dobee | because of line 8 |
| 14:03.53 | dobee | the point is, that from the applications point of view the 2 transactions are exactly the same |
| 14:04.09 | dobee | but the first without the object_infos in cache results in less selects |
| 14:04.15 | niemeyer | Ok.. I don't understand.. probably because I don't know the schema |
| 14:04.29 | niemeyer | You have three different finds, and Storm is performing three different selects |
| 14:04.49 | dobee | i only do a single find |
| 14:04.57 | dobee | i do no get neither another find |
| 14:05.52 | niemeyer | Oh, ok.. let me think then |
| 14:06.21 | niemeyer | Yeah, I think I get it |
| 14:06.27 | dobee | the bug is, that if you iterate over a resultset and there are object_infos that are in cache then the values of the objects are refetched again because the values of the objects are not set |
| 14:06.38 | niemeyer | Right, ok |
| 14:06.42 | niemeyer | Gotcha |
| 14:06.46 | niemeyer | That's pretty bad indeed |
| 14:07.02 | niemeyer | OTOH, we can't _set_values() simply |
| 14:07.07 | niemeyer | I'll have to come up with a fix for that |
| 14:07.29 | niemeyer | dobee: The point is that we can't overwrite values that *are* set already |
| 14:07.33 | dobee | ok, then there must be something untested, because with my change tests pass |
| 14:07.39 | niemeyer | dobee: Otherwise a find() would overwrite updates, for instance |
| 14:08.07 | niemeyer | dobee: There is something untested, which is the situation you just described |
| 14:08.19 | niemeyer | dobee: The problem I'm describing will only be a bug if we introduce it :) |
| 14:08.32 | dobee | ok |
| 14:09.01 | niemeyer | dobee: In any case you're right, in both points.. what I'm saying should be tested as well |
| 14:09.53 | dobee | niemeyer: ok, we simply have to check if the reference is still there |
| 14:10.52 | niemeyer | dobee: Yeah, we need two tests: 1) Ensuring that when we have values we use them; 2) Ensuring that updates to a given attribute is preserved when faced with a find() |
| 14:13.25 | dobee | niemeyer: from a first glance, i think this problem is not find specific, also .get maybe affected |
| 14:14.57 | niemeyer | dobee: The fix will be global, but get() isn't affected specifically |
| 14:15.16 | niemeyer | dobee: get() returns the cached object without any queries |
| 14:16.50 | dobee | ah yeah it validates the cache |
| 14:18.17 | dobee | niemeyer: do you plan to fix this, or shall i |
| 14:19.36 | niemeyer | dobee: I'll definitely fix it if you don't. If you'd like to work on it that'd be great as well. |
| 14:20.11 | dobee | niemeyer: btw. find does a flush, so local changes should be in the db, why not write values |
| 14:21.20 | niemeyer | dobee: If that's the case we don't have to write them |
| 14:21.20 | dobee | get too |
| 14:21.54 | niemeyer | dobee: I mean, the flushed changes were sent from the existent values |
| 14:22.06 | niemeyer | get too? |
| 14:22.12 | niemeyer | Oh, it flushes, yeah |
| 14:23.11 | niemeyer | dobee: Think about non trivial cases, such as pickle variables |
| 14:23.23 | niemeyer | dobee: By setting the value we invalidate in-memory objects |
| 14:23.25 | dobee | huh |
| 14:24.32 | niemeyer | dobee: |
| 14:24.38 | niemeyer | value = obj.pickle |
| 14:24.45 | niemeyer | store.find(SomethingUnrelated) |
| 14:24.50 | niemeyer | value["a"] = 1 |
| 14:24.53 | niemeyer | store.commit() |
| 14:24.59 | niemeyer | Oops.. change's lost |
| 14:26.13 | dobee | if we do the same check as in get() |
| 14:26.21 | dobee | <PROTECTED> |
| 14:26.22 | dobee | <PROTECTED> |
| 14:26.22 | dobee | <PROTECTED> |
| 14:26.22 | dobee | <PROTECTED> |
| 14:26.41 | dobee | this should not be a problem, or do i miss somethin? |
| 14:26.48 | dobee | niemeyer: |
| 14:27.17 | niemeyer | dobee: I don't see the relation between these issues |
| 14:27.35 | niemeyer | dobee: _validate_cache() will verify if the object is still present in the database only |
| 14:28.06 | dobee | ups, ok ... sec |
| 14:36.25 | dobee | niemeyer: what about checking if the variable is set in the object_info and set it if it is not set |
| 14:36.43 | dobee | i have to dig deeper for doing this ... hm |
| 14:46.46 | niemeyer | dobee: Right, that's the idea |
| 14:47.21 | niemeyer | dobee: variables offer is_defined() |
| 14:48.48 | niemeyer | dobee: _set_values can probably get a undefined_only=False attribute |
| 14:49.02 | niemeyer | dobee: Testing this might be tricky |
| 14:52.14 | dobee | niemeyer: yes i think too that the testing is the issue :-( |
| 14:52.32 | dobee | i look if i find the time tomorrow |
| 15:00.43 | niemeyer | dobee: Cool, thanks for debugging this issue |
| 15:11.53 | *** join/#storm bigdog (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 15:15.44 | radix | looks like storm is about to get another interesting optimization :) |
| 15:41.29 | *** join/#storm dotz (n=dotz@chello089078004026.chello.pl) |
| 16:03.57 | *** part/#storm goschtl (n=goschtl@p5495F96A.dip.t-dialin.net) |
| 18:26.30 | *** join/#storm dobee (n=dobeee@81-223-53-162.dornbirn.xdsl-line.inode.at) |
| 18:44.55 | *** join/#storm Zenom (n=Zenom@unaffiliated/aj1973) |
| 19:29.54 | *** join/#storm dobee (n=dobeee@81-223-53-162.dornbirn.xdsl-line.inode.at) |
| 20:17.59 | *** join/#storm thumper (n=tim@125-236-193-95.adsl.xtra.co.nz) |
| 20:26.02 | *** join/#storm jukart (n=jukart@85-124-221-45.static.xdsl-line.inode.at) |
| 21:03.12 | *** join/#storm trisiak (n=tree@chello084010084214.chello.pl) |
| 21:23.10 | *** join/#storm bigdog (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |
| 22:54.22 | *** join/#storm michelp (n=michelp@69-30-72-119.dq1sf.easystreet.com) |
| 23:38.51 | *** join/#storm bigdog (n=scmikes@72-197-8-8-arpa.cust.cinci.current.net) |