kskmori / pacemaker-1.0

Code for the older 1.0 series of Pacemaker
http://www.clusterlabs.org
GNU General Public License v2.0
1 stars 0 forks source link

[1.0.13] Low: PE: Sort nodes alphabetically so that they display consistently in CLI tools #11

Closed kskmori closed 12 years ago

kskmori commented 12 years ago

I would like to backport this too, but the patch will result a big behavior change; regression.sh fails with 23 dot files differences (listed below) at least. It seems that it affects to the default location of nodes when scores are equal. For clones tests indexes such as :0 and :1 are exchanged. For some tests a resource would be migrated. I would hesitate to change like those in 1.0.

Instead I would suggest to modify for crm_mon and sort them only when it is displayed. The reported issue is only related for operators and we do not have to change the internal behavior.

How do you think about it?

PEND, 7be61e6 ,2012-07-30,"Low: PE: Sort nodes alphabetically so that they display consistently in CLI tools (update regression test)" PEND, af407b9 ,2012-07-30,"Low: PE: Sort nodes alphabetically so that they display consistently in CLI tools"

# grep pe.dot regression.failed.diff
+++ ./test10/probe-0.pe.dot 2012-08-17 17:26:13.757405950 +0900
+++ ./test10/order-clone.pe.dot 2012-08-17 17:26:14.610404097 +0900
+++ ./test10/clone-order-primitive.pe.dot   2012-08-17 17:26:14.681404055 +0900
+++ ./test10/coloc-clone-stays-active.pe.dot    2012-08-17 17:26:15.077403779 +0900
+++ ./test10/rec-node-12.pe.dot 2012-08-17 17:26:16.373415865 +0900
+++ ./test10/migrate-start-complex.pe.dot   2012-08-17 17:26:16.693403979 +0900
+++ ./test10/novell-252693-3.pe.dot 2012-08-17 17:26:17.186403272 +0900
+++ ./test10/bug-1718.pe.dot    2012-08-17 17:26:17.836437558 +0900
+++ ./test10/bug-lf-2422.pe.dot 2012-08-17 17:26:17.875417177 +0900
+++ ./test10/clone-anon-probe-1.pe.dot  2012-08-17 17:26:17.936404119 +0900
+++ ./test10/clone-no-shuffle.pe.dot    2012-08-17 17:26:18.584405538 +0900
+++ ./test10/master-4.pe.dot    2012-08-17 17:26:19.100408081 +0900
+++ ./test10/master-5.pe.dot    2012-08-17 17:26:19.163428293 +0900
+++ ./test10/master-6.pe.dot    2012-08-17 17:26:19.214404610 +0900
+++ ./test10/interleave-0.pe.dot    2012-08-17 17:26:20.679403242 +0900
+++ ./test10/interleave-1.pe.dot    2012-08-17 17:26:20.763403414 +0900
+++ ./test10/interleave-2.pe.dot    2012-08-17 17:26:20.849404072 +0900
+++ ./test10/interleave-3.pe.dot    2012-08-17 17:26:20.933403734 +0900
+++ ./test10/novell-239079.pe.dot   2012-08-17 17:26:21.256404660 +0900
+++ ./test10/726.pe.dot 2012-08-17 17:26:21.411404305 +0900
+++ ./test10/829.pe.dot 2012-08-17 17:26:21.565404361 +0900
+++ ./test10/bug-lf-1920.pe.dot 2012-08-17 17:26:22.281406652 +0900
+++ ./test10/bug-lf-2551.pe.dot 2012-08-17 17:26:22.447407486 +0900
# grep pe.dot regression.failed.diff | wc -l
23
beekhof commented 12 years ago

"Instead I would suggest to modify for crm_mon and sort them only when it is displayed."

That sounds like a very good idea :)

kskmori commented 12 years ago

Fixed in: https://github.com/kskmori/pacemaker-1.0/commit/80d6d0018cc2324109f039c181c404be0d34d0c2

kskmori commented 12 years ago

Migration summary should be sorted too:

============
Last updated: Tue Aug 21 20:03:51 2012
Stack: Heartbeat
Current DC: 11-sl6 (51266a31-10ba-4fdb-92b7-a33b3e7ceefd) - partition with quorum
Version: 1.0.12-607b226
2 Nodes configured, unknown expected votes
0 Resources configured.
============

Online: [ 10-sl6 11-sl6 ]

Node Attributes:
* Node 10-sl6:
* Node 11-sl6:

Migration summary:
* Node 11-sl6:
* Node 10-sl6:
kskmori commented 12 years ago

Andrew,

I've pushed two additional changes for this, and for the sake of printing clone resources with sorted nodes I have added 'pe_print_sort' option for clone_print(). Are you comfortable with this change?

f9e5006 Low: tools: crm_mon display nodes in sorted order (clone resources) 657c51d Low: tools: crm_mon display nodes in sorted order (Migration summary)

beekhof commented 12 years ago

Sure, both patches look good :-)

nozawat commented 12 years ago

Thank you. I confirmed it in environment of RHEL6.2-x86_64.

kskmori commented 12 years ago

Thanks for your comfirmation!