https://bugzilla.wikimedia.org/show_bug.cgi?id=61939

Matthias Mullie <mmul...@wikimedia.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mmul...@wikimedia.org

--- Comment #5 from Matthias Mullie <mmul...@wikimedia.org> ---
The last ones here seem ok to me. Only the original one still seems to have the
problem.

I've done some digging and I assume we failed to fetch the content from
ExternalStorage, and cached the incorrect result.

    $ mwscript eval.php --wiki enwiki:

    > $uuid = Flow\Model\UUID::create( 'rp2pfb6ete6pq6fg' );
    > $collection = Flow\Model\PostCollection::newFromId( $uuid );
    > $revision = $collection->getLastRevision();
    > var_dump( $revision );

$revision->content seems to have a false value here. Judging from
AbstractRevision.php, the value should either be the full text or an empty
string ''.

Since the content is stored in ExternalStorage, it should be fetched via
RevisionStorage::mergeExternalContent, which will fetch the content via
ExternalStore::batchFetchFromURLs, which can return false. So I'm pretty sure
that's where the false comes from. And it looks like we're only caching the
value after that.

I've looked at clearing the cache, but has less luck there. It looks like the
key the revision is saved at should be
enwiki:flow_revision:v4:descendant:rp2pfb6ete6pq6fg:3.0 (I've also tried :pk:
instead of :descendant: and :2.0 instead of :3.0, but no luck). Trying to
either fetch those keys from cache returns a false, and trying to delete them
doesn't change anything. I may be assuming an incorrect cache key here, though.
Clearing the cache would only be a stopgap for this particular instance though,
and not properly fix the issue.

Any way; to properly fix this, we should (unless someone has a better idea):
* Make it possible to let our backend storage know to our cache-layers to not
cache certain data -> the proper fix IMO, but a fair bit of work

Inferior alternatives:
* Cache the real DB return (prior to ExternalStore::batchFetchFromURLs) and do
ExternalStore::batchFetchFromURLs afterwards -> probably the easiest fix, but
would result in a lot more calls to resolve data from ExternalStore.
* Implement an 2nd ExternalStore::batchFetchFromURLs in AbstractRevision, to
retry fetching content in case $revision->content is false -> easy too, but
duplicate code, and still unneccessary calls to ExternalStore for the duration
of the corrupt cache

Oh, and also: should we still have occurences of $revision->content being
false, we should probably make AbstractRevision::getContent return a 'We could
not fetch this content' message instead of displaying nothing.

-- 
You are receiving this mail because:
You are the assignee for the bug.
You are on the CC list for the bug.
_______________________________________________
Wikibugs-l mailing list
Wikibugs-l@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikibugs-l

Reply via email to