Page MenuHomePhabricator

Content-Encoding set to none/identity after upgrade to 1.35
Closed, ResolvedPublic

Description

After upgrading my wiki from 1.34.1 to 1.35.0, it is sometimes responding with Content-Encoding: none.

I tried updating to the latest version on REL1_35, and it instead responds with Content-Encoding: identity.

When I load the main page, it properly sends Content-Encoding: gzip (loaded from file cache), but after visiting a Special page (for example, but almost any page), it does not compress the output.

This seems to be caused by a change from T206283 - rMW4f11b614544b: Avoid using "enqueue" mode for deferred updates in doPostOutputShutdown.

Event Timeline

Aklapper added a subscriber: aaron.

@Prod: Did aaron explicitly agree to work on this, as you set aaron as assignee?

Prod updated the task description. (Show Details)

I checked what was in DeferredUpdates::$postSendUpdates:

array(2) {
  [0]=>
  object(MWCallableUpdate)#246 (3) {
    ["callback":"MWCallableUpdate":private]=>
    object(Closure)#253 (0) {
    }
    ["fname":"MWCallableUpdate":private]=>
    string(26) "Pingback::schedulePingback"
    ["trxRoundRequirement":"MWCallableUpdate":private]=>
    int(2)
  }
  [1]=>
  object(ViewCountUpdate)#500 (1) {
    ["pageId":protected]=>
    int(107841)
  }
}

The second one is HitCounters, and the first seems to be checking if a pingback is required (disabled by default).

Krinkle subscribed.

Untagging HitCounters for now, which I'm pretty sure is not causing this. However, I can totally see how this is correlated with that extension for you since for performance reasons MediaWiki core and extensions generally do not perform post-send updates or jobs on regular page views, so you would only find this issue after (relatively) less common actions like registering an account or submitting an edit.

HitCounters is one of very few extensions that indeed performs updates on every page view, and thus exposes this issue really well.

Krinkle changed the task status from Open to Stalled.Dec 14 2020, 9:09 PM
Krinkle triaged this task as Medium priority.
Krinkle added a project: Performance-Team.

Marking stalled until the parent is fixed which seems to be the root cause here. However the Content-Encoding value differing in this way is certainly suspicious and not something I've seen before. To my knowledge we don't set either of those values anywhere explicitly, so that's worth looking into after the the immediate issue is solved.

This issue is due to T258877: MediaWiki sets invalid Content-Encoding: none.

Even with HitCounters disabled, I'm still seeing it put into "identity" mode.

The pingback seems to be triggering identity mode for me, and I'm not sure how to disable that, short of commenting it out in core.

The pingback seems to be triggering identity mode for me, and I'm not sure how to disable that, short of commenting it out in core.

The Pingback feature can be turned off via wgPingback = false, at which point it does "nothing". However it does indeed still do a very light queue and pop through DeferredUpdates which is a pretty core feature to be broken.

But, indeed as much as DeferredUpdates working is critical to a working wiki, it generally does not get involved on simple read requests. Being able to turn even that off, as part of fault tolerance seems useful.

Fixed:

Change 650643 had a related patch set uploaded (by Krinkle):
[mediawiki/core@master] Pingback: Don't instantiate service if disabled by configuration

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

The pingback seems to be triggering identity mode for me, and I'm not sure how to disable that, short of commenting it out in core.

The Pingback feature can be turned off via wgPingback = false, at which point it does "nothing". However it does indeed still do a very light queue and pop through DeferredUpdates which is a pretty core feature to be broken.

But, indeed as much as DeferredUpdates working is critical to a working wiki, it generally does not get involved on simple read requests. Being able to turn even that off, as part of fault tolerance seems useful.

Fixed:

Change 650643 had a related patch set uploaded (by Krinkle):
[mediawiki/core@master] Pingback: Don't instantiate service if disabled by configuration

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

Would it be possible to get this backported to REL1_35? Or can I just drop the master branch includes/Pingback.php file into my 1.35 install?

Would it be possible to get this backported to REL1_35? Or can I just drop the master branch includes/Pingback.php file into my 1.35 install?

This is not a bug fix and is non-trivial new code that I think should sit through the regular development cycle to ensure wide test coverage. In my opinion too risky to backport.

It is a workaround for a bug, not the bug fix itself. This and parent task are still being worked on by Aaron, which will address the underlying issues regardless of Pingback. If you'd like to resolve this sooner, you could write a minimal patch to REL1_35 for disabling Pingback. If that works for you and you can share it on Gerrit, I'd be happy to give it a review if it's sufficiently minimal and low-risk.

Would it be possible to get this backported to REL1_35? Or can I just drop the master branch includes/Pingback.php file into my 1.35 install?

This is not a bug fix and is non-trivial new code that I think should sit through the regular development cycle to ensure wide test coverage. In my opinion too risky to backport.

It is a workaround for a bug, not the bug fix itself. This and parent task are still being worked on by Aaron, which will address the underlying issues regardless of Pingback. If you'd like to resolve this sooner, you could write a minimal patch to REL1_35 for disabling Pingback. If that works for you and you can share it on Gerrit, I'd be happy to give it a review if it's sufficiently minimal and low-risk.

I had seen in T270540 that It's a fairly simple and well-isolated component that consists of only a single class, so I was hoping it would be a trivial drop-in replacement. As this seems to not be the case, I'll just comment out the section in the code to disable the functionality in my deployment.

For others who encounter the same issue, as a temporary workaround, removing all the contents of the /includes/Pingback.php function schedulePingback was enough to disable the feature.

For others who encounter the same issue, as a temporary workaround, removing all the contents of the /includes/Pingback.php function schedulePingback was enough to disable the feature.

To save you having to potentially find the code again in future, just adding a return; at the top would've been easier

	/**
	 * Schedule a deferred callable that will check if a pingback should be
	 * sent and (if so) proceed to send it.
	 */
	public static function schedulePingback() {
		DeferredUpdates::addCallableUpdate( function () {
			$instance = new Pingback;
			if ( $instance->shouldSend() ) {
				$instance->sendPingback();
			}
		} );
	}

becomes

	/**
	 * Schedule a deferred callable that will check if a pingback should be
	 * sent and (if so) proceed to send it.
	 */
	public static function schedulePingback() {
		return;
		DeferredUpdates::addCallableUpdate( function () {
			$instance = new Pingback;
			if ( $instance->shouldSend() ) {
				$instance->sendPingback();
			}
		} );
	}
Krinkle assigned this task to tstarling.

Presumed resolved through the changes on the other tasks.