[00:59:04] Krinkle: looking at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/573342/17/includes/libs/CSSMin.php (and the new lib) [01:59:39] AaronSchulz: if you're around, it seems we have another comons file disappearance issue only this time it lasts 24h per file instead of a few seconds/minutes. [02:00:04] can you take another look at the filebackend commits we landed? remember that 2 weeks went out at once [02:00:40] e.g. https://en.wikipedia.org/wiki/File:Allan_Shivers_(1956).png is consistently seen as not existing [02:00:44] as are various other Commons files [02:00:53] https://en.wikipedia.org/wiki/Allan_Shivers [02:04:05] I've just confirmed that it works on wmf.14 if I revert back on mwdebug1001 [02:17:56] summary at https://phabricator.wikimedia.org/T267668#6619742 [02:18:03] likely culprit: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/604161 [02:53:49] I see a potential key mis-match there with regards to the descripton page [02:53:59] however those keys already didn't match before that commit either [02:54:33] plus, it doesn't explain why it happens even on Codfw with cold memcached. [02:54:47] and afaik descripton pages are not relevant when parser is trying to link a file [03:07:33] OK, so I think I may've gotten to the heart of the issue. [03:07:51] I think the issue is that LocalRepo no longer has a consistent getSharedKey method. It previously returned a key always [03:07:58] whereas not only if hasSharedCache is true. [03:08:14] Which, while true for the ForeignDBLB case, won't be true for the plain LocalRepo case [03:08:58] hm.. except hasAccessibleSharedCache = true by default in the parent class [03:11:28] Ah but it now adds $this FileRepo->getName() to the key [03:11:49] that's presumably not gonna match 'local' for the foreign class calling that [17:22:25] Krinkle: is this a het deploy thing or persistent? [19:59:10] AaronSchulz: only one version in prod (wmf.16), and consistently/persistent for any given file for upto 24h [19:59:18] I've reverted it now, which antoine has deployed this morning [20:01:55] AaronSchulz: I assume the issue is that the return value of getSharedCacheKey varies between commons/LocalRepo and (otherwiki)/Foreign since it doesn't the same 'name'. I don't know why name was added, maybe a copy mistake from the File class where a similar line existed? [20:02:51] wikiId should suffice which is already injected into Foreign Repo by config for the name of the remote wiki (and used e.g. for db conns etc) so there shouldn't be a need for the repo "name" as well. [20:51:26] Krinkle: that probably would work [20:51:43] it worked so far :) [20:51:58] * AaronSchulz is trying to go through edge cases [20:52:21] of course, it will generally work [20:52:57] we should also unit test it this time around e.g. produce two different repos with a non-empty bagostuff (hash?) and confrm that they make the same key (unit test) and as integration test maybe check that if you ask for 'exampe.jpg' existing when it doesn't eixst yet, and then make it existant and call invalidate, that it actually ends up invalidating it correctly [20:53:05] there is a fixture for that test already but it is marked as incomplete currently