I agree that we definitely have too many properties for executor service
configuration. I also think that the only parameter a user may wish to
configure is the thread pool size and even that is extremely rare. In
majority of cases users will trust us to configure the thread pools
correctly.
How about this approach. We create thread configuration class with the
following methods:
---
IngniteThreadPoolConfiugration {
getThreadFactory(); // Optional factory, one for all the pools.
int getComputePoolSize(); // our public pool.
int getSystemPoolSize();
int getPeerDeploymentPoolSize();
int getConnectorPoolSize();
}
---
If user does not configure the above configuration, we should have defaults
for every pool size. Also, since we always create our own thread pools, we
are always responsible for shutting them down and do not need
isExecutorServiceShutdown() flag.
Thoughts?
D.
On Wed, Feb 4, 2015 at 1:40 AM, Vladimir Ozerov <[email protected]>
wrote:
> Hi,
>
> I'd like to start the discussion about infamous "shutdown" properties in
> configuration classes:
>
> - IgniteConfiguration.executorServiceShutdown()
> - IgniteConfiguration.ggfsExecutorServiceShutdown()
> - IgniteConfiguration.managementExecutorServiceShutdown()
> - IgniteConfiguration.peerClassLoadingExecutorServiceShutdown()
> - IgniteConfiguration.systemExecutorServiceShutdown()
> - IgniteConfiguration.restExecutorServiceShutdown()
> - ClientConnectionConfiguration.restExecutorServiceShutdown()
>
> This yields in 12 additional methods in IgniteConfiguration and 2 in
> ClientConnectionConfiguration.
> Reason why we have these properties is clear: if you starts Ignite in
> embedded mode and provides his own thread pool, he may want to keep it
> running even after Ignite node is stopped.
>
> On the other hand, it seems to me that custom executor service is pretty
> unusal and rare use case.
>
> What if we create wrapper class encapsulating both executor service and
> shutdown flag? E.g.:
> IgniteExecutorServiceConfiguration {
> ExecutorService getExecutorService();
> bool isExecutorServiceShutdown();
> }
>
> This will remove 6 properties. +19 deprecated client-related properties
> which are also will be removed. In summary, we will be able to remove 25
> properties (of 103 currently) from IgniteConfiguration.
>
> Any thoughts on this?
>
> Vladimir.
>
