The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Feature #6911

Flexible, deterministic, unguessable application order

Added by Markus Roberts over 3 years ago. Updated over 3 years ago.

Status:ClosedStart date:03/30/2011
Priority:NormalDue date:
Assignee:Markus Roberts% Done:

0%

Category:-
Target version:2.7.0
Affected Puppet version: Branch:MarkusQ:feature/next/resource_application_order
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

We would like to change the order in which resources are applied to meet a very specific set of constraints:

  1. The order should permit responsive adaptation to externalities (graph frontiers)
  2. The order in which resources that do not depend on externalities should be consistent (deterministic) from run to run provided that no structural changes occur in the manifest
  3. Changes in the ordering due to manifest changes should be limited in scope and correspond to the areas that were changed (that is, adding/removing/changing one resource should not affect the relative order of any unrelated pair of resources)
  4. It should not be possible to guess the ordering a priori — this mechanism is not a substitute for declaring dependencies.
  5. It should not violate any implicit or explicit dependencies

Related issues

Related to Puppet - Bug #6944: generate_additional_resources in transaction drops recurs... Closed 04/02/2011
Related to Puppet - Bug #3788: subscribe overrides tags Rejected 05/18/2010
Related to Puppet - Bug #5351: Puppet checksums file resources when no source specified Accepted 11/18/2010
Related to Puppet - Bug #5414: file resource can't be used to monitor file changes recur... Accepted 11/30/2010
Related to Puppet - Bug #5876: Require and Subscribe on the same refreshonly exec doesnt... Needs More Information 01/13/2011
Related to Puppet - Bug #6020: scheduling ignored for tidy Accepted 01/26/2011
Related to Puppet - Bug #6810: File 'fail' became a lot more fatal in 2.6.0 Closed 03/22/2011
Related to Puppet - Bug #5200: Many-to-many implementation of stages needlessly inefficient Closed 11/03/2010

History

#1 Updated by R.I. Pienaar over 3 years ago

Hello,

This is a good summary, I’d like to add to it:

  • Remove parser functions like defined()
  • Remove access to the list of loaded classes in scope
  • Remove access to list of tags in the scope

Really remove all the bits of the language that remains weirdly order broken and that simply cannot work at all till the futures work is completed at which point their design should be re-evaluated.

#2 Updated by Markus Roberts over 3 years ago

R.I. it’s late Friday afternoon, and I’m being a little dense. Could you elaborate on how the points you mentioned are related to this ticket? I’m deep enough in the trees on this that I suspect I’m not always seeing the forest.

#3 Updated by Markus Roberts over 3 years ago

  • Branch set to MarkusQ:feature/next/resource_application_order

Core change up for review. Note that is not a resolution, just a key way-point on the route to a resolution.

(6911) Core change -- replace topsort with frontier ordered by salted SHA1

This is the core change of the ticket; rather than using a topological sort to statically
determine the order in which resources should be applied, we use the graph wrapper
introduced in the prior commit to dynamically determine the order in which to apply
resources based on 1) the status of the resource (ready, done) 2) the explicit &
implied dependencies, 3) the salted SHA1 of the title (for stability).

Further work is needed:

1) Resolving the handling of failed resources
2) Tests of the new behavior, to the extent possible
3) Newly-dead-code removal in simple_graph & transaction
4) Fix the name-prefix ordering hack in eval_generate by either:
    a) Moving the logic into file
    b) Refactoring Type#eval_generate to return a tree
    c) ....?
5) Rough performance testing to look for hotspots
6) Investigation of possible interaction with #3788, #5351, #5414, #5876, #6020, #6810,
   and #6944 which may simplify or complicate their resolution.

Paired-with: Jesse Wolfe

#4 Updated by Markus Roberts over 3 years ago

Note that the list of related tickets includes all those that people have raised concerns/suspicions about in the context of this change; we will probably wind up pairing it down on further investigation.

#5 Updated by R.I. Pienaar over 3 years ago

Markus Roberts wrote:

R.I. it’s late Friday afternoon, and I’m being a little dense. Could you elaborate on how the points you mentioned are related to this ticket? I’m deep enough in the trees on this that I suspect I’m not always seeing the forest.

As we’re overhauling how ordering works and doing lots of work in this area to avoid certain pitfalls I thought i’d mention the other closely related ones to ordering that we are not addressing – but that doesnt really work at the moment.

defined() is parsing order dependent for example so it just doesn’t do what its advertised to do. I realize this ticket is more about sorting the graph than it is about parsing ordering.

Feel free to take it as an aside or punt it to other tickets etc, I just hate the way functionality that exist now doesnt work as a user might expect and its ordering related and we’re not fixing that functionality. I’d favor removing broken functions then.

#6 Updated by Markus Roberts over 3 years ago

Ah. Yes. I 100% agree, but that’s more tied to the server-side futures stuff than to the client-side graph walking. I’d love to tackle that as well, but I’ll take progress on any front I can.

#7 Updated by Markus Roberts over 3 years ago

The branch now contains a more complete draft of the solution with one known bug (event propagation through container boundaries).

#8 Updated by Jesse Wolfe over 3 years ago

  • Status changed from Accepted to Closed

Merged to next as of commit:e17cc651a9625576aa79af428bbaec702e216ac8

#9 Updated by James Turnbull over 3 years ago

  • Target version changed from 2.7.x to 2.7.0

Also available in: Atom PDF