Last modified: 2014-02-12 23:32:46 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T46024, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 44024 - ObjectCache changes (BagOStuff->add) in 1.20.x break XCache 3.x support
ObjectCache changes (BagOStuff->add) in 1.20.x break XCache 3.x support
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.20.x
PC Linux
: High major with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-16 08:28 UTC by Björn M
Modified: 2014-02-12 23:32 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
(bug 44024) xcache_get returns null on non-existent keys (941 bytes, patch)
2013-01-18 15:31 UTC, Pierre Schmitz
Details

Description Björn M 2013-01-16 08:28:42 UTC
I recently faced the exact same problem as described in this old Maillist thread:
http://www.gossamer-threads.com/lists/engine?do=post_view_printable;post=307511;list=wiki

The problem is, that MediaWiki in the 1.20.x Branch with at least XCache 3.x (didn't test another XCache version) takes more then 10 seconds to process every request. The reason for that is a loop with a 10 seconds timeout within "MessageCache::load": 
-------------------------------------
...
Article::view                                                                    1     10064.915     10064.915        99.300%   1640737  (    10064.915 -    10064.915) [291]
MessageCache::load                                                               1     10037.354     10037.354        99.028%      1606  (    10037.354 -    10037.354) [1]
OutputPage::output                                                               1        43.939        43.939         0.434%   1838602  (       43.939 -       43.939) [583]
Output-skin                                                                      1        43.630        43.630         0.430%   1827387  (       43.630 -       43.630) [582]
...
-------------------------------------

The speculation within the maillist thread was that it was some sort of XCache configuration issue, but everything seemed to be fine for me and before the hangup MediaWiki was already succesfully able to add values to the VarCache like

mediawikitest-wiki2_:user:id:1
mediawikitest-wiki2_:resourceloader:filter:...

So i debugged my way through the code and came to the conclusion that the reason for the broken XCache support in 1.20.x are the changes made to the BagOStuff->add function:

1.19:
----------------------------------
	public function add( $key, $value, $exptime = 0 ) {
		if ( !$this->get( $key ) ) {
			$this->set( $key, $value, $exptime );

			return true;
		}
	}
------------------------------------

1.20:
------------------------------------
	public function add( $key, $value, $exptime = 0 ) {
		if ( $this->get( $key ) === false ) {
			return $this->set( $key, $value, $exptime );
		}
		return false; // key already set
	}
------------------------------------

The other keys like "mediawikitest-wiki2_:user:id:1" work fine, because MediaWiki there does not use the add function, but the set function directly instead.
So the changes now break the XCache support, because "get" basically returns the returnvalue of "xcache_get" (serialising aside, because that is a non issue here). It is basically the same function for XCache and APC and this might still work for APC because apc_fetch does infact return "false" in case the key is not found. This is not the case for XCache!
xcache_get simply does not return false in case of a not found key (its an empty string), which causes the new BagOStuff->add function to believe that the key has already been set. 

Assuming the changes to the add function were made for a reason, the easiest way to fix this seems to modify the XCacheBagOStuff->get function to handle not yet set keys correctly as intended by XCache, which then would result in something like this: 

-----------------------------------------
class XCacheBagOStuff extends BagOStuff {
        /**
         * Get a value from the XCache object cache
         *
         * @param $key String: cache key
         * @return mixed
         */
        public function get( $key ) {
                if (!xcache_isset( $key )) { return false; }

                $val = xcache_get( $key );

                if ( is_string( $val ) ) {
                        if ( $this->isInteger( $val ) ) {
                                $val = intval( $val );
                        } else {
                                $val = unserialize( $val );
                        }
                }

                return $val;
        }

-----------------------------------------

With this simple modification it now works fine for me.
Comment 1 Pierre Schmitz 2013-01-18 15:31:45 UTC
Created attachment 11649 [details]
(bug 44024) xcache_get returns null on non-existent keys

Unfortunately I just discovered this bug report when I already debugged this issue and came to the same conclusion.

I attached a git patch for the master branch that fixes the issue. I confirmed with the XCache author that indeed null is returned in case the key does not exist. So we just test for null; saves us one call into xcache from the solution above.

Greetings,

Pierre
Comment 2 Aaron Schulz 2013-01-18 20:24:32 UTC
Can the patch be posted to http://www.mediawiki.org/wiki/Gerrit? Thanks
Comment 3 OverlordQ 2013-01-19 03:48:56 UTC
I ran into this issue today as well, commited to Gerrit.

https://gerrit.wikimedia.org/r/44754

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links