On Aug 29, 2013, at 12:24 PM, John Bollinger <john.bollin...@stjude.org> wrote:
>
>
> On Wednesday, August 28, 2013 5:56:45 PM UTC-5, Andy Parker wrote:
> On Wed, Aug 28, 2013 at 3:22 PM, Luke Kanies <lu...@puppetlabs.com> wrote:
> On Aug 28, 2013, at 12:38 PM, Andy Parker <an...@puppetlabs.com> wrote:
>> On Wed, Aug 28, 2013 at 10:20 AM, Luke Kanies <lu...@puppetlabs.com> wrote:
>> On Aug 28, 2013, at 8:45 AM, Andy Parker <an...@puppetlabs.com> wrote:
>>
>> > * #8040 - anchor pattern. I think a solution is in sight, but it didn't
>> > make 3.3.0 and it is looking like it might be backwards incompatible.
>>
>> Why would it be incompatible?
>>
>> That implies that we can't ship it until 4.0, which would be a tragedy
>> worth fighting hard to avoid.
>>
>>
>> The only possible problem, that I know of, would be that it would change the
>> evaluation order. Once things get contained correctly that might cause
>> problems. We never give very strong guarantees between versions of puppet,
>> but given the concern with manifest order, I thought that I would call this
>> out as well.
>
> Do you mean, for 2 classes that should have a relationship but currently
> don't because of the bug (and the lack of someone using an anchor pattern to
> work around the bug), fixing that bug would cause them to have a relationship
> and thus change the order?
>
>
> No that shouldn't be a problem. I think we will be using making the resource
> syntax for classes ( class { foo: } ) create the containment relationship.
> That doesn't allow multiple declarations and so we shouldn't encounter the
> problem of the class being in two places.
>
>
> But it does allow multiple declarations, so long as only the first one parsed
> uses the parameterized syntax. There can be any number of other places where
> class foo is declared via the include() function or require() function.
>
>
> That is, you're concerned that the bug has been around so long it's
> considered a feature, and thus we can't change it except in a major release?
>
>
> More of just that the class will start being contained in another class and
> so it will change where it is evaluated in an agent run. That could cause
> something that worked before to stop working (it only worked before because
> of random luck). I'm also, right now, wondering if there are possible
> dependency cycles that might show up. I haven't thought that one through.
>
>
> Yes, it is possible that dependency cycles could be created where none
> existed before. About a week ago I added an example to the comments thread
> on this issue; it is part of a larger objection to the proposed solution:
> http://projects.puppetlabs.com/issues/8040#note-35. I also included a
> proposed alternative solution that could go into Puppet 3.
As mentioned in my other email, the solution to this problem should not in any
way require changes to containment semantics, and certainly shouldn't require
class evaluation to indicate class containment. As I said, it used to do that
for the first instance (but not for second, which led to some inconsistencies
and surprises, which is why I removed it). These days, though, in general
classes only contain resources, not other classes. What I can't remember is
how inheritance affects containment, if at all.
Let's be clear on what the original problem was that led us to this:
Fundamentally, we only have a problem in a circumstance like this:
class a { file { "/a": ensure => present }
class b {}
class c { file { "/c": ensure => present }
Class[a] -> Class[b] -> Class[c]
Because Class[b] is empty, the relationship between the resources in A and the
resources in C get lost.
This is a bug that was introduced around 2.7 or 2.8, and it was introduced
because we made an optimization in the graph.
The optimization was needed because the current graph code can have either
containment or dependency edges in it, but not both. The catalog we ship from
the server has containment edges, and all dependencies are specified as
parameters, not edges in the graph.
The transaction creates what we call the relationship graph, which has those
dependency edges, but no containment edges. The way it used to accomplish this
was by essentially multiplying appropriate containment by appropriate
dependency edges. E.g., in this case:
class a {
file { ["/a1", "/a2"]: ensure => present }
} -> class b {
file { ["/b1", "/b2"]: ensure => present }
}
We end up turning our one dependency (Class[a] -> Class[b]) into 4 edges:
File[a1] -> File[b1]
File[a1] -> File[b2]
File[a2] -> File[b1]
File[a2] -> File[b2]
In small catalogs, this isn't really a problem, and might actually result in
fewer edges (e.g., we actually had 4 containment edges in the first graph, and
now have 4 dependency edges, so it's equal).
In large catalogs, though, this multiplication (and note I'm using that word
loosely; you can see the details in the 'splice' method on Catalog) results in
a massive increase in edges, which significantly slows things down.
The optimization done was to add whits as a single collector for those edges,
and thus reduce the overall count.
Unfortunately, this optimization introduced the bug we're all discussing, which
requires the anchor pattern.
The correct fix for this is relatively simple to understand, and should involve
the removal of quite a bit of code, although it might be a bit hard to
implement:
Change the graph handling code so it can correctly handle both dependency and
containment edges. This is relatively simple: Just change the traversal (and
cycle detection) to prefer dependencies over containment.
This way we don't have to multiply edges, which means we don't need whits
(yay!), which means we don't need anchors. Done. We also get to remove the
relationship graph and all related code, which is also a win.
I tried this about 2 years ago, but I couldn't get the cycle detection working.
I'm confident the team we have now can get it working, though, and Patrick and
I have talked through this such that I think he'll get it figured out.
(I'll add this to the ticket.)
--
Luke Kanies | http://about.me/lak | http://puppetlabs.com/ | +1-615-594-8199
--
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+unsubscr...@googlegroups.com.
To post to this group, send email to puppet-dev@googlegroups.com.
Visit this group at http://groups.google.com/group/puppet-dev.
For more options, visit https://groups.google.com/groups/opt_out.
