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 > > >
0001-slurmctld-ensure-reordering-of-nodes-prior-to-topolo.patch
Description: Binary data
