Page MenuHomePhabricator

Allow async foreign set/delete WAN cache operations in mcrouter
Closed, ResolvedPublic0 Story Points

Description

This task is meant to track the efforts to deploy https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/492948/

Mcrouter currently blocks when set/deletes are replicated to codfw until it receives a confirmation from both dcs (memcached in eqiad, mcrouter proxy in codfw). This causes extra latencies that could be avoided, as explained in the correspondence of the above code change.

The plan would be to roll out the change for some mw1* hosts (rather than all of them) to observe/test its behavior, and slowly roll it out everywhere.

Event Timeline

elukey triaged this task as Normal priority.Jun 12 2019, 4:31 PM
elukey created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 12 2019, 4:31 PM
Krinkle moved this task from Backlog to Next-up on the Availability (MediaWiki-MultiDC) board.
Krinkle moved this task from Inbox to Radar on the Performance-Team board.
Krinkle edited projects, added Performance-Team (Radar); removed Performance-Team.
Krinkle moved this task from Limbo to Watching on the Performance-Team (Radar) board.

whenever you have time let me know if you like the plan and the new change :)

Gilles moved this task from Inbox to Radar on the Performance-Team board.Jun 17 2019, 8:13 PM
Gilles edited projects, added Performance-Team (Radar); removed Performance-Team.

Change 520726 had a related patch set uploaded (by Elukey; owner: Elukey):
[operations/puppet@production] Enable mcrouter async replication to codfw on mw1261 and mw1276

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

This is what the new config should look like (from the puppet compiler):

{
  "pools": {
    "codfw": {
      "servers": [
        "10.192.0.61:11214:ascii:ssl",
        "10.192.16.54:11214:ascii:ssl",
        "10.192.32.51:11214:ascii:ssl",
        "10.192.48.93:11214:ascii:ssl"
      ]
    },
    "eqiad": {
      "servers": [
        "10.64.0.80:11211:ascii:plain",
        "10.64.0.81:11211:ascii:plain",
        "10.64.0.82:11211:ascii:plain",
        "10.64.0.83:11211:ascii:plain",
        "10.64.0.84:11211:ascii:plain",
        "10.64.16.107:11211:ascii:plain",
        "10.64.16.108:11211:ascii:plain",
        "10.64.16.109:11211:ascii:plain",
        "10.64.16.110:11211:ascii:plain",
        "10.64.32.208:11211:ascii:plain",
        "10.64.32.209:11211:ascii:plain",
        "10.64.32.210:11211:ascii:plain",
        "10.64.32.211:11211:ascii:plain",
        "10.64.32.212:11211:ascii:plain",
        "10.64.48.155:11211:ascii:plain",
        "10.64.48.156:11211:ascii:plain",
        "10.64.48.157:11211:ascii:plain",
        "10.64.48.158:11211:ascii:plain"
      ]
    }
  },
  "routes": [
    {
      "aliases": [
        "\/eqiad\/mw\/"
      ],
      "route": "PoolRoute|eqiad"
    },
    {
      "aliases": [
        "\/codfw\/mw\/"
      ],
      "route": "PoolRoute|codfw"
    },
    {
      "aliases": [
        "\/eqiad\/mw-wan\/"
      ],
      "route": {
        "default_policy": "PoolRoute|eqiad",
        "operation_policies": {
          "delete": {
            "children": [
              "PoolRoute|eqiad"
            ],
            "type": "AllSyncRoute"
          },
          "set": {
            "children": [
              "PoolRoute|eqiad"
            ],
            "type": "AllSyncRoute"
          }
        },
        "type": "OperationSelectorRoute"
      }
    },
    {
      "aliases": [
        "\/codfw\/mw-wan\/"
      ],
      "route": {
        "default_policy": "PoolRoute|eqiad",
        "operation_policies": {
          "delete": {
            "children": [
              "PoolRoute|codfw"
            ],
            "type": "AllAsyncRoute"
          },
          "set": {
            "children": [
              "PoolRoute|codfw"
            ],
            "type": "AllAsyncRoute"
          }
        },
        "type": "OperationSelectorRoute"
      }
    }
  ]
}

Current config is:

