so it would first fetch the ids, then the actual rg objects which are most likely in cache already
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
luks
not sure I understand
ocharles
well, it's possible to create a ReleaseGroup entity without a name, for example
luks
why is that a problem
?
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
luks
oh, why would you add it to the cache?
ocharles
well, it has to get into the cache somewhere, right?
luks
I thought you use the cache that is used by get_by_id
which contains the full objects
ocharles
oh... so find_by_artist would *only* find release group ids, and then it would do a get_by_ids?
luks
yes
ocharles
ahhh, that's an interesting look at it
luks
well, for the caching query
in case you don't have the release group ids cached, you would fetch the whole thing from the db
and no bother with get_by_ids
*not
ocharles
mm
I think it's the difference between hot and cold cache potentially returning different objects that makes me a bit uneasy
luks
would it return different objects? I though get_by_id and find_by_artist returns the same amount of data
ocharles
they *should*, but nothing guarantees that
so it's possible to introduce a bug there
and bugs where the warm cache and hot cache act differently are a pita to debug
luks
that's always possible
ocharles
not if you change the entitys to always be complete
luks
I can't think of a situation where they are not complete
except when creating a new one
ocharles
there are quite a few places where we don't load certain ids or something
i can't think any off the top of my head
luks
do you want to cache *everything*?
ocharles
just more, for now
luks
that doesn't make much sense to me
I'd only select the most important methods
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 :)
luks
designing a generic caching system would be very hard
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)
luks
but practically you don't need that
the front artist page is what gets most hits probably
ocharles
big releases are the most problematic atm
luks
so ReleaseGroup->find_by_artist is a good target
ocharles
as in they timeout
yea, find_by_artist is also identified as a slow point by my debug panel, so that's why I mentioned it :)
luks
do releases timeout because of ARs?
or just tracklists
ocharles
there's no single problem sadly. A big whack of time spent rendering and a big whack of time constructing objects
luks
ah, yeah
ocharles
hopefully the caching skips the complicated moose object construction and gets us a speed up there
luks
hm, does it?
ocharles
it won't hit ->new, so 'probably' is my hunch
from what I understand, storable will just inflate it and rebless it
luks
doesn't Storable call ->new the usual way?
oh
ocharles
I don't think so, because that could have side effects
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?
olaf_ has left the channel
luks
there are two cases, single artist and va, I don't remember the code if they are in the same function
ocharles
that's find_by_artist and find_by_track_artist, iirc
luks
single artist RGs should be simple, insert/delete automatically invalidates cache
update only if the artist_credit is different
ocharles
so basically, other daos are going to have to be aware of the other caches
luks
or you might want to always invalidate on update as well
to make it simpler
ocharles
oh actually, I guess that's all within the release group dao, so that's not so bad
luks
yes, but the cache would belong to the artist
warp
hello
ocharles
hey warp
wpl joined the channel
ijabz
ocharles, getting lot of 503s on the mb server , has something been added to reduce traffic, or it just very busy ..
ocharles
nothing has changed from me there
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?
ocharles
yep
warp doesn't see it.
what status is the ticket in atm?
warp
I'm on a ticket which is "in progress"
ocharles
ok, it probably can't go from in progress to in beta :)
can it go to in review?
warp
yes
ocharles
do that then
that's how it's meant to go
warp
not for this particular change :)
ocharles
I thought you wanted everything to go on beta to go through code review :)
warp
some changes are too small for code review, normally I would have made this change in master.
also, on jira I just want all transitions to be allowed :)
ocharles
mmm
warp
I just realized that branching from master for bugfixing is not a good idea anymore
ocharles
it should be
warp
as I cannot merge such branches into beta without bringing in unwanted stuff
ocharles
but everything in master should be in beta
warp
oh, you're right.
then stuff has been comitted to master which hasn't been comitted to beta.
ocharles
well it should always be in master and beta
that's why the flow is to trickle the merge into next, then beta, then master
luks
just wondering, do you need the beta branch at all?
ocharles
luks: sort of
luks
or rather, you do need it to be different from a snapshot of master at some point?
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
luks
why can't it go to master?
ocharles
because then when we merge master into production, we've shipped it
luks
I'd say pushing something to beta counts as shipping it
ocharles
pushing to beta means we're allowing users who want to live with buggy stuff to use it
but we shouldn't enforce that on all site users
luks
I kind of disagree with that
warp
luks: if user testing on beta uncovers flaws in a particular branch/patch, it shouldn't be released.
luks
in my opinion, beta should be "this will be shipped in the next release"
UI testing shouldn't be done on live data
warp
luks: beta is "we intend to ship this in the next release". master is "this will actually be shipped in the next release".
luks
when will something go from beta to master?
ocharles
when it passes user testing. that's still the minor hole to work out when that decision has been reached
luks: people don't test unless they can do it on live data
luks
when does something pass user testing?
warp
luks: on the day of release, assuming no problems with that particular branch/bug fix have been reported.
luks
whoa
now I very strongly disagree with that
ocharles
i don't think that's been agreed, warp
I suggested a mailing list that we could post to, but jira should be enough
luks
in my opinion, beta would be the most useful as the next musicbrainz.org
warp
ocharles: you want users to specifically comment "ship it!" on jira?
luks
if you keep something on beta but never ship it, it's not going to be very useful
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"
luks
it should be the final product, not a collection of branches
ocharles
because essentially we currently have that, but x is 0
luks: beta resets after every release, just like next
and then we re-merge stuff that's still in user testing back into it
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.
luks: but the intent is for all branches which are on beta to be part of the next release.
luks: but the whole point of beta is to allow particular things to be rejected as not ready for release.
hawke__ joined the channel
luks
I guess I misunderstood the goal of beta
warp
luks: what did you think the goal of beta was?
luks
making sure you are not going to ship something buggy in the next release