You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Andrew Gaul <no...@github.com> on 2014/02/10 22:50:40 UTC

[jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Previously jclouds could use an unlimited number of threads on its
user ExecutorService.  While this ExecutorService will go away when we
complete deasyncafication, we should prevent jclouds from misbehaving
until that time.
You can merge this Pull Request by running:

  git pull https://github.com/maginatics/jclouds max-user-threads

Or you can view, comment on it, or merge it online at:

  https://github.com/jclouds/jclouds/pull/287

-- Commit Summary --

  * JCLOUDS-459: Limit maximum user threads

-- File Changes --

    M core/src/main/java/org/jclouds/apis/internal/BaseApiMetadata.java (2)

-- Patch Links --

https://github.com/jclouds/jclouds/pull/287.patch
https://github.com/jclouds/jclouds/pull/287.diff

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by Andrew Phillips <no...@github.com>.
> @@ -66,7 +66,7 @@ public static Properties defaultProperties() {
>        props.setProperty(PROPERTY_SO_TIMEOUT, 60000 + "");
>        props.setProperty(PROPERTY_CONNECTION_TIMEOUT, 60000 + "");
>        props.setProperty(PROPERTY_IO_WORKER_THREADS, 20 + "");
> -      props.setProperty(PROPERTY_USER_THREADS, 0 + "");
> +      props.setProperty(PROPERTY_USER_THREADS, 50 + "");

Just out of interest - why 50? And have we seen any concrete examples of jclouds misbehaving?

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287/files#r9623378

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by Andrew Gaul <no...@github.com>.
> @@ -66,7 +66,7 @@ public static Properties defaultProperties() {
>        props.setProperty(PROPERTY_SO_TIMEOUT, 60000 + "");
>        props.setProperty(PROPERTY_CONNECTION_TIMEOUT, 60000 + "");
>        props.setProperty(PROPERTY_IO_WORKER_THREADS, 20 + "");
> -      props.setProperty(PROPERTY_USER_THREADS, 0 + "");
> +      props.setProperty(PROPERTY_USER_THREADS, 50 + "");

I have used 50 in my application for a few years and this seems like a reasonable number compared to the 20 IO threads.  I have witnessed excessive thread creation during ```BlobStore.clearContainer``` which issues submits as many operations as list returns and creating an unbounded number of threads as a result.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287/files#r9635706

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by Andrew Gaul <no...@github.com>.
Closed #287.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by Andrew Phillips <no...@github.com>.
> @@ -66,7 +66,7 @@ public static Properties defaultProperties() {
>        props.setProperty(PROPERTY_SO_TIMEOUT, 60000 + "");
>        props.setProperty(PROPERTY_CONNECTION_TIMEOUT, 60000 + "");
>        props.setProperty(PROPERTY_IO_WORKER_THREADS, 20 + "");
> -      props.setProperty(PROPERTY_USER_THREADS, 0 + "");
> +      props.setProperty(PROPERTY_USER_THREADS, 50 + "");

> I have used 50 in my application for a few years and this seems like a reasonable number compared to the 20 IO 
> threads.

Ah, thanks. Care to add a code comment like `// works well with the 20 I/O threads`? But also fine without.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287/files#r9649110

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by Andrew Phillips <no...@github.com>.
Thanks for the explanation, @andrewgaul! Code comment to explain the value of 50 would be nice, but +1 from me without that, too.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287#issuecomment-34821878

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1058](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1058/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287#issuecomment-34694397

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by Andrew Gaul <no...@github.com>.
Committed to master and 1.7.x.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287#issuecomment-35528615

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #586](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/586/) SUCCESS
This pull request looks good

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287#issuecomment-34694272

Re: [jclouds] JCLOUDS-459: Limit maximum user threads (#287)

Posted by Andrew Gaul <no...@github.com>.
> @@ -66,7 +66,7 @@ public static Properties defaultProperties() {
>        props.setProperty(PROPERTY_SO_TIMEOUT, 60000 + "");
>        props.setProperty(PROPERTY_CONNECTION_TIMEOUT, 60000 + "");
>        props.setProperty(PROPERTY_IO_WORKER_THREADS, 20 + "");
> -      props.setProperty(PROPERTY_USER_THREADS, 0 + "");
> +      props.setProperty(PROPERTY_USER_THREADS, 50 + "");

Done.

---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/287/files#r9875469