{
  "pools": {
    "codfw": {
      "servers": [
        "10.192.0.61:11214:ascii:ssl",
        "10.192.16.54:11214:ascii:ssl",
        "10.192.32.51:11214:ascii:ssl",
        "10.192.48.93:11214:ascii:ssl"
      ]
    },
    "eqiad": {
      "servers": [
        "10.64.0.80:11211:ascii:plain",
        "10.64.0.81:11211:ascii:plain",
        "10.64.0.82:11211:ascii:plain",
        "10.64.0.83:11211:ascii:plain",
        "10.64.0.84:11211:ascii:plain",
        "10.64.16.107:11211:ascii:plain",
        "10.64.16.108:11211:ascii:plain",
        "10.64.16.109:11211:ascii:plain",
        "10.64.16.110:11211:ascii:plain",
        "10.64.32.208:11211:ascii:plain",
        "10.64.32.209:11211:ascii:plain",
        "10.64.32.210:11211:ascii:plain",
        "10.64.32.211:11211:ascii:plain",
        "10.64.32.212:11211:ascii:plain",
        "10.64.48.155:11211:ascii:plain",
        "10.64.48.156:11211:ascii:plain",
        "10.64.48.157:11211:ascii:plain",
        "10.64.48.158:11211:ascii:plain"
      ]
    }
  },
  "routes": [
    {
      "aliases": [
        "\/eqiad\/mw\/"
      ],
      "route": "PoolRoute|eqiad"
    },
    {
      "aliases": [
        "\/codfw\/mw\/"
      ],
      "route": "PoolRoute|codfw"
    },
    {
      "aliases": [
        "\/eqiad\/mw-wan\/"
      ],
      "route": {
        "default_policy": "PoolRoute|eqiad",
        "operation_policies": {
          "delete": "PoolRoute|eqiad",
          "set": "PoolRoute|eqiad"
        },
        "type": "OperationSelectorRoute"
      }
    },
    {
      "aliases": [
        "\/codfw\/mw-wan\/"
      ],
      "route": {
        "default_policy": "PoolRoute|eqiad",
        "operation_policies": {
          "delete": "PoolRoute|codfw",
          "set": "PoolRoute|codfw"
        },
        "type": "OperationSelectorRoute"
      }
    }
  ]
}

@aaron sorry for the delay in working on this! Before starting, I'd like to make a plan about how to test this new config on some nodes.

In https://gerrit.wikimedia.org/r/520726 I chose two hosts, mw1261 (appserver canary) and mw1276 (api-appserver canary). The idea is to do the following:

  1. depool the hosts from live traffic
  2. merge the puppet change and run puppet on the hosts (mcrouter should detect the change in config via inotify and reload itself).
  3. run some tests in localhost to make sure that mcrouter behaves as expected
  4. repool the hosts and observe metrics (specifically, Grafana and Kibana basically)

For point 4), can you give me a list of keys that you know to check? I'd also plan to make a test like:

echo -e "set /eqiad/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
echo -e "get /eqiad/mw-wan/elukey-test" | nc -q 2 localhost 11213

echo -e "set /codfw/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213

echo -e "get __mcrouter__.route(get,/eqiad/mw-wan/elukey-test)" | nc -q 2 localhost 11213
echo -e "get __mcrouter__.route(set,/eqiad/mw-wan/elukey-test)" | nc -q 2 localhost 11213

echo -e "get __mcrouter__.route(get,/codfw/mw-wan/elukey-test)" | nc -q 2 localhost 11213
echo -e "get __mcrouter__.route(set,/codfw/mw-wan/elukey-test)" | nc -q 2 localhost 11213

And also I'd expect to get /codfw/mw-wan/elukey-test from a codfw mw host.

Anything else that you can think of? I'd like to avoid to repool a host and then figure out that the config is somehow broken only from errors logged in logstash..

elukey moved this task from Backlog to Mcrouter/Memcached on the User-Elukey board.Jul 6 2019, 1:51 PM
Krinkle removed a subscriber: Krinkle.Jul 9 2019, 8:17 PM
aaron added a comment.Jul 12 2019, 5:35 PM

For generic key testing, there is always:

php maintenance/mctest.php --cache memcached-pecl -i 1000

I have patches adding on to that script, but it is already there atm.

Some interesting and popular keys that should be on popular wikis can be generated via mwscript shell.php --wiki=enwiki:

use MediaWiki\MediaWikiServices;
$cache = MediaWikiServices::getInstance()->getMainWANObjectCache();
$cache->makeKey( 'ip-autoblock', 'whitelist' );
$cache->makeKey( 'active-tags' );
$cache->makeKey( 'valid-tags-db' );
$cache->makeKey( 'valid-tags-hook' );
$cache->makeKey( 'tags-usage-statistics' );
$cache->makeKey( 'sidebar', 'en' );

E.g.:

