CLOUDSTACK-8901: PrepareTemplate job thread hard-coded to max 8 threads #880

SudharmaJain · 2015-09-24T06:22:25Z

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.

asfbot · 2015-09-24T06:27:09Z

cloudstack-pull-rats #713 SUCCESS
This pull request looks good

asfbot · 2015-09-24T08:02:18Z

cloudstack-pull-analysis #663 SUCCESS
This pull request looks good

asfbot · 2015-09-28T04:18:34Z

cloudstack-pull-rats #752 SUCCESS
This pull request looks good

asfbot · 2015-09-28T05:57:31Z

cloudstack-pull-analysis #703 SUCCESS
This pull request looks good

koushik-das · 2015-09-28T09:25:24Z

server/src/com/cloud/configuration/Config.java

-    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);


Use the mechanism described here to add a new configuration. https://cwiki.apache.org/confluence/display/CLOUDSTACK/Configuration.

@koushik-das, changed the mechanism to use configuration.

asfbot · 2015-09-28T11:56:24Z

cloudstack-pull-rats #760 SUCCESS
This pull request looks good

asfbot · 2015-09-28T13:22:32Z

cloudstack-pull-analysis #711 SUCCESS
This pull request looks good

koushik-das · 2015-09-29T11:04:28Z

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);


@SudharmaJain You have to implement the Configurable interface. Check for its usage in code.

asfbot · 2015-09-30T04:15:59Z

cloudstack-pull-rats #772 SUCCESS
This pull request looks good

asfbot · 2015-09-30T05:20:14Z

cloudstack-pull-analysis #724 SUCCESS
This pull request looks good

rohityadavcloud · 2015-10-10T13:43:04Z

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;
+


Please remove this empty line.

Removed the empty line.

rohityadavcloud · 2015-10-10T13:43:13Z

LGTM.

SudharmaJain · 2016-04-20T13:13:27Z

Rebased against master.

rohityadavcloud · 2016-04-20T15:21:16Z

Thanks @SudharmaJain LGTM (just code review), can you do a push -f (travis job failed for some reason)

SudharmaJain · 2016-04-21T03:45:07Z

@bhaisaab I pushed it again.

rohityadavcloud · 2016-05-02T10:38:53Z

LGTM

tag:easypr

Cc @swill

swill · 2016-05-02T18:48:38Z

We need another code review. How should I test this, does it have to be a manual test?

koushik-das · 2016-05-04T12:35:06Z

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 (logid:49186ba8) Start to preload template 201 into primary storage 2
INFO c.c.t.TemplateManagerImpl (logid:1ea23b56) Start to preload template 201 into primary storage 3

koushik-das · 2016-05-09T08:51:01Z

@swill This has the required LGTMs.

swill · 2016-05-09T22:06:36Z

Thanks @koushik-das. I will run this through CI just to be sure everything is good... Thx.

swill · 2016-05-11T04:07:22Z

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:

/tmp/MarvinLogs/test_network_GDLRM8:

/tmp/MarvinLogs/test_vpc_routers_WTWRQD:

Uploads will be available until 2016-07-11 02:00:00 +0200 CEST

Comment created by upr comment.

swill · 2016-05-11T04:08:25Z

These connectivity issues are unrelated to this PR. I will merge this now...

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. * pr/880: CLOUDSTACK-8901: PrepareTemplate job thread hard-coded to max 8 threads Signed-off-by: Will Stevens <williamstevens@gmail.com>

Co-authored-by: Pearl Dsilva <pearl.dsilva@shapeblue.com> Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

SudharmaJain force-pushed the cs-8901 branch from 49ed8ee to 785741e Compare September 28, 2015 04:15

koushik-das reviewed Sep 28, 2015
View reviewed changes

SudharmaJain force-pushed the cs-8901 branch from 785741e to b2f0c9c Compare September 28, 2015 11:53

koushik-das reviewed Sep 29, 2015
View reviewed changes

SudharmaJain force-pushed the cs-8901 branch from b2f0c9c to b610543 Compare September 30, 2015 04:05

rohityadavcloud reviewed Oct 10, 2015
View reviewed changes

SudharmaJain force-pushed the cs-8901 branch from b610543 to 48c9d39 Compare October 12, 2015 04:27

SudharmaJain force-pushed the cs-8901 branch from 48c9d39 to a7aad3f Compare April 20, 2016 13:12

CLOUDSTACK-8901: PrepareTemplate job thread hard-coded to max 8 threads

93ce108

SudharmaJain force-pushed the cs-8901 branch from a7aad3f to 93ce108 Compare April 21, 2016 03:43

asfgit merged commit 93ce108 into apache:master May 11, 2016

SudharmaJain deleted the cs-8901 branch May 17, 2016 04:18

rohityadavcloud pushed a commit that referenced this pull request Jan 20, 2021

storage: Close snapshot form in case of an error (#880)

db292da

Co-authored-by: Pearl Dsilva <pearl.dsilva@shapeblue.com> Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>

CLOUDSTACK-8901: PrepareTemplate job thread hard-coded to max 8 threads #880

CLOUDSTACK-8901: PrepareTemplate job thread hard-coded to max 8 threads #880

Uh oh!

Conversation

SudharmaJain commented Sep 24, 2015

Uh oh!

asfbot commented Sep 24, 2015

Uh oh!

asfbot commented Sep 24, 2015

Uh oh!

asfbot commented Sep 28, 2015

Uh oh!

asfbot commented Sep 28, 2015

Uh oh!

koushik-das Sep 28, 2015

Choose a reason for hiding this comment

Uh oh!

SudharmaJain Sep 28, 2015

Choose a reason for hiding this comment

Uh oh!

asfbot commented Sep 28, 2015

Uh oh!

asfbot commented Sep 28, 2015

Uh oh!

koushik-das Sep 29, 2015

Choose a reason for hiding this comment

Uh oh!

asfbot commented Sep 30, 2015

Uh oh!

asfbot commented Sep 30, 2015

Uh oh!

rohityadavcloud Oct 10, 2015

Choose a reason for hiding this comment

Uh oh!

SudharmaJain Oct 12, 2015

Choose a reason for hiding this comment

Uh oh!

rohityadavcloud commented Oct 10, 2015

Uh oh!

SudharmaJain commented Apr 20, 2016

Uh oh!

rohityadavcloud commented Apr 20, 2016

Uh oh!

SudharmaJain commented Apr 21, 2016

Uh oh!

rohityadavcloud commented May 2, 2016

Uh oh!

swill commented May 2, 2016

Uh oh!

koushik-das commented May 4, 2016

Uh oh!

koushik-das commented May 9, 2016

Uh oh!

swill commented May 9, 2016

Uh oh!

swill commented May 11, 2016

CI RESULTS

Uh oh!

swill commented May 11, 2016

Uh oh!

Uh oh!