Add an option to not move primary OSDs of unclean PGs

Co-Authored-By: Yury Luneff <koolkhel@mail.ru>

mon/etcd_schema.js

@@ -65,6 +65,7 @@ const etcd_tree = {
             placement_levels: { datacenter: 1, rack: 2, host: 3, osd: 4, ... },
             use_old_pg_combinator: false,
             osd_backfillfull_ratio: 0.99,
+            mon_hold_unclean_pg_primary: false,
             mon_metrics_path: '/prometheus',
             mon_healthcheck_path: '/healthcheck',
             // client and osd

mon/mon.js

@@ -74,6 +74,7 @@ class Mon
         this.state = JSON.parse(JSON.stringify(etcd_tree));
         this.prev_stats = { osd_stats: {}, osd_diff: {} };
         this.recheck_pgs_active = false;
+        this.primaries_held = false;
         this.updating_total_stats = false;
         this.watcher_active = false;
         this.old_pg_config = false;
@@ -199,6 +200,7 @@ class Mon
         {
             this.config.osd_out_time = 600; // 10 minutes by default
         }
+        this.config.mon_hold_unclean_pg_primary = !!this.config.mon_hold_unclean_pg_primary;
     }
 
     on_message(msg)
@@ -252,7 +254,7 @@ class Mon
         {
             this.schedule_update_stats();
         }
-        if (changed)
+        if (changed || this.primaries_held && pg_states_changed)
         {
             this.schedule_recheck();
         }
@@ -564,7 +566,8 @@ class Mon
         else
         {
             // Nothing changed, but we still want to recheck the distribution of primaries
-            let new_pg_config = recheck_primary(this.state, this.config, up_osds, osd_tree);
+            let { new_pg_config, held } = recheck_primary(this.state, this.config, up_osds, osd_tree, held);
+            this.primaries_held = held;
             if (new_pg_config)
             {
                 const ok = await this.save_pg_config(new_pg_config);
@@ -626,7 +629,7 @@ class Mon
                 etcd_request.success.push({ requestDeleteRange: {
                     key: b64(this.config.etcd_prefix+'/pool/stats/'+pool_id),
                 } });
-                save_new_pgs_txn(new_pg_config, etcd_request, this.state, this.config.etcd_prefix,
+                save_new_pgs_txn(new_pg_config, etcd_request, this.state, this.config,
                     this.etcd_watch_revision, pool_id, up_osds, osd_tree, prev_pgs, [], []);
             }
         }
@@ -669,7 +672,7 @@ class Mon
                 key: b64(this.config.etcd_prefix+'/pool/stats/'+pool_id),
                 value: b64(JSON.stringify(stats)),
             } });
-            save_new_pgs_txn(new_pg_config, etcd_request, this.state, this.config.etcd_prefix,
+            save_new_pgs_txn(new_pg_config, etcd_request, this.state, this.config,
                 this.etcd_watch_revision, pool_id, up_osds, osd_tree, real_prev_pgs, pool_res.pgs, pg_history);
         }
         new_pg_config.hash = tree_hash;

mon/pg_gen.js

@@ -10,6 +10,12 @@ const { scale_pg_count } = require('./pg_utils.js');
 const { make_hier_tree, filter_osds_by_root_node,
     filter_osds_by_tags, filter_osds_by_block_layout, get_affinity_osds } = require('./osd_tree.js');
 
