Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use ExecutorService.newFixedThreadPool for LeaseRenewer #135

Merged
merged 1 commit into from Feb 24, 2017

Conversation

slam
Copy link
Contributor

@slam slam commented Feb 15, 2017

The existing ThreadPoolExecutor was misconfigured and caused
the thread pool to recycle idle threads continuously. VisualVM
showed that a thread got recycled about every 3 seconds, creating
and destroying several hundred threads in about 15 minutes.

Use ExecutorService.newFixedThreadPool instead, as recommended
by the javadoc for ExecutorService.

The existing ThreadPoolExecutor was misconfigured and caused
the thread pool to recycle idle threads continuously. VisualVM
showed that a thread got recycled about every 3 seconds, creating
and destroying several hundred threads in about 15 minutes.

Use ExecutorService.newFixedThreadPool instead, as recommended
by the javadoc for ExecutorService.
@slam
Copy link
Contributor Author

slam commented Feb 15, 2017

Here is a VisualVM screenshot of one of our production consumers that had been running for about 10 hours. Note the LeaseRenewer thread number went to about 11000...

image

@slam
Copy link
Contributor Author

slam commented Feb 15, 2017

With this fix, the LeaseRenewer thread count stays at 20.

@pfifer
Copy link
Contributor

pfifer commented Feb 17, 2017

Thanks for sending this. I'm taking a look at this now.

@pfifer
Copy link
Contributor

pfifer commented Feb 17, 2017

Before I forget, please confirm that we can use, modify, copy, and redistribute this contribution.

Thanks.

@slam
Copy link
Contributor Author

slam commented Feb 17, 2017

Yes, confirmed.

@pfifer pfifer added this to the Release 1.7.4 milestone Feb 24, 2017
@pfifer pfifer merged commit 9113ae2 into awslabs:master Feb 24, 2017
@slam slam deleted the lease-renewer-threadpool branch February 24, 2017 14:23
@joelittlejohn
Copy link

@pfifer Any chance of a new release to include this fix sometime soon? It's a reasonably serious issue that would be worth a release IMO.

pfifer added a commit to pfifer/amazon-kinesis-client that referenced this pull request Feb 27, 2017
* Fixed an issue building JavaDoc for Java 8.
  * [Issue awslabs#18](awslabs#18)
  * [PR awslabs#141](awslabs#141)
* Reduce Throttling Messages to WARN, unless throttling occurs 6 times consecutively.
  * [Issue awslabs#4](awslabs#4)
  * [PR awslabs#140](awslabs#140)
* Fixed two bugs occurring in requestShutdown.
  * Fixed a bug that prevented the worker from shutting down, via requestShutdown, when no leases were held.
    * [Issue awslabs#128](awslabs#128)
  * Fixed a bug that could trigger a NullPointerException if leases changed during requestShutdown.
    * [Issue awslabs#129](awslabs#129)
  * [PR awslabs#139](awslabs#139)
* Upgraded the AWS SDK Version to 1.11.91
  * [PR awslabs#138](awslabs#138)
* Use an executor returned from `ExecutorService.newFixedThreadPool` instead of constructing it by hand.
  * [PR awslabs#135](awslabs#135)
* Correctly initialize DynamoDB client, when endpoint is explicitly set.
  * [PR awslabs#142](awslabs#142)
pfifer added a commit that referenced this pull request Feb 27, 2017
* Fixed an issue building JavaDoc for Java 8.
  * [Issue #18](#18)
  * [PR #141](#141)
* Reduce Throttling Messages to WARN, unless throttling occurs 6 times consecutively.
  * [Issue #4](#4)
  * [PR #140](#140)
* Fixed two bugs occurring in requestShutdown.
  * Fixed a bug that prevented the worker from shutting down, via requestShutdown, when no leases were held.
    * [Issue #128](#128)
  * Fixed a bug that could trigger a NullPointerException if leases changed during requestShutdown.
    * [Issue #129](#129)
  * [PR #139](#139)
* Upgraded the AWS SDK Version to 1.11.91
  * [PR #138](#138)
* Use an executor returned from `ExecutorService.newFixedThreadPool` instead of constructing it by hand.
  * [PR #135](#135)
* Correctly initialize DynamoDB client, when endpoint is explicitly set.
  * [PR #142](#142)
@pfifer
Copy link
Contributor

pfifer commented Feb 27, 2017

This should be released a little later today if all my testing goes right. I've been testing these changes for a while so I don't expect any real issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants