You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ignite.apache.org by Vladimir Ozerov <vo...@gridgain.com> on 2015/02/04 10:40:29 UTC

"executorServiceShutdown" properties concept review.

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.

Re: "executorServiceShutdown" properties concept review.

Posted by Dmitriy Setrakyan <ds...@apache.org>.
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 <vo...@gridgain.com>
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.
>

Re: "executorServiceShutdown" properties concept review.

Posted by Dmitriy Setrakyan <ds...@apache.org>.
Yakov,

Can you explain how the need for the isXxxServiceShutdown() methods is
being addressed with the new thread pool configuration?

D.


On Wed, Feb 4, 2015 at 1:49 AM, Yakov Zhdanov <yz...@apache.org> wrote:

> Currently, configuration is being refactored and pools will be created
> internally only.
>
> --Yakov
>
> 2015-02-04 12:40 GMT+03:00 Vladimir Ozerov <vo...@gridgain.com>:
>
> > 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.
> >
>

Re: "executorServiceShutdown" properties concept review.

Posted by Yakov Zhdanov <yz...@apache.org>.
Currently, configuration is being refactored and pools will be created
internally only.

--Yakov

2015-02-04 12:40 GMT+03:00 Vladimir Ozerov <vo...@gridgain.com>:

> 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.
>