HomePhabricator

RFC Meeting: PSR-6 Cache interface in Mediawiki core (2016-05-04, #wikimedia-office)
ActivePublic

Hosted by daniel on May 4 2016, 9:00 PM - 10:00 PM.

Description

See the Architecture meetings page for more general information about this meeting (also: Phab query: list of upcoming RFC meetings, Phab query: list of all RFC meetings).

Recurring Event

Event Series
This event is an instance of E66: ArchCom RFC Meeting Wxx: <topic TBD> (<see "Starts" field>, #wikimedia-office), and repeats every week.

Event Timeline

RobLa-WMF renamed this event from RFC Meeting: <topic TBD> (<see "Starts" field>, #wikimedia-office) to RFC Meeting: PSR-6 Cache interface in Mediawiki core (2016-05-04, #wikimedia-office).Apr 22 2016, 12:26 AM
RobLa-WMF updated the event description. (Show Details)
Addshore invited: ; uninvited: .Apr 22 2016, 7:41 AM
Addshore added a subscriber: Addshore.
brion added a subscriber: brion.Apr 27 2016, 10:01 PM

#wikimedia-office: E169 RFC Meeting: PSR-6 Cache interface in Mediawiki core (T130528)

T130528 was declined. We discussed general cache issues for the bulk of the meeting.

Meeting started by robla at 21:02:48 UTC (full logs).

  • <addshore> So. The outcome I would personally like from the RFC is to have a cache Interface to be used within MediaWiki core. I don't personally have strong opinions of what it should be / look like but I feel we should of course consider PSR6 (robla, 21:04:54)
  • <bd808> I explained the things I explictly dislike about PSR6 at https://phabricator.wikimedia.org/T130528#2245415 (robla, 21:08:17)
  • <DanielK_WMDE> addshore: ok, so rfc rejected ;) shall we repurpose the meeting to discuss alternatives / next steps? what are the needs we have? <addshore> DanielK_WMDE: sounds sensible! (robla, 21:16:33)
  • question discussed: a) need a better caching abstraction than BagOStuff? or b) is the status quo fine or c) should BagOStuff be deprecated, and not replaced with anything? (robla, 21:23:32)
  • question discussed: what problem was this RFC trying to solve? (robla, 21:24:26)
  • <TimStarling> addshore, the usual way to do mass purging is to have a single key which invalidates the collection, which you check on fetch (DanielK_WMDE, 21:36:08)
  • addshore agrees with bd808 that a HashBagOStuff cache is just an in process cache for the duration of a request (robla, 21:53:16)
  • watchlist caching might be worth an rfc on its own. (DanielK_WMDE, 21:53:22)
  • use cases for "tagged" caching: watchlist entries, wikibase usage tracking, parser cache, web cache (varnish buckets), ... (DanielK_WMDE, 21:55:30)
  • discussion planned to continue on #wikimedia-tech (robla, 21:57:22)
  • https://phabricator.wikimedia.org/E171 next week's RFC meeting (robla, 21:59:28)

Meeting ended at 22:00:15 UTC

People present (lines said)

TimStarling (41)
DanielK_WMDE (40)
addshore (30)
bd808 (26)
robla (21)
SMalyshev (17)
wm-labs-meetbot (3)
psychoslave (2)
stashbot (1)
Scott_WUaS (1)

