Skip to content

Commit

Permalink
Fixing node mappings crashes (#2473)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeniawhite authored Jan 4, 2017
1 parent 726dcab commit 4e8e96b
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 18 deletions.
7 changes: 6 additions & 1 deletion src/server/object_services/map_allocator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];

Expand Down
4 changes: 3 additions & 1 deletion src/server/object_services/map_builder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand Down
35 changes: 25 additions & 10 deletions src/server/object_services/map_utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
Expand Down Expand Up @@ -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: ' +
Expand All @@ -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 = [];
Expand All @@ -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
Expand All @@ -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 = [];
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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) {
Expand Down
24 changes: 18 additions & 6 deletions src/test/unit_tests/test_map_utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand All @@ -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');
Expand Down

0 comments on commit 4e8e96b

Please sign in to comment.