Skip to content

Commit

Permalink
Changes after review
Browse files Browse the repository at this point in the history
  • Loading branch information
@henokgetachew
henokgetachew committed Oct 29, 2024
1 parent e0c14bc commit 611dacb
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 66 deletions.
8 changes: 0 additions & 8 deletions bin/move-node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ const getStdin = () => {
};

const parseShardMapJsonInput = async () => {
if (process.env.SHARD_MAPPING) {
try {
return JSON.parse(process.env.SHARD_MAPPING);
} catch (err) {
console.warn('Failed to parse SHARD_MAPPING environment variable');
}
}

try {
const input = await getStdin();
if (input.trim()) {
Expand Down
20 changes: 3 additions & 17 deletions src/move-node.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ const replaceForSingleNode = async (toNode) => {
};

const replaceInCluster = async (nodeMap, shardMapJson) => {
const removedNodes = [];
if (!shardMapJson) {
throw new Error('Shard map JSON is required for multi-node migration');
}
Expand All @@ -35,25 +34,17 @@ const replaceInCluster = async (nodeMap, shardMapJson) => {
// For each shard range in the current distribution
console.log('Current distribution:', currentDistribution);
for (const [shardRange, dbNodes] of Object.entries(currentDistribution)) {
console.log('Shard range:', shardRange);
console.log('DB nodes:', dbNodes);
// dbNodes is an object mapping db names to node names
for (const [dbName, nodeName] of Object.entries(dbNodes)) {
console.log('DB name:', dbName);
console.log('Node name:', nodeName);
if (nodeName === oldNode) {
console.log(
`Moving shard ${shardRange} for db ${dbName} from ${oldNode} to ${newNode}`
);
const oldNodes = await moveShard.moveShard(shardRange, newNode, dbName);
removedNodes.push(...oldNodes);
await moveShard.moveShard(shardRange, newNode, dbName);
}
}
}
if (!removedNodes.includes(oldNode)) {
removedNodes.push(oldNode);
}
return [...new Set(removedNodes)];
return [oldNode];
};

const moveNode = async (nodeMap, shardMapJson) => {
Expand All @@ -68,12 +59,7 @@ const moveNode = async (nodeMap, shardMapJson) => {

const syncShards = async () => {
try {
const membership = await utils.getMembership();
const { all_nodes, cluster_nodes } = membership;

const clusterComplete =
all_nodes.length === cluster_nodes.length &&
all_nodes.every((node) => cluster_nodes.includes(node));
const clusterComplete = await utils.isClusterComplete();

if (clusterComplete) {
const allDbs = await utils.getDbs();
Expand Down
10 changes: 10 additions & 0 deletions src/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,15 @@ const getCouchUrl = async () => {
return couchUrl;
};

const isClusterComplete = async () => {
const membership = await getMembership();
const { all_nodes, cluster_nodes } = membership;

const clusterComplete = all_nodes.length === cluster_nodes.length &&
all_nodes.every((node) => cluster_nodes.includes(node));
return clusterComplete;
};

module.exports = {
request,
getUrl,
Expand All @@ -299,4 +308,5 @@ module.exports = {
getConfig,
getCouchUrl,
prepareCouchUrl,
isClusterComplete,
};
7 changes: 2 additions & 5 deletions test/e2e/multi-node.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,11 @@ docker compose -f ../docker-compose-test.yml run couch-migration check-couchdb-u
# change database metadata to match new node name
# Move nodes one by one using move-node.js oldNode:newNode

# Using env variables
docker compose -f ../docker-compose-test.yml run -e SHARD_MAPPING="$shard_mapping" couch-migration move-node '{"[email protected]":"[email protected]"}'
docker compose -f ../docker-compose-test.yml run couch-migration sh -c "echo '$shard_mapping' | move-node '{\"[email protected]\":\"[email protected]\"}'"

# Using stdin
docker compose -f ../docker-compose-test.yml run couch-migration sh -c "echo '$shard_mapping' | move-node '{\"[email protected]\":\"[email protected]\"}'"

# Using env variables
docker compose -f ../docker-compose-test.yml run -e SHARD_MAPPING="$shard_mapping" couch-migration move-node '{"[email protected]":"[email protected]"}'
docker compose -f ../docker-compose-test.yml run couch-migration sh -c "echo '$shard_mapping' | move-node '{\"[email protected]\":\"[email protected]\"}'"

docker compose -f ../docker-compose-test.yml run couch-migration verify

Expand Down
2 changes: 1 addition & 1 deletion test/e2e/scripts/compare-shard-mappings.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ function compareMappings(nodeMappingJson, oldMappingJson, newMappingJson) {
const newNode = newShardDbs[dbName];

// Map old node name to new node name using the nodeMapping
const expectedNewNode = nodeMapping[oldNode] || oldNode;
const expectedNewNode = nodeMapping[oldNode];

if (expectedNewNode !== newNode) {
console.error(
Expand Down
54 changes: 19 additions & 35 deletions test/unit/move-node.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,33 +175,27 @@ describe('move-node', () => {
});

it('should sync shards for all dbs', async () => {
sinon.stub(utils, 'getMembership').resolves({
all_nodes: ['node1', 'node2'],
cluster_nodes: ['node1', 'node2'],
});
sinon.stub(utils, 'isClusterComplete').resolves(true);
sinon.stub(utils, 'getDbs').resolves(['one', 'two', 'three']);
sinon.stub(utils, 'syncShards').resolves({ ok: true });

await moveNodeSpec.syncShards();

expect(utils.getMembership.callCount).to.equal(1);
expect(utils.isClusterComplete.callCount).to.equal(1);
expect(utils.getDbs.callCount).to.equal(1);
expect(utils.syncShards.callCount).to.equal(3);
expect(utils.syncShards.args).to.deep.equal([['one'], ['two'], ['three']]);
});

it('should sync shards when cluster is complete', async () => {
sinon.stub(utils, 'getMembership').resolves({
all_nodes: ['node1', 'node2'],
cluster_nodes: ['node1', 'node2'],
});
sinon.stub(utils, 'isClusterComplete').resolves(true);
sinon.stub(utils, 'getDbs').resolves(['db1', 'db2']);
sinon.stub(utils, 'syncShards').resolves({ ok: true });
const consoleLogStub = sinon.stub(console, 'log');

await moveNodeSpec.syncShards();

expect(utils.getMembership.callCount).to.equal(1);
expect(utils.isClusterComplete.callCount).to.equal(1);
expect(utils.getDbs.callCount).to.equal(1);
expect(utils.syncShards.callCount).to.equal(2);
expect(utils.syncShards.args).to.deep.equal([['db1'], ['db2']]);
Expand All @@ -211,17 +205,14 @@ describe('move-node', () => {
});

it('should skip syncing shards when cluster is incomplete', async () => {
sinon.stub(utils, 'getMembership').resolves({
all_nodes: ['node1', 'node2'],
cluster_nodes: ['node1'],
});
sinon.stub(utils, 'isClusterComplete').resolves(false);
const consoleLogStub = sinon.stub(console, 'log');
const getDbsStub = sinon.stub(utils, 'getDbs');
const syncShardsStub = sinon.stub(utils, 'syncShards');

await moveNodeSpec.syncShards();

expect(utils.getMembership.callCount).to.equal(1);
expect(utils.isClusterComplete.callCount).to.equal(1);
expect(getDbsStub.notCalled).to.be.true;
expect(syncShardsStub.notCalled).to.be.true;
expect(consoleLogStub.calledWith(
Expand All @@ -231,23 +222,22 @@ describe('move-node', () => {
consoleLogStub.restore();
});

it('should throw error if getMembership fails', async () => {
sinon.stub(utils, 'getMembership').rejects(new Error('membership error'));
await expect(moveNodeSpec.syncShards()).to.be.rejectedWith(Error, 'membership error');
it('should throw error if isClusterComplete fails', async () => {
const error = new Error('membership error');
sinon.stub(utils, 'isClusterComplete').rejects(error);

await expect(moveNodeSpec.syncShards()).to.be.rejectedWith('membership error');
});

it('should sync shards for all dbs when cluster has one node', async () => {
sinon.stub(utils, 'getMembership').resolves({
all_nodes: ['node1'],
cluster_nodes: ['node1'],
});
sinon.stub(utils, 'isClusterComplete').resolves(true);
sinon.stub(utils, 'getDbs').resolves(['db1']);
sinon.stub(utils, 'syncShards').resolves({ ok: true });
const consoleLogStub = sinon.stub(console, 'log');

await moveNodeSpec.syncShards();

expect(utils.getMembership.callCount).to.equal(1);
expect(utils.isClusterComplete.callCount).to.equal(1);
expect(utils.getDbs.callCount).to.equal(1);
expect(utils.syncShards.callCount).to.equal(1);
expect(utils.syncShards.args).to.deep.equal([['db1']]);
Expand All @@ -256,24 +246,18 @@ describe('move-node', () => {
consoleLogStub.restore();
});

it('should throw error if all dbs fails', async () => {
sinon.stub(utils, 'getMembership').resolves({
all_nodes: ['node1', 'node2'],
cluster_nodes: ['node1', 'node2'],
});
it('should throw error if getDbs fails', async () => {
sinon.stub(utils, 'isClusterComplete').resolves(true);
sinon.stub(utils, 'getDbs').rejects(new Error('omg'));

await expect(moveNodeSpec.syncShards()).to.be.rejectedWith(Error, 'omg');
await expect(moveNodeSpec.syncShards()).to.be.rejectedWith('omg');
});

it('should throw error if sync shards fails', async () => {
sinon.stub(utils, 'getMembership').resolves({
all_nodes: ['node1', 'node2'],
cluster_nodes: ['node1', 'node2'],
});
it('should throw error if syncShards fails', async () => {
sinon.stub(utils, 'isClusterComplete').resolves(true);
sinon.stub(utils, 'getDbs').resolves(['one', 'two', 'three']);
sinon.stub(utils, 'syncShards').rejects(new Error('oh noes'));
await expect(moveNodeSpec.syncShards()).to.be.rejectedWith(Error, 'oh noes');
await expect(moveNodeSpec.syncShards()).to.be.rejectedWith('oh noes');
});
});
});

0 comments on commit 611dacb

Please sign in to comment.