I believe that Matthieu identified the problem. His patch could result
in sorting the nodes twice for some systems (once by name and a second
time by rank). I believe the following patch would address the problem
without the possibility of a second sort. This patch is untested.
Chris, Please respond when you have a chance to test the patches.
diff --git a/src/slurmctld/read_config.c b/src/slurmctld/read_config.c
index b45cea5..82ccd33 100644
--- a/src/slurmctld/read_config.c
+++ b/src/slurmctld/read_config.c
@@ -510,8 +510,6 @@ static int _build_all_nodeline_info(void)
info("WARNING: No node %s configured", node_000);
xfree(node_000);
}
-#else /* HAVE_BG */
- slurm_topo_build_config();
#endif /* HAVE_BG */
return rc;
@@ -820,6 +818,7 @@ int read_slurm_conf(int recover, bool reconfig)
_reorder_nodes_by_rank();
else
_reorder_nodes_by_name();
+ slurm_topo_build_config();
rehash_node();
Quoting Matthieu Hautreux <[email protected]>:
> Hi,
>
> nice catch !
>
> However you might still have issues with env variables (SLURM_NODELIST and
> SLURM_TASKS_PER_NODE) not being in sync or incorrect tasks layouts with
> heterogeneous jobs (different number of cores per allocated node with
> cons_res) due to the removal of the reordering in natural order. These
> issues were at the origin of this reordering as well as an easier
> understanding for user when managing their allocated nodesets (in general,
> they do not really want to handle the complex way nodes can be described in
> the configuration file and rather prefer a coherent and unique way to
> handle that)
>
> I would suggest to move the reordering in natural order prior to the
> topology configuration as proposed in the enclosed (but not at all tested)
> patch. Any opposition/suggestion/comment ?
>
> Regards,
> Matthieu
>
>
> 2012/12/18 Holmes, Christopher (CMU) <[email protected]>
>
>>
>> Hello all,
>>
>> After several days of brainstorming and debugging, I have identified a bug
>> in SLURM 2.5.0rc2, related to the 'tree' topology. It was so early in the
>> execution of the whole SLURM machinery that it took me some time to figure
>> it out (say, 100 or 200 jobs showing the issue, with more or less debugging
>> levels increased and extra instrumentation, with sometimes an uncertain
>> reliability)...
>>
>> For every “switch” a bitmap of nodes (seen down by the switch) is built as
>> the topology is discovered through 'topology.conf'.
>>
>> There is code in read_config.c, executed when the SLURM control daemon
>> starts, that reorders the nodes (according to their hostname by default),
>> while the switches table (ie the bitmaps) has already being built. To
>> reorder the nodes means that the bitmaps of the switches become wrong.
>>
>> I've applied this simple patch locally to maintain the consistency between
>> the hostnames and the nodes bitmaps used for the switches:
>>
>> $ no_reordering.patch
>> --- ./src/slurmctld/read_config.c 2012-12-18 13:44:56.000000000 +0100
>> +++ ./src/slurmctld/read_config.c.hp 2012-12-18 13:31:28.000000000 +0100
>> @@ -819,7 +819,7 @@
>> if (do_reorder_nodes)
>> _reorder_nodes_by_rank();
>> else
>> - _reorder_nodes_by_name();
>> +//HP: would make switch table point to wrong data!
>> _reorder_nodes_by_name();
>>
>> rehash_node();
>> rehash_jobs();
>>
>>
>> Ideally, as nodes are sorted, the bitmaps would be updated accordingly. Or
>> they would be recreated afterwards from scratch.
>>
>>
>> The sequence that creates the trouble is in src/slurmctld/read_config.c:
>> ...
>> /* Build node and partition information based upon slurm.conf file
>> */
>> _build_all_nodeline_info();
>> <------------------------------- parsing of slurm.conf (hence
>> topology.conf); the switch table is built here
>> ...
>> /*
>> * Node reordering needs to be done by the topology and/or select
>> * plugin. Reordering the table must be done before hashing the
>> * nodes, and before any position-relative bitmaps are created.
>> */
>>
>> do_reorder_nodes |= slurm_topo_generate_node_ranking();
>> <-------- note that the tree topology does not enforce reordering
>> do_reorder_nodes |=
>> select_g_node_ranking(node_record_table_ptr, (by the fway,
>> do_reorder_nodes is initialized to false)
>> node_record_count);
>> ...
>> if (do_reorder_nodes){
>> _reorder_nodes_by_rank();
>> }
>> else{
>> _reorder_nodes_by_name();
>> <-------------------------------- branch taken: this reorders the nodes
>> table but then the switch bitmaps have their bits pointing to different
>> nodes! The topology as described in topology.conf is simply broken...
>> }
>> ...
>>
>> I’m hopeful that someone more in-tune with the topology plugin will know
>> the proper way to fix this. J
>>
>> Best regards,
>> --Chris
>>
>>
>>
>