121:02:48 <robla> #startmeeting E169 RFC Meeting: PSR-6 Cache interface in Mediawiki core
221:02:48 <wm-labs-meetbot> Meeting started Wed May 4 21:02:48 2016 UTC and is due to finish in 60 minutes. The chair is robla. Information about MeetBot at http://wiki.debian.org/MeetBot.
321:02:48 <wm-labs-meetbot> Useful Commands: #action #agreed #help #info #idea #link #topic #startvote.
421:02:48 <wm-labs-meetbot> The meeting name has been set to 'e169_rfc_meeting__psr_6_cache_interface_in_mediawiki_core'
521:03:05 <addshore> *waves*
621:03:11 <SMalyshev> oh nice. I was wondering why nobody's here :)
721:04:08 <addshore> So. The outcome I would personally like from the RFC is to have a cache Interface to be used within MediaWiki core. I don't personally have strong opinions of what it should be / look like but I feel we should of course consider PSR6
821:04:54 <robla> #info <addshore> So. The outcome I would personally like from the RFC is to have a cache Interface to be used within MediaWiki core. I don't personally have strong opinions of what it should be / look like but I feel we should of course consider PSR6
921:05:11 <robla> #link https://phabricator.wikimedia.org/E169
1021:05:12 <DanielK_WMDE> addshore: you mean an interface different from BagOStuff?
1121:05:23 <robla> #link https://phabricator.wikimedia.org/T130528
1221:05:32 <DanielK_WMDE> addshore: there's also the more recent WANObjectCache
1321:05:44 <addshore> So, the issue that I ran into that started me off looking at this is that BagOStuff is not an Interface
1421:05:48 <Scott_WUaS> Hi All
1521:06:02 <bd808> addshore: does that matter?
1621:06:14 <bd808> it is our common (and crappy) cache abstraction
1721:06:33 <DanielK_WMDE> addshore: that's easy to fix. Just mkae it an interface, and rename the current implementation to bag=Stuff base or AbstractBagOStuff
1821:06:36 <addshore> bd808, yes. I ran into issues while just trying to extend bagOfStuff to make a Taggable / Indexed BagOfStuff
1921:06:48 <bd808> crappy only because abstracting caching pretty much always falis in my experience
2021:07:19 <addshore> DanielK_WMDE: indeed, but we currently have 3 different caches with different appearances in core, so tieing them together (at least a bit) would probably make sense?
2121:07:31 <DanielK_WMDE> addshore: one important querstion is - should we try to cover all zuse cases with a single interface (or hierarchy of interfaces)? Or should we have different interfaces for different use cases?
2221:07:35 <SMalyshev> reading through the ticket and critique of PSR6, it looks to me like it's way overdesigned, and I don't see clear advantage to what we have now
2321:07:44 <bd808> I explained the things I explictly dislike about PSR6 at https://phabricator.wikimedia.org/T130528#2245415
2421:07:54 <SMalyshev> especially in two-step cache interface whose sole purpose seems to be working around null problem
2521:08:17 <robla> #info <bd808> I explained the things I explictly dislike about PSR6 at https://phabricator.wikimedia.org/T130528#2245415
2621:08:39 <DanielK_WMDE> addshore: i'm not sure... if things look the same, people expect them to behave the same. the current BagOStuff system makes basically no guarantees, and there is no way for code to check that the object thea have meets the local requirements
2721:08:43 <SMalyshev> I think having unnecessary cache object API just to be able to cache nulls is an overkill
2821:09:11 <bd808> DanielK_WMDE: that's my general knock against cache interface abstractions
2921:09:24 <bd808> most cache backends are different
3021:09:30 <bd808> and for good reason
3121:09:32 <DanielK_WMDE> bd808: you kind of convinced me that this is an issue, yea
3221:10:01 <bd808> switching from memcache to apc is not just a matter of changing the backend
3321:10:05 <DanielK_WMDE> addshore: i'm all for improving the caching infrastructure and interface. but i'm not clear on what the best way is.
3421:10:22 <addshore> yeh, right now its sounding like the best way is to not ;)
3521:10:26 <DanielK_WMDE> so we need tagged caching. i *really* want tagged caching. Do we want a TaggedbagOfStuff interface?
3621:11:12 <SMalyshev> Maybe. Since PSR-6 doesn't seem to address tagging anyway, that may be the way to go
3721:11:46 <TimStarling> what is tagged caching?
3821:11:46 <robla> one question we seem to be debating is do we a) need a better caching abstraction than BagOStuff? or b) is the status quo fine or c) should BagOStuff be deprecated, and not replaced with anything?
3921:12:18 <TimStarling> in what sense is PSR-6 better than BagOStuff? it has far fewer features
4021:12:26 <SMalyshev> my concern is that we don't seem to get any added value from PSR6 and since we'd need to extend cache model anyway, we'd better extend the devil we know (namely BagOfStuff)
4121:12:29 <bd808> they want to have a way to purge N cache entries that are related somehow
4221:12:44 <bd808> e.g. all cached things that are dependent on Q12345
4321:13:02 <SMalyshev> TimStarling: tagged caching is basically assigning each item 0..N tags and being able to purge all items that have specific tag
4421:13:11 <SMalyshev> (I hope that's what DanielK_WMDE meant :)
4521:13:12 <DanielK_WMDE> TimStarling: tags = buckets
4621:13:45 <TimStarling> but PSR-6 doesn't provide that either
4721:14:11 <addshore> No, I think at this stage the consensus is to ignore PSR6
4821:14:57 <SMalyshev> more restricted case is when you have hierachical tags i.e. you can have cache purge by prefix. It's a subset of tagging cache but may be easier to implement
4921:15:03 <TimStarling> memcached doesn't have tagged caching does it?
5021:15:09 <TimStarling> or redis?
5121:15:24 <TimStarling> is the proposal to switch to some other server which would support this feature?
5221:15:24 <DanielK_WMDE> addshore: ok, so rfc rejected ;) shall we repurpose the meeting to discuss alternatives / next steps? what are the needs we have?
5321:15:43 <addshore> DanielK_WMDE: sounds sensible!
5421:15:50 <SMalyshev> there's https://code.google.com/archive/p/memcached-tag/
5521:16:11 <TimStarling> if we have an external library that requires PSR-6 then it would be fairly easy and harmless to add a PSR-6 adaptor for BagOStuff
5621:16:32 <DanielK_WMDE> TimStarling: my impression was that there are psr-6 based implementations of tagging, but i didn't investigate. but discussing this use case in particular was probably not the intention of the rfc.
5721:16:33 <robla> #info <DanielK_WMDE> addshore: ok, so rfc rejected ;) shall we repurpose the meeting to discuss alternatives / next steps? what are the needs we have? <addshore> DanielK_WMDE: sounds sensible!
5821:16:39 <DanielK_WMDE> (though i'd love to see it solved)
5921:16:49 <TimStarling> but the idea of deprecating BagOStuff in favour of PSR-6, I think that is rejected
6021:16:58 <addshore> TimStarling: indeed, that was my first plan, but the library I was planning on using turned out to not be great
6121:17:11 <DanielK_WMDE> TimStarling: +1
6221:17:39 <psychoslave> The topic was related to https://phabricator.wikimedia.org/T91162, right?
6321:17:46 <robla> does my earlier proposed breakdown make sense? "a) need a better caching abstraction than BagOStuff? or b) is the status quo fine or c) should BagOStuff be deprecated, and not replaced with anything?"
6421:17:53 <addshore> for the library see https://github.com/php-cache/taggable-cache and issue see https://github.com/php-cache/issues/issues/52
6521:18:53 <DanielK_WMDE> addshore: so... we want tagging. and we want bagOStuff to be an actual interface. and we want to clarify the relationship between the different caching systems/interfaces we have (ProcessCacheLRU, BagOStuff, WANObjectCache...)
6621:19:04 <addshore> DanielK_WMDE: yup
6721:19:08 <bd808> addshore: I think you are going to have that class of problem with any backend that doesn't natively support lock for update
6821:19:43 <DanielK_WMDE> i'd also love to see bd808's concern addressed: how do we distinguish between, say, memcached and hhvm's built-in cache? Calling code shouldn't know about the concrete implementation, but it *should* know about the totally different eviction behavior.
6921:19:52 <TimStarling> when you say you want it, do you mean you want some crappy simulation of it like we have in ParserCache, JobQueue etc.?
7021:20:32 <robla> DanielK_WMDE: do we need an abstraction?
7121:20:59 * robla is concerned that DanielK_WMDE is racing off to abstraction-land when it's not clear that's wanted/needed
7221:21:55 <bd808> How about we back up to the actual problem that needs a solution?
7321:21:58 <addshore> My initial rough draft of some sort of indexed bagostuff can be seen at https://gerrit.wikimedia.org/r/#/c/278908/ indexing based on the parts that a key is constructed with and ekeping the same interface as bagostuff, but this failed due to having to extend the base rather than just implement something
7421:22:35 <addshore> My plan was to use this in the WatchedItemStore to allow more caching as cache purging would actually be possible on mass
7521:22:38 <bd808> right now this is a solution in need of a problem (or 3) to become a useful utility
7621:22:50 <DanielK_WMDE> robla: all of programming is an abstraction :) but i'm indeed trying to figure out whether we want more abstraction of caching, or perhaps less.
7721:23:32 <robla> #info question discussed: a) need a better caching abstraction than BagOStuff? or b) is the status quo fine or c) should BagOStuff be deprecated, and not replaced with anything?
7821:23:47 <DanielK_WMDE> addshore: so, the use case driving this is indeed just tagging/buckets? i thought thjat was just the one i'm interested in ;)
7921:23:51 <TimStarling> addshore, the usual way to do mass purging is to have a single key which invalidates the collection, which you check on fetch
8021:23:59 <SMalyshev> I think we're still not clear which exactly problem we're solving. If it's tagging, then I'm not sure we know how we want to do it yet, do we?
8121:24:26 <robla> #info question discussed: what problem was this RFC trying to solve?
8221:24:57 <bd808> Agree with TimStarling that indirect keys are a common solution to mass invalidation assuming the cached data has a TTL based cleanup when abaindoned
8321:25:10 <TimStarling> addshore: $item = $cache->get($itemKey); $collection = $cache->get($collectionKey); $expired = $itemKey['collection'] == $collection;
8421:25:21 <DanielK_WMDE> TimStarling: what happens when that key gets evicted=
8521:25:43 <bd808> all the things it keyed are "lost"
8621:25:44 <TimStarling> when the collection key is evicted? the whole collection is invalidated obviously
8721:25:58 <TimStarling> but it never is because it's the hottest key in the collection
8821:26:08 <TimStarling> since it is accessed as often as all the others put together
8921:26:19 <DanielK_WMDE> if the algorithm is LRU or otherwise decent
9021:26:32 <DanielK_WMDE> if it's FIFO, you lose
9121:26:42 <DanielK_WMDE> but yea, i see the point
9221:26:58 <TimStarling> this is how it is done in the parser cache, and the job queue has a similar concept of "root job"
9321:27:21 <SMalyshev> so I think if we had an API implementing this as simple operations get/set/delete it may be helpful
9421:27:23 <TimStarling> and the message cache has a similar concept of a hash key which invalidates a local store
9521:27:24 <DanielK_WMDE> yea, it would be nice to generalize this a bit
9621:27:35 <DanielK_WMDE> the way this is implemented in ParserCache is a bit... arcane.
9721:27:48 <TimStarling> well, the use case is very special
9821:28:18 <TimStarling> the code is only as complex as the reality it models :)
9921:28:23 <DanielK_WMDE> also, this doesn't allow me to enumerate the stale keys, right? so i can't actually purge, or update
10021:28:34 <TimStarling> right
10121:28:54 <bd808> a cache that allows you to enumerate keys is a very special type of cache
10221:29:04 <TimStarling> all available operations are O(1) which probably helps programmers to implement efficient code
10321:30:18 <TimStarling> I called it a crappy simulation of tagged caching because invalidation is not quite the same as deletion
10421:30:25 <DanielK_WMDE> though it's no guarantee ;)
10521:31:06 <TimStarling> specifically, invalidation doesn't create free space in the cache, it relies on later eviction to make free space
10621:31:45 <TimStarling> deleting invalidated keys would be better than LRU eviction since those keys are guaranteed to not hold useful data
10721:31:56 <SMalyshev> I wonder what happens with sharded cache and collections... Would that mean whereever the collection key is stored, everybody talks to that place?
10821:32:01 * robla put a list of questions in https://phabricator.wikimedia.org/T130528#2265793
10921:32:25 <TimStarling> SMalyshev: yes
11021:32:43 <DanielK_WMDE> TimStarling: so, the wikidata use case is triggering RefershLinks for all pages affected by a change on wikidata.org. We are currently doing this using a tracking table in the db. but the tracking table is effectively for tracking parser cache entries, and may need to be updated when things get rendered on the fly (causing a master insert in a GET request).
11121:33:00 <DanielK_WMDE> for that use case, we need enumeration
11221:33:06 <SMalyshev> that may create some undesired imbalances... not sure if it's a real problem or imagined
11321:33:28 <TimStarling> DanielK_WMDE: you could use redis
11421:34:45 <TimStarling> SMalyshev: memcached is pretty fast, in the olden days of 100 Mbps network connections we used to saturate ports by accident but it's not such a problem now
11521:34:57 <TimStarling> a single memcached server can handle a very hot key
11621:35:24 <DanielK_WMDE> TimStarling: possibly. i usually try to avoid hard dependencies on external services. but it's probably worth investiagting.
11721:35:40 <DanielK_WMDE> anyway - i didn't want us to get hung up on that use case specifically
11821:36:08 <DanielK_WMDE> #info <TimStarling> addshore, the usual way to do mass purging is to have a single key which invalidates the collection, which you check on fetch
11921:36:40 <TimStarling> yeah, read the redis manual, it is quite nice, and several things are already using it
12021:37:01 <TimStarling> you can always build a huge abstract hierarchy on top of it if it makes you feel better
12121:37:11 <DanielK_WMDE> hehe, not really :P
12221:37:48 <DanielK_WMDE> just a thin lyer or two... ;)
12321:38:23 <addshore> okay, well, I think that may wrap everything up?
12421:38:25 <SMalyshev> speaking of which, so do we want a generic tagging implementation doing this collection key thing?
12521:38:44 <DanielK_WMDE> addshore: do you have everything you need to continue? are you happy with the meeting?
12621:39:02 <DanielK_WMDE> SMalyshev: i'm for it :D
12721:39:09 <addshore> as am I
12821:39:26 <TimStarling> I would be happier with an implementation that has each key in a single collection
12921:39:27 <SMalyshev> so that seems to be one meeting outcome/todo
13021:39:30 <TimStarling> instead of 0..N
13121:39:49 <TimStarling> until we actually have a use case for tagging
13221:39:59 <SMalyshev> TimStarling: we could start with that... I didn't look into use cases
13321:40:08 <TimStarling> assuming that is enough for addshore
13421:40:37 <bd808> We have 1..N use cases for Varnish cache invalidation but that is a different beast
13521:40:52 <TimStarling> for Daniel's use case he just needs to use redis :)
13621:40:54 <bd808> And I think bb.lack is already thinking about that
13721:41:23 <bd808> LADD my_thing
13821:41:32 <addshore> would that work in the case of watched items? I would want to purge all items for a user while also being able to purge all items for a page?
13921:42:35 <addshore> wait, yes, I guess :)
14021:42:47 <TimStarling> addshore: by purge you mean make the cache key go away?
14121:43:11 <addshore> yes ;) so essentially it would be 2 sets of collections
14221:43:23 <TimStarling> yes, it would work I think
14321:43:36 <bd808> s/LADD/LPUSH/ (memory fail)
14421:43:56 <addshore> hmm, but perhaps in that case If I invalidate the colleciton key for say the Berlin article said key could also remain in some user collections?
14521:44:50 <TimStarling> addshore: where is your use case documented again?
14621:46:42 <DanielK_WMDE> TimStarling: the skin shows whether user U watches current page X. We don't want to hit the DB for that, so we cache. When the user (bulk?) edits their watchlist, we need to purge that.
14721:46:56 <DanielK_WMDE> not sure when we'd need to purge all waicthes for a page, though... when moving it?
14821:47:09 <addshore> DanielK_WMDE: when the notification timestamp is updated
14921:47:13 <addshore> on edit
15021:47:23 <DanielK_WMDE> ah, right, the timestamp.
15121:48:57 <addshore> While looking at all of this was well I looked at the usage of the caching (which was carried over form User) https://grafana.wikimedia.org/dashboard/db/mediawiki-watcheditemstore and the level of cache hits is very low really IMO, thus another option could be just remove this particular caching? https://gerrit.wikimedia.org/r/#/c/286892/
15221:49:35 <DanielK_WMDE> addshore: maybe a watchlist caching rfc would be useful?
15321:51:15 <addshore> DanielK_WMDE: perhaps, to increase the level of caching, some form of tagging is needed, or to reduce complexity we can just remove it causing roughly a 10% increase in the slave db hits to retrieve watched item detials
15421:51:19 <robla> addshore: were those links an answer to TimStarling's question about use cases?
15521:51:33 <TimStarling> robla: probably nowhere
15621:51:39 <bd808> addshore: a HashBagOStuff cache is just an in process cache for the duration of a request
15721:51:47 <addshore> bd808: yes
15821:51:53 <TimStarling> I think that is the answer
15921:53:16 <robla> #info addshore agrees with bd808 that a HashBagOStuff cache is just an in process cache for the duration of a request
16021:53:22 <DanielK_WMDE> #info watchlist caching might be worth an rfc on its own.
16121:53:32 <bd808> addshore: you don't need to worry about invalidation at all in an in-process cache
16221:53:34 <addshore> bd808: It can't cache over processes without being able to invalidate stuff, in process caching is just what has been taken from User
16321:53:37 * bd808 is confused
16421:53:43 <bd808> ah
16521:54:03 <addshore> bd808: yes, but to actually make the caching do something usefull it needs to not be in process and thus you need to be able to invalidate things!
16621:54:25 <bd808> a 10% hit rate on an in process cache is actually not bad. the cost of said cache is really really small
16721:54:46 <TimStarling> I guess I need to do a review of WatchedItemStore
16821:55:03 * robla notes we're coming up on the end of the hour. RFC is declined; rest of the discussion is about cache abstraction
16921:55:23 <addshore> bd808: I'm quite surprised about the hits at all, I was wondering why the caching was in User
17021:55:30 <DanielK_WMDE> #info use cases for "tagged" caching: watchlist entries, wikibase usage tracking, parser cache, web cache (varnish buckets), ...
17121:56:14 <addshore> bd808: that sounds like a vote to keep the caching in there!
17221:56:30 <TimStarling> what channel should we go to for post-meeting discussion?
17321:56:32 <addshore> robla: yep, rfc for psr6 is defintly declined :)
17421:56:50 <bd808> #-tech?
17521:57:04 <addshore> sounds good
17621:57:22 <robla> #info discussion planned to continue on #wikimedia-tech
17721:58:10 <robla> next week, we're planning to discuss T113034
17821:58:27 <stashbot> T113034: RFC: Overhaul Interwiki map, unify with Sites and WikiMap - https://phabricator.wikimedia.org/T113034
17921:59:03 <psychoslave> Well, that was dense and informative for me. Thank you
18021:59:28 <robla> #link https://phabricator.wikimedia.org/E171 next week's RFC meeting
18121:59:49 <robla> alright, thank you all!
18222:00:15 <robla> #endmeeting

daniel renamed this event from RFC Meeting: PSR-6 Cache interface in Mediawiki core (2016-05-04, #wikimedia-office) to ArchCom RFC Meeting Wxx: <topic TBD> (<see "Starts" field>, #wikimedia-office).Nov 21 2016, 6:11 PM
daniel changed the host of this event from RobLa-WMF to daniel.
daniel uninvited: Addshore.
daniel updated the event description. (Show Details)
daniel updated the event description. (Show Details)Dec 9 2016, 7:43 AM
daniel renamed this event from ArchCom RFC Meeting Wxx: <topic TBD> (<see "Starts" field>, #wikimedia-office) to RFC Meeting: PSR-6 Cache interface in Mediawiki core (2016-05-04, #wikimedia-office).