From 94e6cab002418d0e6f87d0b39bea5370a2d35072 Mon Sep 17 00:00:00 2001 From: Menno Pruijssers Date: Wed, 3 Aug 2016 13:17:12 +0200 Subject: [PATCH] Add support local labels to the membership This commit adds support for labels to the local membership. To make testing cleaner/easier, I extended the `testRingpop`-function to support subtests and testing on a ringpop that isn't ready yet. I also had to add a private function - `_newIncarnationNumber` - to `Membership` to generate a new incarnation number. Otherwise the tests were flaky because of the issue descibred in #298. Note: labels are not yet gossiped. I will add that in a separate PR. --- lib/membership/index.js | 93 ++++++++++++- test/lib/test-ringpop.js | 40 +++++- test/unit/membership-labels-test.js | 199 ++++++++++++++++++++++++++++ 3 files changed, 323 insertions(+), 9 deletions(-) create mode 100644 test/unit/membership-labels-test.js diff --git a/lib/membership/index.js b/lib/membership/index.js index 3de715d3..b85a0cde 100644 --- a/lib/membership/index.js +++ b/lib/membership/index.js @@ -42,6 +42,9 @@ function Membership(opts) { this.stashedUpdates = []; this.decayTimer = null; this.localMember = null; + this._localLabels = {}; // local labels so it's possible to set labels before bootstrap + + this._newIncarnationNumber = Date.now; // used in tests to mock incarnation bumping, } util.inherits(Membership, EventEmitter); @@ -196,6 +199,92 @@ Membership.prototype.isPingable = function isPingable(member) { Member.isStatusPingable(member.status); }; +/** + * Get the value associated with a key. + * + * @param {string} key the key to retrieve. + * @return {string|null} return the value associated with the given key. + */ +Membership.prototype.getLocalLabel = function getLocalLabel(key) { + return this._localLabels[key] || null; +}; + +/** + * Get the labels for the local member. + * @return {Object} A hash of the labels. + */ +Membership.prototype.getLocalLabels = function getLocalLabels() { + return _.clone(this._localLabels); +}; + +/** + * Set a label on the local member + * @param {string} key the name of the label. + * @param {string} value the value. + * + * @return {boolean} True if the label is set, false otherwise. + */ +Membership.prototype.setLocalLabel = function addLocalLabel(key, value) { + if (_.has(this._localLabels, key) && this._localLabels[key] === value) { + return false; + } + + this._localLabels[key] = value; + if (this.localMember) { + this._postLocalUpdate(this._reincarnate()); + } + + return true; +}; + +/** + * Set labels on the local member. Labels that are currently present won't be be + * removed but will be overwritten by the values in `labels`. + * + * @param {Object} labels The labels to set. + * + * @return {boolean} True if one or more labels are applied; false otherwise. + */ +Membership.prototype.setLocalLabels = function addLocalLabels(labels) { + var self = this; + var changed = false; + _.each(labels, function eachLabel(value, key) { + if(!self._localLabels[key] || self._localLabels[key] !== value) { + changed = true; + + self._localLabels[key] = value; + } + }); + if (changed && this.localMember) { + this._postLocalUpdate(this._reincarnate()); + } + return changed; +}; + +/** + * Remove one of more labels. + * + * @param {string[] | string} keys a single key or an array of keys to remove. + * + * @returns {boolean} True if one or more labels are removed; false otherwise, + */ +Membership.prototype.removeLocalLabels = function removeLocalLabels(keys) { + if (!Array.isArray(keys)) { + keys = [keys]; + } + var self = this; + var changed = false; + _.each(keys, function eachKey(key) { + changed = changed || _.has(self._localLabels, key); + delete self._localLabels[key]; + }); + + if (changed && this.localMember) { + this._postLocalUpdate(this._reincarnate()); + } + return changed; +}; + /** * Change the status of the local member to alive * @see Membership#setLocalStatus @@ -213,7 +302,7 @@ Membership.prototype.makeLocalAlive = function makeLocalAlive(){ * @private */ Membership.prototype._reincarnate = function _reincarnate() { - this.localMember.incarnationNumber = Date.now(); + this.localMember.incarnationNumber = this._newIncarnationNumber(); return new Update(this.localMember, this.localMember); }; @@ -235,7 +324,7 @@ Membership.prototype.setLocalStatus = function setLocalStatus(status) { } else { this.localMember = new Member(this.ringpop, new Update({ address: this.ringpop.whoami(), - incarnationNumber: Date.now(), + incarnationNumber: this._newIncarnationNumber(), status: status })); this.members.push(this.localMember); diff --git a/test/lib/test-ringpop.js b/test/lib/test-ringpop.js index 267cb87b..e9bb84b8 100644 --- a/test/lib/test-ringpop.js +++ b/test/lib/test-ringpop.js @@ -23,22 +23,48 @@ var Ringpop = require('../../index.js'); var tape = require('tape'); -function testRingpop(opts, name, test) { +/** + * @callback testRingpopCallback + * @param {object} deps top-level dependencies made available for convenience. + * @param {tape.Test} assert The assert + * @param {testRingpopCleanupCallback} [cleanup] The callback to call when done. Only available on async tests. + */ + +/** + * @callback testRingpopCleanupCallback + */ + +/** + * A util function to test ringpop. + * @param {object} opts the options + * @param {string} [opts.app=test] The app-name passed into ringpop. + * @param {boolean} [opts.async] start a async test. An async test will not clean-up itself automatically. The callback should call the clean-up argument when done. + * @param {string} [opts.hostPort=127.0.01:3000] the hostPort passed into ringpop + * @param {boolean} [opts.makeAlive=true] configure if ringpop should be made alive and ready or not. + * @param {tape.Test} [opts.test] when used as a sub-test, pass in the parent test. When not given, create a new root-level test. + * @param {string} name the name of the (sub) test + * @param cb + */ +function testRingpop(opts, name, cb) { if (typeof opts === 'string' && typeof name === 'function') { - test = name; + cb = name; name = opts; opts = {}; } - tape(name, function onTest(assert) { + var test = opts.test || tape; + test(name, function onTest(assert) { var ringpop = new Ringpop({ app: opts.app || 'test', hostPort: opts.hostPort || '127.0.0.1:3000' }); - ringpop.isReady = true; + // default to true when not defined + if (opts.makeAlive !== false) { + ringpop.isReady = true; - ringpop.membership.makeLocalAlive(); + ringpop.membership.makeLocalAlive(); + } // These are made top-level dependencies as a mere // convenience to users of the test suite. @@ -58,9 +84,9 @@ function testRingpop(opts, name, test) { }; if (opts.async) { - test(deps, assert, cleanup); + cb(deps, assert, cleanup); } else { - test(deps, assert); + cb(deps, assert); cleanup(); } diff --git a/test/unit/membership-labels-test.js b/test/unit/membership-labels-test.js new file mode 100644 index 00000000..2e3fefc7 --- /dev/null +++ b/test/unit/membership-labels-test.js @@ -0,0 +1,199 @@ +// Copyright (c) 2016 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +'use strict'; + +var test = require('tape'); +var testRingpop = require('../lib/test-ringpop.js'); + + +function labelTests(subTest, shouldValidateIncarnationBump) { + + /** + * Create a validator for the incarnation number. + * @param membership the membership + * @param assert the assert of the current test harness + * @return {newIncarnationAssert~incarnationAssert} + */ + function newIncarnationAssert(membership, assert) { + var preIncarnationNumber = shouldValidateIncarnationBump && membership.localMember.incarnationNumber; + + /** + * Assert the incarnation number is bumped or unchanged + * @param {boolean} shouldBeBumped a boolean indicating if the incarnation number should be bumped (true) or unchanged (false) + * @param {string} [msg] an optional message for the assertion. + */ + function incarnationAssert(shouldBeBumped, msg) { + if (!shouldValidateIncarnationBump) { + assert.skip(msg); + return; + } + + var postIncarnationNumber = membership.localMember.incarnationNumber; + + if (shouldBeBumped) { + assert.ok(postIncarnationNumber > preIncarnationNumber, msg || 'incarnation is bumped', { + pre: preIncarnationNumber, + post: postIncarnationNumber + }); + } else { + assert.equals(postIncarnationNumber, preIncarnationNumber, msg || 'incarnation is not bumped'); + } + } + + return incarnationAssert; + } + + + subTest('get/set local label', function(membership, assert) { + var assertBumped = newIncarnationAssert(membership, assert); + + var result = membership.setLocalLabel('hello', 'world'); + assertBumped(true); + + assert.equal(result, true, 'set local label returns true'); + + assertBumped = newIncarnationAssert(membership, assert); + assert.equal(membership.getLocalLabel('hello'), 'world', 'getLocalLabel returns the value'); + assertBumped(false, 'incarnation number does not change on getLocalLabel'); + }); + + subTest('set existing label with different value', function(membership, assert) { + membership.setLocalLabel('key', 'value'); + + var assertBumped = newIncarnationAssert(membership, assert); + var result = membership.setLocalLabel('key', 'new'); + assertBumped(true, 'overwriting a label with a different value bumps incarnation number'); + + assert.equal(result, true, 'overwrite returns true'); + assert.equal(membership.getLocalLabel('key'), 'new', 'label contains new value'); + }); + + subTest('set existing label with same value', function(membership, assert) { + membership.setLocalLabel('key', 'value'); + + var assertBumped = newIncarnationAssert(membership, assert); + var result = membership.setLocalLabel('key', 'value'); + assertBumped(false, 'setLocalLabel with same value should not bump incarnation number'); + assert.equal(result, false, 'overwrite returns false'); + assert.equal(membership.getLocalLabel('key'), 'value', 'label contains old value'); + }); + + subTest('get local labels', function(membership, assert) { + membership.setLocalLabel('hello', 'world'); + + var assertBumped = newIncarnationAssert(membership, assert); + var localLabels = membership.getLocalLabels(); + assertBumped(false, 'getLocalLabels does not bump incarnation number'); + + assert.notEqual(localLabels, membership._localLabels, 'getLocalLabels returns a clone copy'); + assert.deepEqual(localLabels, {'hello': 'world'}, 'getLocalLabels returns the hash'); + + localLabels['hello'] = 'world2'; + assert.equal(membership.getLocalLabel('hello'), 'world', 'value in local label didn\'t change'); + }); + + subTest('remove single label', function(membership, assert) { + membership.setLocalLabels({'key1': 'value1', 'key2': 'value2'}); + + var assertBumped = newIncarnationAssert(membership, assert); + var result = membership.removeLocalLabels('key1'); + + assertBumped(true, 'remove an existing label bumps incarnation'); + assert.equal(result, true, 'remove existing label returns true'); + assert.deepEqual(membership.getLocalLabels(), {'key2': 'value2'}); + + assertBumped = newIncarnationAssert(membership, assert); + result = membership.removeLocalLabels('hello'); + + assertBumped(false, 'remove non-existing label does not bump incarnation number'); + assert.equal(result, false, 'remove non-existing label returns false'); + assert.deepEqual(membership.getLocalLabels(), {'key2': 'value2'}); + }); + + subTest('remove multiple labels', function(membership, assert) { + var fixture = {'key1': 'value1', 'key2': 'value2', 'key3': 'value3'}; + membership.setLocalLabels(fixture); + + var assertBumped = newIncarnationAssert(membership, assert); + var result = membership.removeLocalLabels(['key1', 'key2']); + + assertBumped(true, 'remove existing labels bumps incarnation number'); + assert.equal(result, true, 'remove existing labels returns true'); + assert.deepEqual(membership.getLocalLabels(), {'key3': 'value3'}); + + assertBumped = newIncarnationAssert(membership, assert); + result = membership.removeLocalLabels(['hello', 'hi']); + + assertBumped(false, 'remove non-existing labels does not bump incarnation number'); + assert.equal(result, false, 'remove non-existing labels returns false'); + assert.deepEqual(membership.getLocalLabels(), {'key3': 'value3'}); + + // reset + membership.setLocalLabels(fixture); + + var assertBumped = newIncarnationAssert(membership, assert); + result = membership.removeLocalLabels(['key1', 'hello']); + + assertBumped(true, 'remove an existing and non-existing label bumps incarnation number'); + assert.equal(result, true, 'remove an existing and non-existing key returns true.'); + assert.deepEqual(membership.getLocalLabels(), { + 'key2': 'value2', + 'key3': 'value3' + }); + }); +} + +test('labels before bootstrap', function t(t) { + function subTest(name, callback) { + testRingpop({ + makeAlive: false, + test: t.test + }, name, function(deps, assert) { + var membership = deps.membership; + + callback(membership, assert); + }); + } + + labelTests(subTest, false); + + t.end(); +}); + +test('labels after bootstrap', function t(t) { + function subTest(name, callback) { + testRingpop({test: t.test}, name, function(deps, assert) { + var membership = deps.membership; + + // force incarnation bump + var incarnationNumber = membership.localMember.incarnationNumber; + membership._newIncarnationNumber = function() { + return ++incarnationNumber; + }; + + callback(membership, assert); + }); + } + + labelTests(subTest, true); + + t.end(); +});