From 4e8e96b6c2514ed2219e6fcaee017dda5a8c9da6 Mon Sep 17 00:00:00 2001 From: jeniawhite Date: Wed, 4 Jan 2017 15:53:03 +0200 Subject: [PATCH] Fixing node mappings crashes (#2473) --- src/server/object_services/map_allocator.js | 7 ++++- src/server/object_services/map_builder.js | 4 ++- src/server/object_services/map_utils.js | 35 +++++++++++++++------ src/test/unit_tests/test_map_utils.js | 24 ++++++++++---- 4 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/server/object_services/map_allocator.js b/src/server/object_services/map_allocator.js index 3fb7646108..36a25cc35b 100644 --- a/src/server/object_services/map_allocator.js +++ b/src/server/object_services/map_allocator.js @@ -87,7 +87,12 @@ class MapAllocator { _.each(this.parts, part => { if (part.chunk_dedup) return; // already found dup let tiering_pools_status = node_allocator.get_tiering_pools_status(this.bucket.tiering); - let status = map_utils.get_chunk_status(part.chunk, this.bucket.tiering, /*async_mirror=*/ true, tiering_pools_status); + let status = map_utils.get_chunk_status( + part.chunk, + this.bucket.tiering, { + async_mirror: true, + tiering_pools_status: tiering_pools_status + }); var avoid_nodes = []; let allocated_hosts = []; diff --git a/src/server/object_services/map_builder.js b/src/server/object_services/map_builder.js index 36b245e737..d78c4c0718 100644 --- a/src/server/object_services/map_builder.js +++ b/src/server/object_services/map_builder.js @@ -84,7 +84,9 @@ class MapBuilder { let bucket = system_store.data.get_by_id(chunk.bucket); map_utils.set_chunk_frags_from_blocks(chunk, chunk.blocks); let tiering_pools_status = node_allocator.get_tiering_pools_status(bucket.tiering); - chunk.status = map_utils.get_chunk_status(chunk, bucket.tiering, /*async_mirror=*/ false, tiering_pools_status); + chunk.status = map_utils.get_chunk_status(chunk, bucket.tiering, { + tiering_pools_status: tiering_pools_status + }); // only delete blocks if the chunk is in good shape, // that is no allocations needed, and is accessible. if (chunk.status.accessible && diff --git a/src/server/object_services/map_utils.js b/src/server/object_services/map_utils.js index 180e098009..c37f7cc039 100644 --- a/src/server/object_services/map_utils.js +++ b/src/server/object_services/map_utils.js @@ -139,12 +139,18 @@ function _handle_under_policy_threshold(decision_params) { } spill_status.allocations = _.concat(spill_status.allocations, decision_params.mirror_status.cloud_pools); + } else if (_.get(decision_params, 'additional_params.status_check', false)) { + // This is used when we only check the status without any actions + spill_status.allocations = _.concat(spill_status.allocations, ['dummy']); } else { throw new Error('_handle_under_policy_threshold:: Cannot allocate without valid pools'); } } else if (_.get(decision_params.mirror_status, 'picked_pools.length', 0)) { spill_status.allocations = _.concat(spill_status.allocations, decision_params.mirror_status.picked_pools); + } else if (_.get(decision_params, 'additional_params.status_check', false)) { + // This is used when we only check the status without any actions + spill_status.allocations = _.concat(spill_status.allocations, ['dummy']); } else { throw new Error('_handle_under_policy_threshold:: Cannot allocate without valid pools'); } @@ -188,7 +194,7 @@ function _handle_over_policy_threshold(decision_params) { } -function get_chunk_status(chunk, tiering, async_mirror, tiering_pools_status) { +function get_chunk_status(chunk, tiering, additional_params) { // TODO handle multi-tiering if (tiering.tiers.length !== 1) { throw new Error('analyze_chunk: ' + @@ -204,8 +210,8 @@ function get_chunk_status(chunk, tiering, async_mirror, tiering_pools_status) { }; // When allocating we will pick the best mirror using weights algorithm - const participating_mirrors = async_mirror ? - select_prefered_mirrors(tier, tiering_pools_status) : + const participating_mirrors = _.get(additional_params, 'async_mirror', false) ? + select_prefered_mirrors(tier, _.get(additional_params, 'tiering_pools_status', undefined)) : tier.mirrors || []; let used_blocks = []; @@ -215,8 +221,10 @@ function get_chunk_status(chunk, tiering, async_mirror, tiering_pools_status) { _.each(participating_mirrors, mirror => { // Selecting the allocating pool for the current mirror // Notice that this is only relevant to the current chunk - let mirror_status = select_pool_type(mirror.spread_pools, tiering_pools_status); - let status_result = _get_mirror_chunk_status(chunk, tier, mirror_status, mirror.spread_pools); + let mirror_status = select_pool_type(mirror.spread_pools, + _.get(additional_params, 'tiering_pools_status', undefined)); + let status_result = _get_mirror_chunk_status(chunk, tier, mirror_status, + mirror.spread_pools, additional_params); chunk_status.allocations = _.concat(chunk_status.allocations, status_result.allocations); chunk_status.deletions = _.concat(chunk_status.deletions, status_result.deletions); // These two are used in order to delete all unused blocks by the policy @@ -237,7 +245,7 @@ function get_chunk_status(chunk, tiering, async_mirror, tiering_pools_status) { } -function _get_mirror_chunk_status(chunk, tier, mirror_status, mirror_pools) { +function _get_mirror_chunk_status(chunk, tier, mirror_status, mirror_pools, additional_params) { const tier_pools_by_name = _.keyBy(mirror_pools, 'name'); let allocations = []; @@ -306,7 +314,8 @@ function _get_mirror_chunk_status(chunk, tier, mirror_status, mirror_pools) { mirror_status: mirror_status, placement_weights: PLACEMENT_WEIGHTS, max_replicas: max_replicas, - current_weight: num_good + current_weight: num_good, + additional_params: additional_params }; // Checking if we are under and over the policy threshold @@ -446,12 +455,16 @@ function is_block_accessible(block) { } function is_chunk_good(chunk, tiering) { - let status = get_chunk_status(chunk, tiering, /*async_mirror=*/ false); + let status = get_chunk_status(chunk, tiering, { + status_check: true + }); return status.accessible && !status.allocations.length; } function is_chunk_accessible(chunk, tiering) { - let status = get_chunk_status(chunk, tiering, /*async_mirror=*/ false); + let status = get_chunk_status(chunk, tiering, { + status_check: true + }); return status.accessible; } @@ -484,7 +497,9 @@ function get_chunk_info(chunk, adminfo) { if (adminfo) { c.adminfo = {}; let bucket = system_store.data.get_by_id(chunk.bucket); - let status = get_chunk_status(chunk, bucket.tiering, /*async_mirror=*/ false); + let status = get_chunk_status(chunk, bucket.tiering, { + status_check: true + }); if (!status.accessible) { c.adminfo.health = 'unavailable'; } else if (status.allocations.length) { diff --git a/src/test/unit_tests/test_map_utils.js b/src/test/unit_tests/test_map_utils.js index 271afaee23..fc470ea442 100644 --- a/src/test/unit_tests/test_map_utils.js +++ b/src/test/unit_tests/test_map_utils.js @@ -41,7 +41,9 @@ mocha.describe('map_utils', function() { let chunk = {}; chunk.frags = map_utils.get_missing_frags_in_chunk( chunk, tiering.tiers[0].tier); - let status = map_utils.get_chunk_status(chunk, tiering, false, tiering_pools_status); + let status = map_utils.get_chunk_status(chunk, tiering, { + tiering_pools_status: tiering_pools_status + }); assert.strictEqual(status.allocations.length, total_num_blocks); assert.strictEqual(status.deletions.length, 0); assert(!status.accessible, '!accessible'); @@ -61,7 +63,9 @@ mocha.describe('map_utils', function() { frag: 0, node: mock_node(pools[i % num_pools]._id) }))); - let status = map_utils.get_chunk_status(chunk, tiering, false, tiering_pools_status); + let status = map_utils.get_chunk_status(chunk, tiering, { + tiering_pools_status: tiering_pools_status + }); assert.strictEqual(status.allocations.length, 0); assert.strictEqual(status.deletions.length, 0); assert(status.accessible, 'accessible'); @@ -84,7 +88,9 @@ mocha.describe('map_utils', function() { }); }); map_utils.set_chunk_frags_from_blocks(chunk, blocks); - let status = map_utils.get_chunk_status(chunk, tiering, false, tiering_pools_status); + let status = map_utils.get_chunk_status(chunk, tiering, { + tiering_pools_status: tiering_pools_status + }); assert.strictEqual(status.allocations.length, 0); assert.strictEqual(status.deletions.length, num_extra); assert(status.accessible, 'accessible'); @@ -98,7 +104,9 @@ mocha.describe('map_utils', function() { frag: 0, node: mock_node(pools[0]._id) }]); - let status = map_utils.get_chunk_status(chunk, tiering, false, tiering_pools_status); + let status = map_utils.get_chunk_status(chunk, tiering, { + tiering_pools_status: tiering_pools_status + }); assert.strictEqual(status.allocations.length, total_num_blocks - 1); assert.strictEqual(status.deletions.length, 0); assert(status.accessible, 'accessible'); @@ -125,7 +133,9 @@ mocha.describe('map_utils', function() { node: mock_node(pools[0]._id) }]; map_utils.set_chunk_frags_from_blocks(chunk, blocks); - let status = map_utils.get_chunk_status(chunk, tiering, false, tiering_pools_status); + let status = map_utils.get_chunk_status(chunk, tiering, { + tiering_pools_status: tiering_pools_status + }); assert.strictEqual(status.allocations.length, total_num_blocks - 1); assert.strictEqual(status.deletions.length, 1); assert(status.accessible, 'accessible'); @@ -137,7 +147,9 @@ mocha.describe('map_utils', function() { }); }); map_utils.set_chunk_frags_from_blocks(chunk, blocks); - status = map_utils.get_chunk_status(chunk, tiering, false, tiering_pools_status); + status = map_utils.get_chunk_status(chunk, tiering, { + tiering_pools_status: tiering_pools_status + }); assert.strictEqual(status.allocations.length, 0); assert.strictEqual(status.deletions.length, 1); assert(status.accessible, 'accessible');