payload: {"action":"created","issue":{"url":"https://api.github.com/repos/PBSPro/pbspro/issues/1034","repository_url":"https://api.github.com/repos/PBSPro/pbspro","labels_url":"https://api.github.com/repos/PBSPro/pbspro/issues/1034/labels{/name}","comments_url":"https://api.github.com/repos/PBSPro/pbspro/issues/1034/comments","events_url":"https://api.github.com/repos/PBSPro/pbspro/issues/1034/events","html_url":"https://github.com/PBSPro/pbspro/pull/1034","id":420759876,"node_id":"MDExOlB1bGxSZXF1ZXN0MjYwOTcwNjkx","number":1034,"title":"PTL setUp takes too long when there's large no. of vnodes","user":{"login":"agrawalravi90","id":4574875,"node_id":"MDQ6VXNlcjQ1NzQ4NzU=","avatar_url":"https://avatars1.githubusercontent.com/u/4574875?v=4","gravatar_id":"","url":"https://api.github.com/users/agrawalravi90","html_url":"https://github.com/agrawalravi90","followers_url":"https://api.github.com/users/agrawalravi90/followers","following_url":"https://api.github.com/users/agrawalravi90/following{/other_user}","gists_url":"https://api.github.com/users/agrawalravi90/gists{/gist_id}","starred_url":"https://api.github.com/users/agrawalravi90/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/agrawalravi90/subscriptions","organizations_url":"https://api.github.com/users/agrawalravi90/orgs","repos_url":"https://api.github.com/users/agrawalravi90/repos","events_url":"https://api.github.com/users/agrawalravi90/events{/privacy}","received_events_url":"https://api.github.com/users/agrawalravi90/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2019-03-13T23:09:01Z","updated_at":"2019-03-19T11:38:39Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/PBSPro/pbspro/pulls/1034","html_url":"https://github.com/PBSPro/pbspro/pull/1034","diff_url":"https://github.com/PBSPro/pbspro/pull/1034.diff","patch_url":"https://github.com/PBSPro/pbspro/pull/1034.patch"},"body":"<!--- Please review your changes in preview mode -->\r\n<!--- Provide a general summary of your changes in the Title above -->\r\n\r\n#### Bug/feature Description\r\n* PTL's setUp() was taking to long (15+ minutes) when there are large number of nodes (~10k) in the system (reference testsuite: TestSchedPerf)\r\n\r\n#### Affected Platform(s)\r\n* Linux\r\n\r\n#### Cause / Analysis\r\n* There were 2 bottlenecks:\r\n  1) pbs_init.d restart: This was taking 10+ minutes. Cause: the server function check_and_set_multivnode() takes too long when there are that many nodes in the system\r\n  2) delete custom resource: Deleting just one custom resource was taking 4+ minutes. Cause: when PBS deletes a resource, it has to unset it from all the PBS objects where it is set\r\n\r\n#### Solution Description\r\n* Modified Server's revert_to_default() to delete all nodes before deleting all custom resources\r\n* Reordered the revert_to_defaults calls in setUp() so that revert_server is called first, this makes sure that we don't restart pbs before all the nodes have been deleted from the server\r\n\r\n#### Testing logs/output\r\n* I saw a performance improvement of around 51 minutes (~39%) when running TestSchedPerf. The run time went down from 2 hours 11 mins to 1 hour 20 mins. Logs here:\r\n[testschedperf_log.tar.gz](https://github.com/PBSPro/pbspro/files/2964010/testschedperf_log.tar.gz)\r\n\r\nPlease note that one of the test fails in both the original and after changes logs, so it has nothing to do with these changes. Hopefully we can run performance tests more often with this performance boost.\r\n\r\n#### Checklist:\r\n<!--- Use the preview button to see the checkboxes/links properly. -->\r\n<!--- Go over the following points, and put an `x` (without spaces around it) in the boxes that apply. -->\r\n<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->\r\n- [X] I have joined the **[pbspro community forum](http://community.pbspro.org/)**.\r\n- [X] My pull request contains a **single, signed** commit. See **[setting up gpg signature](https://pbspro.atlassian.net/wiki/display/DG/Signing+Your+Git+Commits)**.\r\n- [X] My code follows the **[coding style](https://pbspro.atlassian.net/wiki/display/DG/Coding+Standards)** of this project.\r\n- [ ] My change requires project documentation. See **[required documentation checklist](https://pbspro.atlassian.net/wiki/display/DG/Checklist+for+Developing+Features+and+Bug+Fixes)** for details.\r\n   - [ ] I have added documentation in the **[project documentation area](https://pbspro.atlassian.net/wiki/display/PD)**.\r\n- [ ] I have added new **PTL test(s) to my commit**. (See **[using PTL for testing](https://pbspro.atlassian.net/wiki/display/DG/Using+PTL+for+Testing)**) *(or)*\r\n   - [ ] I have added  **manual test(s) to this pull request and explained why PTL is not appropriate** for this case.\r\n- [ ] All new and existing automated tests have passed. (See **[running automated PTL tests](https://pbspro.atlassian.net/wiki/display/DG/PTL+Quick+Start+Guide)**).\r\n- [X] I have attached **test logs to this pull request** as evidence of testing/verification.\r\n\r\n\r\n__***For further information please visit the [Developer Guide Home](https://pbspro.atlassian.net/wiki/display/DG/Developer+Guide+Home).***__\r\n"},"comment":{"url":"https://api.github.com/repos/PBSPro/pbspro/issues/comments/474325531","html_url":"https://github.com/PBSPro/pbspro/pull/1034#issuecomment-474325531","issue_url":"https://api.github.com/repos/PBSPro/pbspro/issues/1034","id":474325531,"node_id":"MDEyOklzc3VlQ29tbWVudDQ3NDMyNTUzMQ==","user":{"login":"agrawalravi90","id":4574875,"node_id":"MDQ6VXNlcjQ1NzQ4NzU=","avatar_url":"https://avatars1.githubusercontent.com/u/4574875?v=4","gravatar_id":"","url":"https://api.github.com/users/agrawalravi90","html_url":"https://github.com/agrawalravi90","followers_url":"https://api.github.com/users/agrawalravi90/followers","following_url":"https://api.github.com/users/agrawalravi90/following{/other_user}","gists_url":"https://api.github.com/users/agrawalravi90/gists{/gist_id}","starred_url":"https://api.github.com/users/agrawalravi90/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/agrawalravi90/subscriptions","organizations_url":"https://api.github.com/users/agrawalravi90/orgs","repos_url":"https://api.github.com/users/agrawalravi90/repos","events_url":"https://api.github.com/users/agrawalravi90/events{/privacy}","received_events_url":"https://api.github.com/users/agrawalravi90/received_events","type":"User","site_admin":false},"created_at":"2019-03-19T11:38:39Z","updated_at":"2019-03-19T11:38:39Z","author_association":"CONTRIBUTOR","body":"> My only comment is that I see you deleting the nodes in a new place, but I don't see you removing the old place. Aren't you deleting them twice now?\r\n\r\nEarlier we were doing it as part of reverting mom to default. We'd delete vnodes for each mom, 1 mom at a time. Now, revert of server deletes all vnodes from the server, so the revert of mom tries to query vnodes on the mom and just returns when it doesn't find any vnodes on it. I had previously tried to do it as part of reverting mom itself, but it didn't work for the following reasons:\r\n1) Mom's delete_vnodes() only deletes vnodes which are on the same host as the mom node. So, it wasn't actually deleting all vnodes which reported to that mom.\r\n2) Even if we fix 1), reverting moms deletes only the vnodes associated to that mom, and each mom revert needs to restart PBS as it also reverts the mom config files. Restarting PBS is very expensive when there are many nodes in the system, so this wouldn't give the desired performance boost."}}
other: {"actor":{"display_login":"agrawalravi90"}}