=> "enwiki:ip-autoblock:whitelist"
=> "enwiki:active-tags"
=> "enwiki:valid-tags-db"
=> "enwiki:valid-tags-hook"
=> "enwiki:tags-usage-statistics"
=> "enwiki:sidebar:en"

Change 520726 merged by Elukey:
[operations/puppet@production] Enable mcrouter async replication to codfw on mw1261 and mw1276

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

elukey added a comment.EditedJul 16 2019, 8:01 AM

This is happening on mw1261 (currently depooled):

elukey@mw1261:~$ echo -e "set /eqiad/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
STORED

elukey@mw1261:~$ echo -e "get /eqiad/mw-wan/elukey-test" | nc -q 2 localhost 11213
VALUE /eqiad/mw-wan/elukey-test 0 11
hello world
END

elukey@mw1261:~$ echo -e "set /codfw/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
NOT_STORED

elukey@mw1261:~$ echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
END

elukey@mw2235:~$ echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
VALUE /codfw/mw-wan/elukey-test 0 11
hello world
END

I am not sure if I am not getting how the replication should work, but when using the /codfw/mw-wan prefix on mw1261 the sets seems to go only in codfw, and the subsequent gets in eqiad come up empty (but the ones in codfw succeed). This is not what happens on mw1262:

elukey@mw1262:~$ echo -e "set /eqiad/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
STORED

elukey@mw1262:~$ echo -e "get /eqiad/mw-wan/elukey-test" | nc -q 2 localhost 11213
VALUE /eqiad/mw-wan/elukey-test 0 11
hello world
END

elukey@mw1262:~$ echo -e "set /codfw/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
STORED

elukey@mw1262:~$ echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
VALUE /codfw/mw-wan/elukey-test 0 11
hello world
END

The doc for https://github.com/facebook/mcrouter/wiki/List-of-Route-Handles#allasyncroute says:

Immediately sends the same request to all child route handles. Does not wait for response. Returns the default reply for the request right away as returned by NullRoute.

But in our config we don't specify more than one child route handle:

elukey@mw1261:~$ cat /etc/mcrouter/config.json | jq '.'
{
  "pools": {
    "codfw": {
      "servers": [
        "10.192.0.61:11214:ascii:ssl",
        "10.192.16.54:11214:ascii:ssl",
        "10.192.32.51:11214:ascii:ssl",
        "10.192.48.93:11214:ascii:ssl"
      ]
    },
    "eqiad": {
      "servers": [
        "10.64.0.80:11211:ascii:plain",
        "10.64.0.81:11211:ascii:plain",
        "10.64.0.82:11211:ascii:plain",
        "10.64.0.83:11211:ascii:plain",
        "10.64.0.84:11211:ascii:plain",
        "10.64.16.107:11211:ascii:plain",
        "10.64.16.108:11211:ascii:plain",
        "10.64.16.109:11211:ascii:plain",
        "10.64.16.110:11211:ascii:plain",
        "10.64.32.208:11211:ascii:plain",
        "10.64.32.209:11211:ascii:plain",
        "10.64.32.210:11211:ascii:plain",
        "10.64.32.211:11211:ascii:plain",
        "10.64.32.212:11211:ascii:plain",
        "10.64.48.155:11211:ascii:plain",
        "10.64.48.156:11211:ascii:plain",
        "10.64.48.157:11211:ascii:plain",
        "10.64.48.158:11211:ascii:plain"
      ]
    }
  },
  "routes": [
    {
      "aliases": [
        "/eqiad/mw/"
      ],
      "route": "PoolRoute|eqiad"
    },
    {
      "aliases": [
        "/codfw/mw/"
      ],
      "route": "PoolRoute|codfw"
    },
    {
      "aliases": [
        "/eqiad/mw-wan/"
      ],
      "route": {
        "default_policy": "PoolRoute|eqiad",
        "operation_policies": {
          "delete": {
            "children": [
              "PoolRoute|eqiad"
            ],
            "type": "AllSyncRoute"
          },
          "set": {
            "children": [
              "PoolRoute|eqiad"
            ],
            "type": "AllSyncRoute"
          }
        },
        "type": "OperationSelectorRoute"
      }
    },
    {
      "aliases": [
        "/codfw/mw-wan/"
      ],
      "route": {
        "default_policy": "PoolRoute|eqiad",
        "operation_policies": {
          "delete": {
            "children": [
              "PoolRoute|codfw"
            ],
            "type": "AllAsyncRoute"
          },
          "set": {
            "children": [
              "PoolRoute|codfw"
            ],
            "type": "AllAsyncRoute"
          }
        },
        "type": "OperationSelectorRoute"
      }
    }
  ]
}