+function is_clean(state)
+{
+    return (state instanceof Array) && !state.filter(s => s != 'active' && s != 'left_on_dead' &&
+        s != 'has_incomplete' && s != 'has_invalid').length;
+}
+
 function pick_primary(pool_id, pg_num, pool_config, osd_set, up_osds, aff_osds)
 {
     let alive_set;
@@ -44,6 +50,7 @@ function pick_primary(pool_id, pg_num, pool_config, osd_set, up_osds, aff_osds)
 function recheck_primary(state, global_config, up_osds, osd_tree)
 {
     let new_pg_config;
+    let held = false;
     for (const pool_id in state.config.pools)
     {
         const pool_cfg = state.config.pools[pool_id];
@@ -62,7 +69,16 @@ function recheck_primary(state, global_config, up_osds, osd_tree)
             const pg_cfg = state.pg.config.items[pool_id][pg_num];
             if (pg_cfg)
             {
-                const new_primary = pick_primary(pool_id, pg_num, state.config.pools[pool_id], pg_cfg.osd_set, up_osds, aff_osds);
+                const pg_state = (state.pg.state[pool_id]||{})[pg_num]||{};
+                let new_primary = pg_state.primary;
+                if (global_config.mon_hold_unclean_pg_primary && !is_clean(pg_state.state) && new_primary && up_osds[new_primary])
+                {
+                    held = true;
+                }
+                else
+                {
+                    new_primary = pick_primary(pool_id, pg_num, state.config.pools[pool_id], pg_cfg.osd_set, up_osds, aff_osds);
+                }
                 if (pg_cfg.primary != new_primary)
                 {
                     if (!new_pg_config)
@@ -91,19 +107,24 @@ function recheck_primary(state, global_config, up_osds, osd_tree)
             }
         }
     }
-    return new_pg_config;
+    return { new_pg_config, held };
 }
 
-function save_new_pgs_txn(save_to, request, state, etcd_prefix, etcd_watch_revision, pool_id, up_osds, osd_tree, prev_pgs, new_pgs, pg_history)
+function save_new_pgs_txn(save_to, request, state, config, etcd_watch_revision, pool_id, up_osds, osd_tree, prev_pgs, new_pgs, pg_history)
 {
     const aff_osds = get_affinity_osds(state.config.pools[pool_id] || {}, up_osds, osd_tree);
     const pg_items = {};
+    const cur_pgs = config.mon_hold_unclean_pg_primary && prev_pgs.length == new_pgs.length && state.pg.state[pool_id];
     new_pgs.map((osd_set, i) =>
     {
         osd_set = osd_set.map(osd_num => osd_num === LPOptimizer.NO_OSD ? 0 : osd_num);
+        const hold = (cur_pgs && cur_pgs[i+1] &&
+            cur_pgs[i+1].state && !is_clean(cur_pgs[i+1].state) &&
+            cur_pgs[i+1].primary && up_osds[cur_pgs[i+1].primary] &&
+            osd_set.filter(c => c == cur_pgs[i+1].primary).length > 0);
         pg_items[i+1] = {
             osd_set,
-            primary: pick_primary(pool_id, i+1, state.config.pools[pool_id], osd_set, up_osds, aff_osds),
+            primary: hold ? cur_pgs[i+1].primary : pick_primary(pool_id, i+1, state.config.pools[pool_id], osd_set, up_osds, aff_osds),
         };
         if (prev_pgs[i] && prev_pgs[i].join(' ') != osd_set.join(' ') &&
             prev_pgs[i].filter(osd_num => osd_num).length > 0)
@@ -123,7 +144,7 @@ function save_new_pgs_txn(save_to, request, state, etcd_prefix, etcd_watch_revis
         // FIXME: etcd has max_txn_ops limit, and it's 128 by default
         // Sooo we probably want to change our storage scheme for PG histories...
         request.compare.push({
-            key: b64(etcd_prefix+'/pg/history/'+pool_id+'/'+(i+1)),
+            key: b64(config.etcd_prefix+'/pg/history/'+pool_id+'/'+(i+1)),
             target: 'MOD',
             mod_revision: ''+etcd_watch_revision,
             result: 'LESS',
@@ -132,7 +153,7 @@ function save_new_pgs_txn(save_to, request, state, etcd_prefix, etcd_watch_revis
         {
             request.success.push({
                 requestPut: {
-                    key: b64(etcd_prefix+'/pg/history/'+pool_id+'/'+(i+1)),
+                    key: b64(config.etcd_prefix+'/pg/history/'+pool_id+'/'+(i+1)),
                     value: b64(JSON.stringify(pg_history[i])),
                 },
             });
@@ -141,7 +162,7 @@ function save_new_pgs_txn(save_to, request, state, etcd_prefix, etcd_watch_revis
         {
             request.success.push({
                 requestDeleteRange: {
-                    key: b64(etcd_prefix+'/pg/history/'+pool_id+'/'+(i+1)),
+                    key: b64(config.etcd_prefix+'/pg/history/'+pool_id+'/'+(i+1)),
                 },
             });
         }

mon/tests/primary-stickiness.js

@@ -0,0 +1,158 @@
+// Copyright (c) Vitaliy Filippov, 2019+
+// License: VNPL-1.1 (see README.md for details)
+
+// Primary OSD selection must not move a PG away from a live primary while the
+// PG is unclean. Run: node mon/test-primary-stickiness.js
+
+const assert = require('assert');
+
+const { recheck_primary, save_new_pgs_txn } = require('../pg_gen.js');
+const LPOptimizer = require('../lp_optimizer/lp_optimizer.js');
+
+const HOSTS = 6, OSDS_PER_HOST = 16;
+const host_osds = (h) => Array.from({ length: OSDS_PER_HOST }, (_, i) => h*OSDS_PER_HOST+i+1);
+
+// EC 4+2 over 6 hosts, one OSD of each host per PG; every PG active+clean on its balanced primary
+function make_state(pg_count)
+{
+    const state = {
+        config: { pools: { 1: { name: 'ec', scheme: 'ec', pg_size: 6, parity_chunks: 2, pg_minsize: 4,
+            pg_count, failure_domain: 'host' } } },
+        pg: { config: { items: { 1: {} } }, state: { 1: {} } },
+    };
+    for (let pg = 1; pg <= pg_count; pg++)
+    {
+        const osd_set = [];
+        for (let h = 0; h < HOSTS; h++)
+            osd_set.push(host_osds((h + pg) % HOSTS)[(pg * 7 + h) % OSDS_PER_HOST]);
+        state.pg.config.items[1][pg] = { osd_set, primary: 0 };
+    }
+    const all_up = up_except([]);
+    const moved = recheck_primary(state, { mon_hold_unclean_pg_primary: true }, all_up, {}).new_pg_config;
+    state.pg.config = moved;
+    for (let pg = 1; pg <= pg_count; pg++)
+        state.pg.state[1][pg] = { primary: state.pg.config.items[1][pg].primary, state: [ 'active' ] };
+    return state;
+}
+
+function up_except(down)
+{
+    const up = {};
+    for (let h = 0; h < HOSTS; h++)
+        for (const osd of host_osds(h))
+            up[osd] = true;
+    for (const osd of down)
+        delete up[osd];
+    return up;
+}
+
+// OSDs report every PG that lost a member as degraded+has_unclean
+function mark_unclean(state, up_osds)
+{
+    for (const pg in state.pg.config.items[1])
+    {
+        const cfg = state.pg.config.items[1][pg];
+        if (cfg.osd_set.some(osd => !up_osds[osd]))
+            state.pg.state[1][pg] = { primary: cfg.primary, state: [ 'active', 'degraded', 'has_unclean' ] };
+    }
+}
+
+function moves(state, new_cfg)
+{
+    const res = [];
+    for (const pg in (new_cfg||{items:{1:{}}}).items[1])
+    {
+        const from = state.pg.config.items[1][pg].primary, to = new_cfg.items[1][pg].primary;
+        if (from != to)
+            res.push({ pg, from, to });
+    }
+    return res;
+}
+
+function test_node_failure_moves_only_dead_primaries()
+{
+    const state = make_state(1024);
+    const down = host_osds(4);
+    const up_osds = up_except(down);
+    mark_unclean(state, up_osds);
+    const recheck = recheck_primary(state, { mon_hold_unclean_pg_primary: true }, up_osds, {});
+    const mv = moves(state, recheck.new_pg_config);
+    const from_live = mv.filter(m => up_osds[m.from]);
+    console.log(`node down: ${mv.length} moves, ${from_live.length} away from a live primary, held = ${recheck.held}`);
+    assert(mv.length > 0, 'PGs whose primary died must move');
+    assert.equal(from_live.length, 0, 'a live primary must stay while its PG is unclean');
+    assert(mv.every(m => up_osds[m.to]));
+    assert(recheck.held);
+}
+
+// clean_state: a state the monitor counts as clean (see Mon.save_last_clean())
+function test_no_flap_back_when_osd_returns(clean_state)
+{
+    const state = make_state(256);
+    const pg = Object.keys(state.pg.config.items[1])[0];
+    const cfg = state.pg.config.items[1][pg];
+    const old_primary = cfg.primary;
+    // The primary crashes: the PG moves away from it
+    let up_osds = up_except([ old_primary ]);
+    mark_unclean(state, up_osds);
+    const after_crash = recheck_primary(state, { mon_hold_unclean_pg_primary: true }, up_osds, {}).new_pg_config;
+    const new_primary = after_crash.items[1][pg].primary;
+    assert.notEqual(new_primary, old_primary);
+    state.pg.config = after_crash;
+    state.pg.state[1][pg] = { primary: new_primary, state: [ 'active', 'degraded', 'has_unclean' ] };
+    // It restarts 2 s later while the PG is still unclean: no move back
+    up_osds = up_except([]);
+    const back = recheck_primary(state, { mon_hold_unclean_pg_primary: true }, up_osds, {}).new_pg_config;
+    assert(!back || back.items[1][pg].primary == new_primary, 'no move back while the PG is unclean');
+    // Once the PGs are clean on their new primaries, they return to the balanced ones
+    for (const p in state.pg.config.items[1])
+        state.pg.state[1][p] = { primary: state.pg.config.items[1][p].primary, state: clean_state };
+    const clean = recheck_primary(state, { mon_hold_unclean_pg_primary: true }, up_osds, {});
+    assert(clean.new_pg_config.items[1][pg].primary == old_primary);
+    assert(!clean.held);
+    console.log(`no flap back, back once ${clean_state.join('+')}: ok`);
+}
+
+function test_reoptimize_keeps_live_primary()
+{
+    const state = make_state(64);
+    const up_osds = up_except(host_osds(4));
+    mark_unclean(state, up_osds);
+    const prev_pgs = Object.values(state.pg.config.items[1]).map(c => c.osd_set);
+    const save_to = {}, request = { compare: [], success: [] };
+    save_new_pgs_txn(save_to, request, state, { etcd_prefix: '/vitastor', mon_hold_unclean_pg_primary: true },
+        1, 1, up_osds, {}, prev_pgs, prev_pgs.map(s => [ ...s ]), []);
+    const mv = moves(state, save_to);
+    assert(mv.length > 0, 'PGs whose primary died must move');
+    console.log(save_to);
+    console.log(mv);
+    assert.equal(mv.filter(m => up_osds[m.from]).length, 0, 're-optimisation must not move a live primary');
+    console.log(`re-optimize: ${mv.length} moves, all from dead primaries`);
+}
+
+// A clean PG with its whole current set up may move even if its new set has an empty slot
+function test_reoptimize_judges_current_set()
+{
+    const state = make_state(64);
+    const up_osds = up_except([]);
+    const prev_pgs = Object.values(state.pg.config.items[1]).map(c => c.osd_set);
+    const new_pgs = prev_pgs.map(s => s.map((osd, i) => i == 1 ? LPOptimizer.NO_OSD : osd));
+    const save_to = {}, request = { compare: [], success: [] };
+    save_new_pgs_txn(save_to, request, state, { etcd_prefix: '/vitastor', mon_hold_unclean_pg_primary: true },
+        1, 1, up_osds, {}, prev_pgs, new_pgs, []);
+    const kept_in_set = moves(state, save_to).filter(m => save_to.items[1][m.pg].osd_set.includes(m.from));
+    assert(kept_in_set.length > 0, 'a clean PG must not be held because of its new set');
+    console.log(`re-optimize, clean PGs: ${kept_in_set.length} rebalanced`);
+}
+
+async function run()
+{
+    test_node_failure_moves_only_dead_primaries();
+    test_no_flap_back_when_osd_returns([ 'active' ]);
+    test_no_flap_back_when_osd_returns([ 'active', 'left_on_dead' ]);
+    test_reoptimize_keeps_live_primary();
+    test_reoptimize_judges_current_set();
+    console.log('OK');
+}
+
+run().catch(e => { console.error(e); process.exit(1); });