so it would first fetch the ids, then the actual rg objects which are most likely in cache already
2011-12-16 35003, 2011
ocharles
this seems good in theory, but I think the way we use our DAO stuff is going to be a problem. mostly due to the ability to construct partial objects
2011-12-16 35033, 2011
luks
not sure I understand
2011-12-16 35046, 2011
ocharles
well, it's possible to create a ReleaseGroup entity without a name, for example
2011-12-16 35054, 2011
luks
why is that a problem
2011-12-16 35055, 2011
luks
?
2011-12-16 35003, 2011
ocharles
I doubt we do that, but if we end up putting something like that in the cache, something might pull it and expect a name to be there
2011-12-16 35030, 2011
luks
oh, why would you add it to the cache?
2011-12-16 35040, 2011
ocharles
well, it has to get into the cache somewhere, right?
2011-12-16 35041, 2011
luks
I thought you use the cache that is used by get_by_id
2011-12-16 35048, 2011
luks
which contains the full objects
2011-12-16 35021, 2011
ocharles
oh... so find_by_artist would *only* find release group ids, and then it would do a get_by_ids?
2011-12-16 35028, 2011
luks
yes
2011-12-16 35033, 2011
ocharles
ahhh, that's an interesting look at it
2011-12-16 35041, 2011
luks
well, for the caching query
2011-12-16 35058, 2011
luks
in case you don't have the release group ids cached, you would fetch the whole thing from the db
2011-12-16 35010, 2011
luks
and no bother with get_by_ids
2011-12-16 35018, 2011
luks
*not
2011-12-16 35024, 2011
ocharles
mm
2011-12-16 35042, 2011
ocharles
I think it's the difference between hot and cold cache potentially returning different objects that makes me a bit uneasy
2011-12-16 35021, 2011
luks
would it return different objects? I though get_by_id and find_by_artist returns the same amount of data
2011-12-16 35044, 2011
ocharles
they *should*, but nothing guarantees that
2011-12-16 35050, 2011
ocharles
so it's possible to introduce a bug there
2011-12-16 35003, 2011
ocharles
and bugs where the warm cache and hot cache act differently are a pita to debug
2011-12-16 35014, 2011
luks
that's always possible
2011-12-16 35022, 2011
ocharles
not if you change the entitys to always be complete
2011-12-16 35056, 2011
luks
I can't think of a situation where they are not complete
2011-12-16 35001, 2011
luks
except when creating a new one
2011-12-16 35021, 2011
ocharles
there are quite a few places where we don't load certain ids or something
2011-12-16 35032, 2011
ocharles
i can't think any off the top of my head
2011-12-16 35040, 2011
luks
do you want to cache *everything*?
2011-12-16 35047, 2011
ocharles
just more, for now
2011-12-16 35048, 2011
luks
that doesn't make much sense to me
2011-12-16 35001, 2011
luks
I'd only select the most important methods
2011-12-16 35004, 2011
ocharles
in BookBrainz, entities must be complete - you can't construct a partial entity. Because of this, they don't actually link directly to other entities. If you want the loaded country of an artist, you return a (Artist, Country) tuple. But due to Perl's general suckiness, I don't think it'll work very well :)
2011-12-16 35023, 2011
luks
designing a generic caching system would be very hard
2011-12-16 35024, 2011
ocharles
luks: right, that's why I wrote all this profiling stuff, to see what is actually important (and it seems to be relationship code)
2011-12-16 35027, 2011
luks
but practically you don't need that
2011-12-16 35054, 2011
luks
the front artist page is what gets most hits probably
2011-12-16 35004, 2011
ocharles
big releases are the most problematic atm
2011-12-16 35004, 2011
luks
so ReleaseGroup->find_by_artist is a good target
2011-12-16 35007, 2011
ocharles
as in they timeout
2011-12-16 35021, 2011
ocharles
yea, find_by_artist is also identified as a slow point by my debug panel, so that's why I mentioned it :)
2011-12-16 35017, 2011
luks
do releases timeout because of ARs?
2011-12-16 35029, 2011
luks
or just tracklists
2011-12-16 35051, 2011
ocharles
there's no single problem sadly. A big whack of time spent rendering and a big whack of time constructing objects
2011-12-16 35008, 2011
luks
ah, yeah
2011-12-16 35026, 2011
ocharles
hopefully the caching skips the complicated moose object construction and gets us a speed up there
2011-12-16 35038, 2011
luks
hm, does it?
2011-12-16 35007, 2011
ocharles
it won't hit ->new, so 'probably' is my hunch
2011-12-16 35020, 2011
ocharles
from what I understand, storable will just inflate it and rebless it
2011-12-16 35030, 2011
luks
doesn't Storable call ->new the usual way?
2011-12-16 35038, 2011
luks
oh
2011-12-16 35039, 2011
ocharles
I don't think so, because that could have side effects
2011-12-16 35018, 2011
luks
but I guess it still hits the Moose attribute validation at some point
so back to the find_by_artist stuff... how would it handle cache invalidation? ie, a new release group is created - i guess insert is going to be responsible for clearing the find-by-artist cache?
2011-12-16 35005, 2011
olaf_ has left the channel
2011-12-16 35054, 2011
luks
there are two cases, single artist and va, I don't remember the code if they are in the same function
2011-12-16 35009, 2011
ocharles
that's find_by_artist and find_by_track_artist, iirc
2011-12-16 35014, 2011
luks
single artist RGs should be simple, insert/delete automatically invalidates cache
2011-12-16 35022, 2011
luks
update only if the artist_credit is different
2011-12-16 35047, 2011
ocharles
so basically, other daos are going to have to be aware of the other caches
2011-12-16 35050, 2011
luks
or you might want to always invalidate on update as well
2011-12-16 35052, 2011
luks
to make it simpler
2011-12-16 35023, 2011
ocharles
oh actually, I guess that's all within the release group dao, so that's not so bad
2011-12-16 35049, 2011
luks
yes, but the cache would belong to the artist
2011-12-16 35025, 2011
warp
hello
2011-12-16 35030, 2011
ocharles
hey warp
2011-12-16 35012, 2011
wpl joined the channel
2011-12-16 35009, 2011
ijabz
ocharles, getting lot of 503s on the mb server , has something been added to reduce traffic, or it just very busy ..
2011-12-16 35038, 2011
ocharles
nothing has changed from me there
2011-12-16 35000, 2011
ijabz
so its just very busy then, going to get worse over Xmas then I expect
ocharles: did you add the On Beta workflow option yet?
2011-12-16 35037, 2011
ocharles
yep
2011-12-16 35044, 2011
warp doesn't see it.
2011-12-16 35005, 2011
ocharles
what status is the ticket in atm?
2011-12-16 35011, 2011
warp
I'm on a ticket which is "in progress"
2011-12-16 35013, 2011
ocharles
ok, it probably can't go from in progress to in beta :)
2011-12-16 35020, 2011
ocharles
can it go to in review?
2011-12-16 35027, 2011
warp
yes
2011-12-16 35038, 2011
ocharles
do that then
2011-12-16 35041, 2011
ocharles
that's how it's meant to go
2011-12-16 35011, 2011
warp
not for this particular change :)
2011-12-16 35032, 2011
ocharles
I thought you wanted everything to go on beta to go through code review :)
2011-12-16 35023, 2011
warp
some changes are too small for code review, normally I would have made this change in master.
2011-12-16 35047, 2011
warp
also, on jira I just want all transitions to be allowed :)
2011-12-16 35006, 2011
ocharles
mmm
2011-12-16 35037, 2011
warp
I just realized that branching from master for bugfixing is not a good idea anymore
2011-12-16 35053, 2011
ocharles
it should be
2011-12-16 35003, 2011
warp
as I cannot merge such branches into beta without bringing in unwanted stuff
2011-12-16 35014, 2011
ocharles
but everything in master should be in beta
2011-12-16 35027, 2011
warp
oh, you're right.
2011-12-16 35035, 2011
warp
then stuff has been comitted to master which hasn't been comitted to beta.
2011-12-16 35050, 2011
ocharles
well it should always be in master and beta
2011-12-16 35002, 2011
ocharles
that's why the flow is to trickle the merge into next, then beta, then master
2011-12-16 35054, 2011
luks
just wondering, do you need the beta branch at all?
2011-12-16 35010, 2011
ocharles
luks: sort of
2011-12-16 35024, 2011
luks
or rather, you do need it to be different from a snapshot of master at some point?
2011-12-16 35025, 2011
ocharles
if something needs user testing, it shouldn't be in master, because if we want to do a server release while doing user testing, we'll end up releasing that too
2011-12-16 35033, 2011
luks
why can't it go to master?
2011-12-16 35041, 2011
ocharles
because then when we merge master into production, we've shipped it
2011-12-16 35002, 2011
luks
I'd say pushing something to beta counts as shipping it
2011-12-16 35027, 2011
ocharles
pushing to beta means we're allowing users who want to live with buggy stuff to use it
2011-12-16 35032, 2011
ocharles
but we shouldn't enforce that on all site users
2011-12-16 35004, 2011
luks
I kind of disagree with that
2011-12-16 35017, 2011
warp
luks: if user testing on beta uncovers flaws in a particular branch/patch, it shouldn't be released.
2011-12-16 35034, 2011
luks
in my opinion, beta should be "this will be shipped in the next release"
2011-12-16 35056, 2011
luks
UI testing shouldn't be done on live data
2011-12-16 35058, 2011
warp
luks: beta is "we intend to ship this in the next release". master is "this will actually be shipped in the next release".
2011-12-16 35056, 2011
luks
when will something go from beta to master?
2011-12-16 35014, 2011
ocharles
when it passes user testing. that's still the minor hole to work out when that decision has been reached
2011-12-16 35023, 2011
ocharles
luks: people don't test unless they can do it on live data
2011-12-16 35023, 2011
luks
when does something pass user testing?
2011-12-16 35033, 2011
warp
luks: on the day of release, assuming no problems with that particular branch/bug fix have been reported.
2011-12-16 35040, 2011
luks
whoa
2011-12-16 35045, 2011
luks
now I very strongly disagree with that
2011-12-16 35059, 2011
ocharles
i don't think that's been agreed, warp
2011-12-16 35018, 2011
ocharles
I suggested a mailing list that we could post to, but jira should be enough
2011-12-16 35036, 2011
luks
in my opinion, beta would be the most useful as the next musicbrainz.org
2011-12-16 35042, 2011
warp
ocharles: you want users to specifically comment "ship it!" on jira?
2011-12-16 35001, 2011
luks
if you keep something on beta but never ship it, it's not going to be very useful
2011-12-16 35013, 2011
ocharles
warp: with my ML idea I would say "MBS-foo is now in beta.musicbrainz.org. Assuming no negative feedback this will ship on now() + x days"
2011-12-16 35013, 2011
luks
it should be the final product, not a collection of branches
2011-12-16 35022, 2011
ocharles
because essentially we currently have that, but x is 0
2011-12-16 35034, 2011
ocharles
luks: beta resets after every release, just like next
2011-12-16 35047, 2011
ocharles
and then we re-merge stuff that's still in user testing back into it
2011-12-16 35039, 2011
warp
luks: not everything can be tested on live data, so it will not neccesarily contain all branches which will be part of the next release.
2011-12-16 35007, 2011
warp
luks: but the intent is for all branches which are on beta to be part of the next release.
2011-12-16 35025, 2011
warp
luks: but the whole point of beta is to allow particular things to be rejected as not ready for release.
2011-12-16 35017, 2011
hawke__ joined the channel
2011-12-16 35042, 2011
luks
I guess I misunderstood the goal of beta
2011-12-16 35046, 2011
warp
luks: what did you think the goal of beta was?
2011-12-16 35005, 2011
luks
making sure you are not going to ship something buggy in the next release
