llampak wrote:

BTW:
$wgMemc->set( $APSkipKey, 'true', 3600 * 24 * $spacing * ( $benchmarks - $needed - 1 ) );

Isn't $benchmarks less than $needed?

This all looks like some fairly old code, so if it's broken, it's been broken for a while. That said, I agree that it looks like there might be some scope problems with $spacing, $benchmarks and $needed (at a minimum, there's an explode statement or something that needs a comment explaining where the variables come from)

I'm leaving this assigned to myself for now, but will reassign when we're not in a release crunch.

Probably crept in when editSpacingCheck was made it's own function. This just seems to make should-be negative cache hits not hit. Fixing this now.

(In reply to comment #0)

Actually I'm not sure if I'm not wrong, because I thought something like this
should cause a crash, so maybe it's ok, but...

$pass = self::editSpacingCheck(
    $wgFlaggedRevsAutoconfirm['spacing'],
    $wgFlaggedRevsAutoconfirm['benchmarks'],
    $user
);
# Make a key to store the results
if ( !$pass ) {
    $wgMemc->set( $APSkipKey, 'true',
        3600 * 24 * $spacing * ( $benchmarks - $needed - 1 ) );
    return true;
} else {
    $wgMemc->set( $sTestKey, 'true', 7 * 24 * 3600 );
}

$spacing, $benchmarks and $needed seem to be local variables in
self::editSpacingCheck(). As far as I can see there are no such variables in
the current scope. Or are there?

Fixed in r74710

(In reply to comment #1)

BTW:
$wgMemc->set( $APSkipKey, 'true', 3600 * 24 * $spacing * ( $benchmarks -
$needed - 1 ) );

Isn't $benchmarks less than $needed?

Also inadvertently fixed in r74710. Those were obviously backwards.