Change #1164457 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/dns@master] wmnet: add discovery records for thumbor

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

Change #1164458 had a related patch set uploaded (by Hnowlan; author: Hnowlan):

[operations/puppet@production] service: add discovery active/active config

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

@JMeybohm @Clement_Goubert this sounds like something we may need to do before next Kubernetes upgrade (or at least surfacing it so you know about that special case).

In case there's capacity to do it this quarter, please move it to Scheduled

Actually @hnowlan I see CR submitted, did you complete that?

Tagging Data-Persistence so we can get opinions on if/how we actually *can* make swift fail over to another thumbor when we need to or not.

@MatthewVernon can you take a look at this when you get a chance?

Swift calls out to thumbor via our own rewrite middleware. As the ticket notes, puppet knows about both (profile::swift::proxy::thumborhost and profile::swift::proxy::inactivedc_thumborhost). The variables are rather misnamed, since they in fact refer to the local and remote thumbor host respectively.

Further, since work done on T313102, we don't actually use inactivedc_thumborhost anywhere (so it should probably be cleaned out of puppet in our CFT).

So thumborhost is used in thumborify_url which (inter alia) rewrites the incoming request for a thumbnail that would otherwise 404 to replace the host in the request with thumbor.svc.codfw.wmnet:8800 (WLOG as to DC).

So I don't see any particular reason why that DC-specific hostname couldn't simply be replaced with a global discovery hostname (assuming that it'd still be reachable at port 8800). Obviously, that would require said discovery hostname to actually exist (I think the CR to make it hasn't been approved nor merged).

Change #1238729 had a related patch set uploaded (by MVernon; author: MVernon):

[operations/puppet@production] wip: swift: use thumbor.discovery, remove unused inactivedc setting

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

Thanks Matthew. @JMeybohm @Clement_Goubert is that something we should find someone to do this quarter, or shall we move it to Backlog?

In T397618#11607310, @MatthewVernon wrote:

Swift calls out to thumbor via our own rewrite middleware. As the ticket notes, puppet knows about both (profile::swift::proxy::thumborhost and profile::swift::proxy::inactivedc_thumborhost). The variables are rather misnamed, since they in fact refer to the local and remote thumbor host respectively.

Further, since work done on T313102, we don't actually use inactivedc_thumborhost anywhere (so it should probably be cleaned out of puppet in our CFT).

So thumborhost is used in thumborify_url which (inter alia) rewrites the incoming request for a thumbnail that would otherwise 404 to replace the host in the request with thumbor.svc.codfw.wmnet:8800 (WLOG as to DC).

So I don't see any particular reason why that DC-specific hostname couldn't simply be replaced with a global discovery hostname (assuming that it'd still be reachable at port 8800). Obviously, that would require said discovery hostname to actually exist (I think the CR to make it hasn't been approved nor merged).

I'm still unsure of the actual flow of request here. My mental model for current operations is:

request -> swift 404 (middleware) -> dc-local thumbor -> writes thumb to dc-local swift -> returns URL to swift -> response

If we switch to a global active/active discovery hostname, and one of the DCs is depooled for thumbor, does it mean swift will call out to dc-remote thumbor, and the thumb will be written to the "wrong" swift?

If that's the case, we need a way to enforce that the pooled status is the same for any DC swift/thumbor pair.

In T397618#11611792, @MLechvien-WMF wrote:

Thanks Matthew. @JMeybohm @Clement_Goubert is that something we should find someone to do this quarter, or shall we move it to Backlog?

In my opinion we have two options:

• Do it ASAP so we can chase out the gremlins before the DC switchover (March 24th)
• Put it in backlog and do it after the DC switchover single-DC period

The amount of work assuming everything goes right isn't much, basically merging the three CRs attached to this task and making sure nothing blows up, then updating the switchover cookbooks/procedures to ensure we're doing the right thing.

In T397618#11622476, @Clement_Goubert wrote:

I'm still unsure of the actual flow of request here. My mental model for current operations is:

request -> swift 404 (middleware) -> dc-local thumbor -> writes thumb to dc-local swift -> returns URL to swift -> response

Not quite. Our middleware sends the request to thumbor, which returns an image (or error), which the middleware passes on to the client. Separately, thumbor sends a PUT to dc-local swift to "cache" the generated thumb in swift.

So I think that if you had a cross-DC swift/thumbor pair then the thumb would be returned to the client OK, but written to the wrong swift cluster.

In T397618#11623253, @MatthewVernon wrote:

In T397618#11622476, @Clement_Goubert wrote:

I'm still unsure of the actual flow of request here. My mental model for current operations is:

request -> swift 404 (middleware) -> dc-local thumbor -> writes thumb to dc-local swift -> returns URL to swift -> response

Not quite. Our middleware sends the request to thumbor, which returns an image (or error), which the middleware passes on to the client. Separately, thumbor sends a PUT to dc-local swift to "cache" the generated thumb in swift.

So I think that if you had a cross-DC swift/thumbor pair then the thumb would be returned to the client OK, but written to the wrong swift cluster.

And only originals are replicated, right?

In T397618#11623277, @Clement_Goubert wrote:

And only originals are replicated, right?

Yes (but NB that is done by MediaWiki not Swift).

In T397618#11623282, @MatthewVernon wrote:

In T397618#11623277, @Clement_Goubert wrote:

And only originals are replicated, right?

Yes (but NB that is done by MediaWiki not Swift).

So if we were to put this into place, and we had a cross-DC swift/thumbor pair, we would basically end up never caching any thumb. That seems bad :D

So we do need to either enforce same-pooled-status, or a way for thumbor to know the origin of the request and write back there instead of swift-local. I think the former is easier, the latter would require both code changes to thumbor, and ACL changes to swift.

Yes, it wouldn't be good for anything other than short periods of time.

The two swift clusters have separate credentials, so for cross-DC-writes to work you'd need thumbor to be able to pick the right credentials for the swift cluster it was aiming for (with the correct credentials, there's nothing currently stopping codfw-thumbor from writing thumbs to eqiad-swift); but you're likely right that same-pooled status is easier...

@JTweed-WMF would you have inputs on how to triage this task?

Removing from the Media Platform team board until we've decided the approach to thumbnailing. Once we've done that, we'll review existing Thumbor issues and determine if they are still relevant.