Based on this discrepancy, I am going to rollback the setting for the two initial nodes until I have a chat with @aaron.

elukey added a comment.EditedJul 16 2019, 8:57 AM

All right this is a clear PEBCAK (problem exists between computer and keyboard). First of all my tests were not correct:

elukey@mw1262:~$ echo -e "set /eqiad/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
STORED

elukey@mw1262:~$ echo -e "get /eqiad/mw-wan/elukey-test" | nc -q 2 localhost 11213
VALUE /eqiad/mw-wan/elukey-test 0 11
hello world
END

elukey@mw1262:~$ echo -e "set /codfw/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
STORED

elukey@mw1262:~$ echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
VALUE /codfw/mw-wan/elukey-test 0 11
hello world
END

What happens behind the scenes is that:

  1. elukey-test (without the prefix of course) is stored in memcached eqiad
  2. elukey-test is correctly retrieved
  3. elukey-test is stored in memcached codfw only
  4. elukey-test is retrieved, but the value is the previous one set in 1)

What we really want when setting a value with /codfw/mw-wan is only to set it in codfw, not also in eqiad. For some reason I assumed that our mcrouter config was taking care of replicating to both DCs transparently, but I guess that this is handled by MediaWiki instead.

To verify what I wrote (and this is what confused me):

elukey@mw1261:~$ echo -e "set /codfw/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
STORED
elukey@mw1261:~$  echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
END

The above is obtained with the current config (I had already rolled back), and it is the same on mw1262 too. But then:

elukey@mw2235:~$ echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
VALUE /codfw/mw-wan/elukey-test 0 11
hello world
END

Last but not the least, the weird NOT_STORED result. I completely forgot that this is expected:

Immediately sends the same request to all child route handles. Does not wait for response. Returns the default reply for the request right away as returned by NullRoute.

From https://gerrit.wikimedia.org/r/#/c/operations/puppet/+/492948/ Aaron wrote:

If the "worst" reply of all routes used in the SET broadcast is NOT_STORED (e.g. due to NullRoute) and that is given back to either of the memcached BagOStuff PHP classes, it will not be treated as an error. This was done a while back for mcrouter async route use. Likewise for NOT_FOUND and DELETE.

In light of what I wrote above:

elukey@mw1261:~$  echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
END

elukey@mw1261:~$ echo -e "set /codfw/mw-wan/elukey-test 0 60 11\r\nhello world" | nc localhost 11213 -q 2
NOT_STORED

elukey@mw2235:~$ echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
VALUE /codfw/mw-wan/elukey-test 0 11
hello world
END

elukey@mw1261:~$  echo -e "get /codfw/mw-wan/elukey-test" | nc -q 2 localhost 11213
END

elukey@mw1261:~$  echo -e "get /eqiad/mw-wan/elukey-test" | nc -q 2 localhost 11213
END

elukey@mw1261:~$ echo -e "set /eqiad/mw-wan/elukey-test-2 0 60 11\r\nhello world" | nc localhost 11213 -q 2
STORED

elukey@mw1261:~$  echo -e "get /eqiad/mw-wan/elukey-test-2" | nc -q 2 localhost 11213
VALUE /eqiad/mw-wan/elukey-test-2 0 11
hello world
END

Mentioned in SAL (#wikimedia-operations) [2019-07-16T09:23:13Z] <elukey> pool mw1261 back with mcrouter async replication settings - T225642

Mentioned in SAL (#wikimedia-operations) [2019-07-16T09:24:56Z] <elukey> apply mcrouter async replication settings to mw1276 - T225642

aaron added a comment.Jul 16 2019, 6:19 PM

In terms of what MediaWiki actually queries, you have the following cases from both eqiad and codfw:

a) Local getWithSetCallback() requests for (regular) value keys:
"get WANCache:v:elukey-test"
"add WANCache:v:elukey-test"
"cas WANCache:v:elukey-test"

b) Local getWithSetCallback() requests for interim keys (when value key is tombstoned):
"get WANCache:i:elukey-test"
"add WANCache:i:elukey-test"
"cas WANCache:i:elukey-test"

c) Local getWithSetCallback() requests for time/"check" keys:
"get WANCache:t:elukey-test"
"add WANCache:t:elukey-test"

d) Local getWithSetCallback() requests for mutex keys:
"add WANCache:m:elukey-test"
"touch WANCache:m:elukey-test"

