Closed davidjumani closed 10 months ago
in scripts this could cause a lot of print/lines @davidjumani ?
Nope @rohityadavcloud as this is just refactoring the code to be more efficient. There are no additional print statements added
@rohityadavcloud Could you please review / merge this ? I'd like to add a fix for https://github.com/apache/cloudstack-cloudmonkey/issues/128 but it would cause a conflict with this
cc @davidjumani @nvazquez could you discuss and review if we need this for 6.3 cc @borisstoyanov
Since this is a technical depth, not a feature, I'm thinking to push it for next release. I believe we should merge this after we release 6.3 and start building 6.4 on top of it. That way we'll get a lot more testing done on these changes.
@rohityadavcloud @davidjumani please shout if you have any concerns.
Fine by me @borisstoyanov but if this improves the user-experience and is easy to review/test then we should consider to get it in - pl advise @nvazquez @davidjumani
There seems to be a bit of performance improvement, but I don't think it's worth risking, since it does require more testing.
➜ cloudstack-cloudmonkey git:(optimize-nw) time ./bin/cmk sync
Discovered 624 APIs
./bin/cmk sync 0.23s user 0.03s system 19% cpu 1.291 total
➜ cloudstack-cloudmonkey git:(optimize-nw) time ./bin/cmk deploy virtualmachine zoneid=dc7c3d06-3495-4dfd-b8a5-793e4d269de3 templateid=ce8e5b85-e347-11ed-b67b-1e009d000109 serviceofferingid=1b2f3aae-0b5f-4aea-ac11-5c1f3c8e6be9 networkids=60c0f051-444e-429f-8cae-5bfa3a59913d
{
....
}
}
./bin/cmk deploy virtualmachine zoneid=dc7c3d06-3495-4dfd-b8a5-793e4d269de3
0.10s user 0.02s system 0% cpu 20.179 total
vs old way
[root@ref-trl-4827-v-M7-boris-stoyanov-mgmt1 ~]# time cmk sync
Discovered 624 APIs
real 0m1.352s
user 0m0.269s
sys 0m0.039s
[root@ref-trl-4827-v-M7-boris-stoyanov-mgmt1 ~]# time cmk deploy virtualmachine zoneid=dc7c3d06-3495-4dfd-b8a5-793e4d269de3 templateid=ce8e5b85-e347-11ed-b67b-1e009d000109 serviceofferingid=1b2f3aae-0b5f-4aea-ac11-5c1f3c8e6be9 networkids=60c0f051-444e-429f-8cae-5bfa3a59913d
{
......
}
}
real 0m22.197s
user 0m0.122s
sys 0m0.026s
There seems to be a bit of performance improvement, but I don't think it's worth risking, since it does require more testing.
➜ cloudstack-cloudmonkey git:(optimize-nw) time ./bin/cmk sync Discovered 624 APIs ./bin/cmk sync 0.23s user 0.03s system 19% cpu 1.291 total ➜ cloudstack-cloudmonkey git:(optimize-nw) time ./bin/cmk deploy virtualmachine zoneid=dc7c3d06-3495-4dfd-b8a5-793e4d269de3 templateid=ce8e5b85-e347-11ed-b67b-1e009d000109 serviceofferingid=1b2f3aae-0b5f-4aea-ac11-5c1f3c8e6be9 networkids=60c0f051-444e-429f-8cae-5bfa3a59913d { .... } } ./bin/cmk deploy virtualmachine zoneid=dc7c3d06-3495-4dfd-b8a5-793e4d269de3 0.10s user 0.02s system 0% cpu 20.179 total
vs old way
[root@ref-trl-4827-v-M7-boris-stoyanov-mgmt1 ~]# time cmk sync Discovered 624 APIs real 0m1.352s user 0m0.269s sys 0m0.039s [root@ref-trl-4827-v-M7-boris-stoyanov-mgmt1 ~]# time cmk deploy virtualmachine zoneid=dc7c3d06-3495-4dfd-b8a5-793e4d269de3 templateid=ce8e5b85-e347-11ed-b67b-1e009d000109 serviceofferingid=1b2f3aae-0b5f-4aea-ac11-5c1f3c8e6be9 networkids=60c0f051-444e-429f-8cae-5bfa3a59913d { ...... } } real 0m22.197s user 0m0.122s sys 0m0.026s
looks good !
Agree, we can merge this after 6.3 is out @borisstoyanov
merging this as all comments point to this being the moment (and also lgtm by Rohit + tested by Wei)
Uses tickers instead of sleep to optimize waiting after querying an async job