You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cloudstack.apache.org by SudharmaJain <gi...@git.apache.org> on 2015/09/24 08:22:27 UTC
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
GitHub user SudharmaJain opened a pull request:
https://github.com/apache/cloudstack/pull/880
CLOUDSTACK-8901: PrepareTemplate job thread hard-coded to max 8 threads
The thread pool was hardcoded to use 8 threads,
com.cloud.template.TemplateManagerImpl.configure(String, Map<String, Object>):
_preloadExecutor = Executors.newFixedThreadPool(8, new NamedThreadFactory("Template-Preloader"));
Added the change to pick threadpool size from configuration.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/SudharmaJain/cloudstack cs-8901
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cloudstack/pull/880.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #880
----
commit 49ed8ee960acce13f32def10e15924c84257c4e4
Author: SudharmaJain <su...@citrix.com>
Date: 2015-09-24T06:11:54Z
CLOUDSTACK-8901: PrepareTemplate job thread hard-coded to max 8 threads
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/880#discussion_r41697322
--- Diff: setup/db/db/schema-452to460.sql ---
@@ -413,3 +413,4 @@ CREATE TABLE `cloud`.`ldap_trust_map` (
UNIQUE KEY `uk_ldap_trust_map__domain_id` (`domain_id`),
CONSTRAINT `fk_ldap_trust_map__domain_id` FOREIGN KEY (`domain_id`) REFERENCES `domain` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
--- End diff --
Please remove this empty line.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-212474909
Thanks @SudharmaJain LGTM (just code review), can you do a push -f (travis job failed for some reason)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by bhaisaab <gi...@git.apache.org>.
Github user bhaisaab commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-147090210
LGTM.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-218004558
Thanks @koushik-das. I will run this through CI just to be sure everything is good... Thx.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-218356820
These connectivity issues are unrelated to this PR. I will merge this now...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-212724576
@bhaisaab I pushed it again.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-216847988
LGTM. Verified that a configuration entry is added with default 8 as pool size. Increased the default threadpool size, restarted MS and verified that when multiple prepareTemplate APIs are invoked required number of threads gets created.
INFO [c.c.t.TemplateManagerImpl] (Template-Preloader-11:ctx-9cefa1ea) (logid:49186ba8) Start to preload template 201 into primary storage 2
INFO [c.c.t.TemplateManagerImpl] (Template-Preloader-12:ctx-601679be) (logid:1ea23b56) Start to preload template 201 into primary storage 3
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-217809810
@swill This has the required LGTMs.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/880#discussion_r41722125
--- Diff: setup/db/db/schema-452to460.sql ---
@@ -413,3 +413,4 @@ CREATE TABLE `cloud`.`ldap_trust_map` (
UNIQUE KEY `uk_ldap_trust_map__domain_id` (`domain_id`),
CONSTRAINT `fk_ldap_trust_map__domain_id` FOREIGN KEY (`domain_id`) REFERENCES `domain` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
--- End diff --
Removed the empty line.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/880#discussion_r40658483
--- Diff: server/src/com/cloud/template/TemplateManagerImpl.java ---
@@ -278,6 +278,8 @@
@Inject
private EndPointSelector selector;
+ protected final ConfigKey<Integer> TemplatePreloaderPoolSize = new ConfigKey<Integer>("Advanced", Integer.class, "template.preloader.pool.size", "8", "Size of the TemplateManager threadpool", false);
--- End diff --
@SudharmaJain You have to implement the Configurable interface. Check for its usage in code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by rhtyd <gi...@git.apache.org>.
Github user rhtyd commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-216194476
LGTM
tag:easypr
Cc @swill
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/880#discussion_r40658531
--- Diff: setup/db/db/schema-452to460.sql ---
@@ -413,3 +413,6 @@ CREATE TABLE `cloud`.`ldap_trust_map` (
UNIQUE KEY `uk_ldap_trust_map__domain_id` (`domain_id`),
CONSTRAINT `fk_ldap_trust_map__domain_id` FOREIGN KEY (`domain_id`) REFERENCES `domain` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+INSERT IGNORE INTO `cloud`.`configuration` VALUES ('Advanced', 'DEFAULT', 'TemplateManager', 'template.preloader.pool.size', '8', 'Size of the threadpool for preparetemplate api', '8', NULL, 'Global', 0);
--- End diff --
This is not required once the Configurable is implemented.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cloudstack/pull/880
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-216325329
We need another code review. How should I test this, does it have to be a manual test?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by swill <gi...@git.apache.org>.
Github user swill commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-218356736
### CI RESULTS
```
Tests Run: 85
Skipped: 0
Failed: 3
Errors: 0
Duration: 6h 24m 57s
```
**Summary of the problem(s):**
```
FAIL: Test redundant router internals
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/git/cs1/cloudstack/test/integration/smoke/test_routers_network_ops.py", line 483, in test_02_RVR_Network_FW_PF_SSH_default_routes_egress_false
"Attempt to retrieve google.com index page should be successful once rule is added!"
AssertionError: Attempt to retrieve google.com index page should be successful once rule is added!
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_GDLRM8/results.txt
```
```
FAIL: test_02_vpc_privategw_static_routes (integration.smoke.test_privategw_acl.TestPrivateGwACL)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 253, in test_02_vpc_privategw_static_routes
self.performVPCTests(vpc_off)
File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 324, in performVPCTests
self.check_pvt_gw_connectivity(vm1, public_ip_1, vm2.nic[0].ipaddress)
File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 559, in check_pvt_gw_connectivity
"Ping to outside world from VM should be successful"
AssertionError: Ping to outside world from VM should be successful
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_GDLRM8/results.txt
```
```
FAIL: test_03_vpc_privategw_restart_vpc_cleanup (integration.smoke.test_privategw_acl.TestPrivateGwACL)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 265, in test_03_vpc_privategw_restart_vpc_cleanup
self.performVPCTests(vpc_off, True)
File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 331, in performVPCTests
self.check_pvt_gw_connectivity(vm1, public_ip_1, vm2.nic[0].ipaddress)
File "/data/git/cs1/cloudstack/test/integration/smoke/test_privategw_acl.py", line 559, in check_pvt_gw_connectivity
"Ping to outside world from VM should be successful"
AssertionError: Ping to outside world from VM should be successful
----------------------------------------------------------------------
Additional details in: /tmp/MarvinLogs/test_network_GDLRM8/results.txt
```
**Associated Uploads**
**`/tmp/MarvinLogs/DeployDataCenter__May_10_2016_19_39_49_I0OP3U:`**
* [dc_entries.obj](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/DeployDataCenter__May_10_2016_19_39_49_I0OP3U/dc_entries.obj)
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/DeployDataCenter__May_10_2016_19_39_49_I0OP3U/failed_plus_exceptions.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/DeployDataCenter__May_10_2016_19_39_49_I0OP3U/runinfo.txt)
**`/tmp/MarvinLogs/test_network_GDLRM8:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/test_network_GDLRM8/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/test_network_GDLRM8/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/test_network_GDLRM8/runinfo.txt)
**`/tmp/MarvinLogs/test_vpc_routers_WTWRQD:`**
* [failed_plus_exceptions.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/test_vpc_routers_WTWRQD/failed_plus_exceptions.txt)
* [results.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/test_vpc_routers_WTWRQD/results.txt)
* [runinfo.txt](https://objects-east.cloud.ca/v1/e465abe2f9ae4478b9fff416eab61bd9/PR880/tmp/MarvinLogs/test_vpc_routers_WTWRQD/runinfo.txt)
Uploads will be available until `2016-07-11 02:00:00 +0200 CEST`
*Comment created by [`upr comment`](https://github.com/cloudops/upr).*
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by koushik-das <gi...@git.apache.org>.
Github user koushik-das commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/880#discussion_r40532523
--- Diff: server/src/com/cloud/configuration/Config.java ---
@@ -1999,7 +1999,9 @@
// StatsCollector
StatsOutPutGraphiteHost("Advanced", ManagementServer.class, String.class, "stats.output.uri", "", "URI to additionally send StatsCollector statistics to", null),
- SSVMPSK("Hidden", ManagementServer.class, String.class, "upload.post.secret.key", "", "PSK with SSVM", null);
+ SSVMPSK("Hidden", ManagementServer.class, String.class, "upload.post.secret.key", "", "PSK with SSVM", null),
+
+ TemplatePreloaderPoolSize("Advanced", TemplateManager.class, Integer.class, "template.preloader.pool.size", "8", "Size of the TemplateManager threadpool", null);
--- End diff --
Use the mechanism described here to add a new configuration. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Configuration.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/880#discussion_r40543530
--- Diff: server/src/com/cloud/configuration/Config.java ---
@@ -1999,7 +1999,9 @@
// StatsCollector
StatsOutPutGraphiteHost("Advanced", ManagementServer.class, String.class, "stats.output.uri", "", "URI to additionally send StatsCollector statistics to", null),
- SSVMPSK("Hidden", ManagementServer.class, String.class, "upload.post.secret.key", "", "PSK with SSVM", null);
+ SSVMPSK("Hidden", ManagementServer.class, String.class, "upload.post.secret.key", "", "PSK with SSVM", null),
+
+ TemplatePreloaderPoolSize("Advanced", TemplateManager.class, Integer.class, "template.preloader.pool.size", "8", "Size of the TemplateManager threadpool", null);
--- End diff --
@koushik-das, changed the mechanism to use configuration.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on the pull request:
https://github.com/apache/cloudstack/pull/880#issuecomment-212418415
Rebased against master.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cloudstack pull request: CLOUDSTACK-8901: PrepareTemplate job thre...
Posted by SudharmaJain <gi...@git.apache.org>.
Github user SudharmaJain commented on a diff in the pull request:
https://github.com/apache/cloudstack/pull/880#discussion_r40756846
--- Diff: setup/db/db/schema-452to460.sql ---
@@ -413,3 +413,6 @@ CREATE TABLE `cloud`.`ldap_trust_map` (
UNIQUE KEY `uk_ldap_trust_map__domain_id` (`domain_id`),
CONSTRAINT `fk_ldap_trust_map__domain_id` FOREIGN KEY (`domain_id`) REFERENCES `domain` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+INSERT IGNORE INTO `cloud`.`configuration` VALUES ('Advanced', 'DEFAULT', 'TemplateManager', 'template.preloader.pool.size', '8', 'Size of the threadpool for preparetemplate api', '8', NULL, 'Global', 0);
--- End diff --
@koushik-das Implemented Configurable interface.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---