e) Broadcasted delete() requests for tombstoned value keys:
"set /mw-wan/*/WANCache:v:elukey-test"

f) Broadcasted touchCheckKey() requests for time/"check" keys:
"set /mw-wan/*/WANCache:t:elukey-test"

g) Broadcasted resetCheckKey() requests for time/"check" keys:
"delete /mw-wan/*/WANCache:t:elukey-test"

Methods are at https://github.com/wikimedia/mediawiki/blob/master/includes/libs/objectcache/WANObjectCache.php

@aaron it seems that the broadcast case is the only one really relevant for the test right? The WANCache keys without any mw-wan prefix should be ok, the related behavior for mcrouter didn't change afaics.

Let me know if it is ok to move forward to all the canaries, or if more tests are needed.

aaron added a comment.Jul 16 2019, 7:09 PM

Is there a codfw host with the patch applied?

Change 523855 had a related patch set uploaded (by Elukey; owner: Elukey):
[operations/puppet@production] Add mw2224 to the list of hosts with async replication in mcrouter

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

Change 523855 merged by Elukey:
[operations/puppet@production] Add mw2224 to the list of hosts with async replication in mcrouter

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

Mentioned in SAL (#wikimedia-operations) [2019-07-17T06:59:04Z] <elukey> apply mcrouter async replication to mw2224 - T225642

@aaron mw2224 ready for testing :)

One interesting thing is that the avg latency provided by mcrouter seems not changed when comparing mw1261 (that is async = true) and mw1262 (async = false):

https://grafana.wikimedia.org/d/000000549/mcrouter?orgId=1&var-source=eqiad%20prometheus%2Fops&var-cluster=All&var-instance=mw1261&var-memcached_server=All
https://grafana.wikimedia.org/d/000000549/mcrouter?orgId=1&var-source=eqiad%20prometheus%2Fops&var-cluster=All&var-instance=mw1261&var-memcached_server=All

The latency to hosts:11214 (codfw proxies) seems to be stable at around 50/60ms, so I guess that even if mcrouter returns immediately this is not reflected in the latency metrics.

aaron added a comment.Jul 18 2019, 6:49 PM

So, I've noticed that on mw1261/mw2224 as *well* as plain old mwmaint1002,mwmaint2001, that broadcasting keys doesn't seem to work, e.g.:

aaron@mwmaint2001:~$ echo -e "set /mw-wan/*/aaron 0 300 11\r\nhello world" | nc localhost 11213 -q 2
STORED
aaron@mwmaint2001:~$ echo -e "get aaron" | nc -q 2 localhost 11213
VALUE aaron 0 11
hello world
END

aaron@mwmaint1002:~$ echo -e "get aaron" | nc -q 2 localhost 11213
END
aaron@mwmaint1002:~$ echo -e "set /mw-wan/*/aaron 0 300 11\r\nhello world" | nc localhost 11213 -q 2
STORED
aaron@mwmaint1002:~$ echo -e "get aaron" | nc -q 2 localhost 11213
VALUE aaron 0 11
hello world
END

aaron@mwmaint2001:~$ echo -e "get aaron" | nc -q 2 localhost 11213
END
aaron added a comment.Jul 18 2019, 7:01 PM

Err, more PEBCAK . I put the * in the wrong spot...

aaron added a comment.Jul 18 2019, 7:07 PM

OK, replication for SET/DELETE seems fine on mw1261/mw2224 for me and the STORED/NOT_STORED and FOUND/NOT_FOUND replies are what I expect when using (no prefix, /otherdc/mw-wan, and /thisdc/mwwan).

All looks good to me.

Change 525053 had a related patch set uploaded (by Elukey; owner: Elukey):
[operations/puppet@production] Set async replication for mcrouter on mw api/appserv canaries

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

Change 525053 merged by Elukey:
[operations/puppet@production] Set async replication for mcrouter on mw api/appserv canaries

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

Change 525224 had a related patch set uploaded (by Elukey; owner: Elukey):
[operations/puppet@production] profile::mediawiki::mcrouter_wancache: set async behavior as default

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

Change 525224 merged by Elukey:
[operations/puppet@production] profile::mediawiki::mcrouter_wancache: set async behavior as default

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

Mentioned in SAL (#wikimedia-operations) [2019-07-29T08:47:02Z] <elukey> set mcrouter async behavior for codfw replication to all mw app/api servers (changes will be picked up when puppet runs on the hosts) - T225642

elukey closed this task as Resolved.Mon, Jul 29, 10:12 AM

@aaron changed deployed, closing the task but please re-open if anything is missing :)