Metaswitch/cpp-common — pull request #218 by rkday (2014-12-20 22:02:09+00:00)
Title: Remove the need for a relationship between servers and new_servers in MemcachedStoreView

Previously, servers and new_servers had to share a prefix, because MemcachedStoreView returned indexes into an array of memcached connections (so if your servers[0] didn't match new_servers[0], read attempts would go to a different memcached and fail).

This change makes MemcachedStoreView return the address of the relevant memcached server directly, rather than an index, allowing for new_servers to have any (or no) overlap with servers. This mostly just makes use of information it already has - it's not a very big change.

This substantially mitigates https://github.com/Metaswitch/sprout/issues/852 - it doesn't do read-repair, but it is now possible to do a "create new VMs" upgrade with no risk of an outage by adding new memcacheds to the end and then removing the old ones from the start.

I've tested by writing a script that creates a MemcachedStore and starts off with four servers, writes a series of keys in, starts scaling (in five different ways noted below), checks that the keys are still readable, writes a different series of keys, finishes scaling, and checks that the second series of keys are still readable.

It scales:
1) up, by adding new servers to the end of the list
2) down, by removing servers from the end of the list
3) up, by adding new servers to the start of the list
4) down, by removing servers from the start of the list
5) "across", by making the new_servers a different pair of servers (i.e. there's no overlap at all between the old and the new servers)

Against the master branch, that test script passes cases 1 and 2 and fails the rest (as expected). Against this branch, all five cases pass.

(That testing was live, in that it linked in the real MemcachedStore object and I had six memcached processes running on my machine for it to use. I haven't tested live in the sense of doing it within Sprout, but I think my test script replicates what would really happen on scale up/down and gives me enough confidence that it works.)

The test script is at https://github.com/rkday/sprout/blob/memstore_tester/sprout/memcachedstore_tester.cpp if you want to see it, but I'm not proposing to merge it - it requires some work to run (i.e. running six memcacheds locally) so will probably just get stale.

(We should probably have some UT of MemcachedStoreView - I'll add some to Sprout in a separate pull request.)