You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by revans2 <gi...@git.apache.org> on 2017/01/05 20:08:47 UTC
[GitHub] storm pull request #1862: STORM-2278: Allow max number of disruptor queue fl...
GitHub user revans2 opened a pull request:
https://github.com/apache/storm/pull/1862
STORM-2278: Allow max number of disruptor queue flusher threads to be configurable
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/revans2/incubator-storm STORM-2278
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/storm/pull/1862.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 #1862
----
commit 753b1cbca44ff26784eee8f1a0a5cbce5a1b97d6
Author: Robert (Bobby) Evans <ev...@yahoo-inc.com>
Date: 2017-01-05T20:08:02Z
STORM-2278: Allow max number of disruptor queue flusher threads to be configurable
----
---
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] storm issue #1862: STORM-2278: Allow max number of disruptor queue flusher t...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1862
+1 Thanks for the fix.
---
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] storm pull request #1862: STORM-2278: Allow max number of disruptor queue fl...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1862#discussion_r95174977
--- Diff: storm-core/src/jvm/org/apache/storm/utils/DisruptorQueue.java ---
@@ -63,6 +63,18 @@
private static final String PREFIX = "disruptor-";
private static final FlusherPool FLUSHER = new FlusherPool();
+ private static int getNumFlusherPoolThreads() {
+ int numThreads = 100;
+ try {
+ String threads = System.getProperty("num_flusher_pool_threads", "100");
--- End diff --
The issue with using a Config for this is that readStormConfig inside the worker would read the system config, not the topology config, and would not let us override it on a per topology bases. I will add in a config for the system default, but in the documents there indicate the system property that can be set to override it for a topology.
---
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] storm issue #1862: STORM-2278: Allow max number of disruptor queue flusher t...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on the issue:
https://github.com/apache/storm/pull/1862
@HeartSaVioR I rebased and I think I addressed your review comments. Please have a look and see if it is what you wanted.
---
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] storm pull request #1862: STORM-2278: Allow max number of disruptor queue fl...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:
https://github.com/apache/storm/pull/1862#discussion_r95096103
--- Diff: storm-core/src/jvm/org/apache/storm/utils/DisruptorQueue.java ---
@@ -63,6 +63,18 @@
private static final String PREFIX = "disruptor-";
private static final FlusherPool FLUSHER = new FlusherPool();
+ private static int getNumFlusherPoolThreads() {
+ int numThreads = 100;
+ try {
+ String threads = System.getProperty("num_flusher_pool_threads", "100");
--- End diff --
Rethinking this a bit: this will be going to be undocumented feature.
Given that we don't reuse this for outside of Storm project, how about adding property to Config and calling Utils.readStormConfig() to read the value?
---
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] storm issue #1862: STORM-2278: Allow max number of disruptor queue flusher t...
Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on the issue:
https://github.com/apache/storm/pull/1862
+1
Build failure on storm-core is not related.
---
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] storm pull request #1862: STORM-2278: Allow max number of disruptor queue fl...
Posted by revans2 <gi...@git.apache.org>.
Github user revans2 commented on a diff in the pull request:
https://github.com/apache/storm/pull/1862#discussion_r95169637
--- Diff: storm-core/src/jvm/org/apache/storm/utils/DisruptorQueue.java ---
@@ -63,6 +63,18 @@
private static final String PREFIX = "disruptor-";
private static final FlusherPool FLUSHER = new FlusherPool();
+ private static int getNumFlusherPoolThreads() {
+ int numThreads = 100;
+ try {
+ String threads = System.getProperty("num_flusher_pool_threads", "100");
--- End diff --
I agree that it does not add a lot of value in it's current form. If you want me to document it I am happy to.
---
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] storm pull request #1862: STORM-2278: Allow max number of disruptor queue fl...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/storm/pull/1862
---
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.
---