21:02:48 #startmeeting E169 RFC Meeting: PSR-6 Cache interface in Mediawiki core 21:02:48 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. 21:02:48 Useful Commands: #action #agreed #help #info #idea #link #topic #startvote. 21:02:48 The meeting name has been set to 'e169_rfc_meeting__psr_6_cache_interface_in_mediawiki_core' 21:03:05 *waves* 21:03:11 oh nice. I was wondering why nobody's here :) 21:04:08 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 21:04:54 #info 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 21:05:11 #link https://phabricator.wikimedia.org/E169 21:05:12 addshore: you mean an interface different from BagOStuff? 21:05:23 #link https://phabricator.wikimedia.org/T130528 21:05:32 addshore: there's also the more recent WANObjectCache 21:05:44 So, the issue that I ran into that started me off looking at this is that BagOStuff is not an Interface 21:05:48 Hi All 21:06:02 addshore: does that matter? 21:06:14 it is our common (and crappy) cache abstraction 21:06:33 addshore: that's easy to fix. Just mkae it an interface, and rename the current implementation to bag=Stuff base or AbstractBagOStuff 21:06:36 bd808, yes. I ran into issues while just trying to extend bagOfStuff to make a Taggable / Indexed BagOfStuff 21:06:48 crappy only because abstracting caching pretty much always falis in my experience 21:07:19 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? 21:07:31 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? 21:07:35 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 21:07:44 I explained the things I explictly dislike about PSR6 at https://phabricator.wikimedia.org/T130528#2245415 21:07:54 especially in two-step cache interface whose sole purpose seems to be working around null problem 21:08:17 #info I explained the things I explictly dislike about PSR6 at https://phabricator.wikimedia.org/T130528#2245415 21:08:39 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 21:08:43 I think having unnecessary cache object API just to be able to cache nulls is an overkill 21:09:11 DanielK_WMDE: that's my general knock against cache interface abstractions 21:09:24 most cache backends are different 21:09:30 and for good reason 21:09:32 bd808: you kind of convinced me that this is an issue, yea 21:10:01 switching from memcache to apc is not just a matter of changing the backend 21:10:05 addshore: i'm all for improving the caching infrastructure and interface. but i'm not clear on what the best way is. 21:10:22 yeh, right now its sounding like the best way is to not ;) 21:10:26 so we need tagged caching. i *really* want tagged caching. Do we want a TaggedbagOfStuff interface? 21:11:12 Maybe. Since PSR-6 doesn't seem to address tagging anyway, that may be the way to go 21:11:46 what is tagged caching? 21:11:46 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? 21:12:18 in what sense is PSR-6 better than BagOStuff? it has far fewer features 21:12:26 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) 21:12:29 they want to have a way to purge N cache entries that are related somehow 21:12:44 e.g. all cached things that are dependent on Q12345 21:13:02 TimStarling: tagged caching is basically assigning each item 0..N tags and being able to purge all items that have specific tag 21:13:11 (I hope that's what DanielK_WMDE meant :) 21:13:12 TimStarling: tags = buckets 21:13:45 but PSR-6 doesn't provide that either 21:14:11 No, I think at this stage the consensus is to ignore PSR6 21:14:57 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 21:15:03 memcached doesn't have tagged caching does it? 21:15:09 or redis? 21:15:24 is the proposal to switch to some other server which would support this feature? 21:15:24 addshore: ok, so rfc rejected ;) shall we repurpose the meeting to discuss alternatives / next steps? what are the needs we have? 21:15:43 DanielK_WMDE: sounds sensible! 21:15:50 there's https://code.google.com/archive/p/memcached-tag/ 21:16:11 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 21:16:32 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. 21:16:33 #info addshore: ok, so rfc rejected ;) shall we repurpose the meeting to discuss alternatives / next steps? what are the needs we have? DanielK_WMDE: sounds sensible! 21:16:39 (though i'd love to see it solved) 21:16:49 but the idea of deprecating BagOStuff in favour of PSR-6, I think that is rejected 21:16:58 TimStarling: indeed, that was my first plan, but the library I was planning on using turned out to not be great 21:17:11 TimStarling: +1 21:17:39 The topic was related to https://phabricator.wikimedia.org/T91162, right? 21:17:46 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?" 21:17:53 for the library see https://github.com/php-cache/taggable-cache and issue see https://github.com/php-cache/issues/issues/52 21:18:53 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...) 21:19:04 DanielK_WMDE: yup 21:19:08 addshore: I think you are going to have that class of problem with any backend that doesn't natively support lock for update 21:19:43 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. 21:19:52 when you say you want it, do you mean you want some crappy simulation of it like we have in ParserCache, JobQueue etc.? 21:20:32 DanielK_WMDE: do we need an abstraction? 21:20:59 * robla is concerned that DanielK_WMDE is racing off to abstraction-land when it's not clear that's wanted/needed 21:21:55 How about we back up to the actual problem that needs a solution? 21:21:58 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 21:22:35 My plan was to use this in the WatchedItemStore to allow more caching as cache purging would actually be possible on mass 21:22:38 right now this is a solution in need of a problem (or 3) to become a useful utility 21:22:50 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. 21:23:32 #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? 21:23:47 addshore: so, the use case driving this is indeed just tagging/buckets? i thought thjat was just the one i'm interested in ;) 21:23:51 addshore, the usual way to do mass purging is to have a single key which invalidates the collection, which you check on fetch 21:23:59 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? 21:24:26 #info question discussed: what problem was this RFC trying to solve? 21:24:57 Agree with TimStarling that indirect keys are a common solution to mass invalidation assuming the cached data has a TTL based cleanup when abaindoned 21:25:10 addshore: $item = $cache->get($itemKey); $collection = $cache->get($collectionKey); $expired = $itemKey['collection'] == $collection; 21:25:21 TimStarling: what happens when that key gets evicted= 21:25:43 all the things it keyed are "lost" 21:25:44 when the collection key is evicted? the whole collection is invalidated obviously 21:25:58 but it never is because it's the hottest key in the collection 21:26:08 since it is accessed as often as all the others put together 21:26:19 if the algorithm is LRU or otherwise decent 21:26:32 if it's FIFO, you lose 21:26:42 but yea, i see the point 21:26:58 this is how it is done in the parser cache, and the job queue has a similar concept of "root job" 21:27:21 so I think if we had an API implementing this as simple operations get/set/delete it may be helpful 21:27:23 and the message cache has a similar concept of a hash key which invalidates a local store 21:27:24 yea, it would be nice to generalize this a bit 21:27:35 the way this is implemented in ParserCache is a bit... arcane. 21:27:48 well, the use case is very special 21:28:18 the code is only as complex as the reality it models :) 21:28:23 also, this doesn't allow me to enumerate the stale keys, right? so i can't actually purge, or update 21:28:34 right 21:28:54 a cache that allows you to enumerate keys is a very special type of cache 21:29:04 all available operations are O(1) which probably helps programmers to implement efficient code 21:30:18 I called it a crappy simulation of tagged caching because invalidation is not quite the same as deletion 21:30:25 though it's no guarantee ;) 21:31:06 specifically, invalidation doesn't create free space in the cache, it relies on later eviction to make free space 21:31:45 deleting invalidated keys would be better than LRU eviction since those keys are guaranteed to not hold useful data 21:31:56 I wonder what happens with sharded cache and collections... Would that mean whereever the collection key is stored, everybody talks to that place? 21:32:01 * robla put a list of questions in https://phabricator.wikimedia.org/T130528#2265793 21:32:25 SMalyshev: yes 21:32:43 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). 21:33:00 for that use case, we need enumeration 21:33:06 that may create some undesired imbalances... not sure if it's a real problem or imagined 21:33:28 DanielK_WMDE: you could use redis 21:34:45 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 21:34:57 a single memcached server can handle a very hot key 21:35:24 TimStarling: possibly. i usually try to avoid hard dependencies on external services. but it's probably worth investiagting. 21:35:40 anyway - i didn't want us to get hung up on that use case specifically 21:36:08 #info addshore, the usual way to do mass purging is to have a single key which invalidates the collection, which you check on fetch 21:36:40 yeah, read the redis manual, it is quite nice, and several things are already using it 21:37:01 you can always build a huge abstract hierarchy on top of it if it makes you feel better 21:37:11 hehe, not really :P 21:37:48 just a thin lyer or two... ;) 21:38:23 okay, well, I think that may wrap everything up? 21:38:25 speaking of which, so do we want a generic tagging implementation doing this collection key thing? 21:38:44 addshore: do you have everything you need to continue? are you happy with the meeting? 21:39:02 SMalyshev: i'm for it :D 21:39:09 as am I 21:39:26 I would be happier with an implementation that has each key in a single collection 21:39:27 so that seems to be one meeting outcome/todo 21:39:30 instead of 0..N 21:39:49 until we actually have a use case for tagging 21:39:59 TimStarling: we could start with that... I didn't look into use cases 21:40:08 assuming that is enough for addshore 21:40:37 We have 1..N use cases for Varnish cache invalidation but that is a different beast 21:40:52 for Daniel's use case he just needs to use redis :) 21:40:54 And I think bb.lack is already thinking about that 21:41:23 LADD my_thing 21:41:32 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? 21:42:35 wait, yes, I guess :) 21:42:47 addshore: by purge you mean make the cache key go away? 21:43:11 yes ;) so essentially it would be 2 sets of collections 21:43:23 yes, it would work I think 21:43:36 s/LADD/LPUSH/ (memory fail) 21:43:56 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? 21:44:50 addshore: where is your use case documented again? 21:46:42 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. 21:46:56 not sure when we'd need to purge all waicthes for a page, though... when moving it? 21:47:09 DanielK_WMDE: when the notification timestamp is updated 21:47:13 on edit 21:47:23 ah, right, the timestamp. 21:48:57 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/ 21:49:35 addshore: maybe a watchlist caching rfc would be useful? 21:51:15 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 21:51:19 addshore: were those links an answer to TimStarling's question about use cases? 21:51:33 robla: probably nowhere 21:51:39 addshore: a HashBagOStuff cache is just an in process cache for the duration of a request 21:51:47 bd808: yes 21:51:53 I think that is the answer 21:53:16 #info addshore agrees with bd808 that a HashBagOStuff cache is just an in process cache for the duration of a request 21:53:22 #info watchlist caching might be worth an rfc on its own. 21:53:32 addshore: you don't need to worry about invalidation at all in an in-process cache 21:53:34 bd808: It can't cache over processes without being able to invalidate stuff, in process caching is just what has been taken from User 21:53:37 * bd808 is confused 21:53:43 ah 21:54:03 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! 21:54:25 a 10% hit rate on an in process cache is actually not bad. the cost of said cache is really really small 21:54:46 I guess I need to do a review of WatchedItemStore 21: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 21:55:23 bd808: I'm quite surprised about the hits at all, I was wondering why the caching was in User 21:55:30 #info use cases for "tagged" caching: watchlist entries, wikibase usage tracking, parser cache, web cache (varnish buckets), ... 21:56:14 bd808: that sounds like a vote to keep the caching in there! 21:56:30 what channel should we go to for post-meeting discussion? 21:56:32 robla: yep, rfc for psr6 is defintly declined :) 21:56:50 #-tech? 21:57:04 sounds good 21:57:22 #info discussion planned to continue on #wikimedia-tech 21:58:10 next week, we're planning to discuss T113034 21:58:27 T113034: RFC: Overhaul Interwiki map, unify with Sites and WikiMap - https://phabricator.wikimedia.org/T113034 21:59:03 Well, that was dense and informative for me. Thank you 21:59:28 #link https://phabricator.wikimedia.org/E171 next week's RFC meeting 21:59:49 alright, thank you all! 22:00:15 #endmeeting