Re: [Puppet-dev] Perf improvement

[DEGREMONT Aurelien]

Nobody cares?
This is a performance improvement which seems harmless.

Aur�lien

Le 17/12/2013 16:42, DEGREMONT Aurelien a �crit :
> Hello
>
> Working on upgrading from 0.25 to 3.x, I faced a regression regarding
> Service ensure behavior.
>
> In 0.25, this code, on a RedHat system:
>
> service { 'ntpd':
> enable => false,
> hasstatus => true;
> }
>
> would trigger:
>
> /sbin/chkconfig ntpd
>
> and NO:
>
> service ntpd status
>
> Starting from 2.6 and up to 3.x, both commands are run, the service
> status is checked (service ntpd status is run), even if ensure is not
> set (or set to "undef").
> On some systems, we have up to 50 services checked, and this is a huge
> performance impact on our Puppet runs to launch this command when we
> do not care.
> (some services could be quite long to be checked).
>
> I tracked down this behavior change up to this (old) patch:
>
>
> commit 3f6c9481ce9ac59fb3a84ab6792543d039ee403f
> Author: Luke Kanies <lu...@reductivelabs.com>
> Date: Tue Jan 19 18:24:15 2010 -0800
>
> Changing Transaction to use the new ResourceHarness
>
> This is a much messier commit than I would like,
> mostly because of how 'file' works. I had to
> fix multiple special cases, and I had to move others.
>
> The whole system appears to now work, though, and we're
> ready to change reports to receive resource status
> instances rather than events.
>
> Signed-off-by: Luke Kanies <lu...@reductivelabs.com>
>
>
> And I quickly workaround this using this oneliner patch:
>
> diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
> index 1933097..bf09fdb 100644
> --- a/lib/puppet/type.rb
> +++ b/lib/puppet/type.rb
> @@ -681,7 +681,7 @@ class Type
> # Provide the name, so we know we'll always refer to a real thing
> result[:name] = self[:name] unless self[:name] == title
>
> - if ensure_prop = property(:ensure) or
> (self.class.validattr?(:ensure) and ensure_prop = newattr(:ensure))
> + if ensure_prop = property(:ensure) or
> (self.class.validattr?(:ensure) and ensure_prop = newattr(:ensure))
> and ensure_prop.should
> result[:ensure] = ensure_state = ensure_prop.retrieve
> else
> ensure_state = nil
>
>
> What do you think of this?
>
>
> Aur�lien
>

[Felix Frank]

Well, I guess this is the sort of patch you want to apply to your own
boxen for the time being.

I believe it may take a while to reach a consensus about accepting it
upstream, because it changes behavior pretty deep inside the very core
of puppet, and the possible implications must be weighed carefully.

Cheers,
Felix

[DEGREMONT Aurelien]

That was why I posted this here, to see if there is some other
implications/complications/drawbacks to this change.


Aur�lien

Le 19/12/2013 12:05, Felix Frank a �crit :

[Adrien Thebo]

Your original email is actually under active discussion at https://groups.google.com/forum/#!topic/puppet-dev/P8ReCHvmd2o and in fact was updated 3 hours ago, I think we are trying to sort out those implications/complications/drawbacks in that thread. Are you able to view that conversation?

On Thu, Dec 19, 2013 at 3:14 AM, DEGREMONT Aurelien <aurelien....@cea.fr> wrote:

That was why I posted this here, to see if there is some other implications/complications/drawbacks to this change.

Aurélien

Le 19/12/2013 12:05, Felix Frank a écrit :

Well, I guess this is the sort of patch you want to apply to your own
boxen for the time being.

I believe it may take a while to reach a consensus about accepting it
upstream, because it changes behavior pretty deep inside the very core
of puppet, and the possible implications must be weighed carefully.

Cheers,
Felix

On 12/19/2013 12:02 PM, DEGREMONT Aurelien wrote:

Nobody cares?
This is a performance improvement which seems harmless.

Aurélien

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/52B2D4FB.4030800%40cea.fr.

For more options, visit https://groups.google.com/groups/opt_out.

--

Adrien Thebo | Puppet Labs

[DEGREMONT Aurelien]

Le 19/12/2013 17:32, Adrien Thebo a écrit :

Your original email is actually under active discussion at https://groups.google.com/forum/#!topic/puppet-dev/P8ReCHvmd2o and in fact was updated 3 hours ago, I think we are trying to sort out those implications/complications/drawbacks in that thread. Are you able to view that conversation?

I did view those e-mails and I'm reading them attentively. I'm eagerly waiting for the conclusion :)


Aurélien

On Thu, Dec 19, 2013 at 3:14 AM, DEGREMONT Aurelien <aurelien....@cea.fr> wrote:

That was why I posted this here, to see if there is some other implications/complications/drawbacks to this change.


Aurélien

Le 19/12/2013 12:05, Felix Frank a écrit :

Well, I guess this is the sort of patch you want to apply to your own
boxen for the time being.

I believe it may take a while to reach a consensus about accepting it
upstream, because it changes behavior pretty deep inside the very core
of puppet, and the possible implications must be weighed carefully.

Cheers,
Felix

On 12/19/2013 12:02 PM, DEGREMONT Aurelien wrote:

Nobody cares?
This is a performance improvement which seems harmless.

Aurélien

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/52B2D4FB.4030800%40cea.fr.

For more options, visit https://groups.google.com/groups/opt_out.

--

Adrien Thebo | Puppet Labs

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.

To unsubscribe from this group and stop receiving emails from it, send an email to puppet-dev+...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CALVJ9S%2B0tqFz%3DhyqNYSnsJ%2Buday-562nhZibYQ%2BfK0k9x4qUFQ%40mail.gmail.com.