+1 to all of this. It all makes sense and I think it'll solve all of the use cases that I can think of.
On Wed, Dec 17, 2014 at 8:34 PM, Michael Smith <[email protected] > wrote: > > Ok, after some discussions with Josh and Andy (Andy's below), came up with > a proposal for how one might write a stash for re-using data. Just for > clarification, in what sense do you mean a 'queueing' mechanism? > > Create a Stash class of some sort, probably in Puppet::Util, that's a > simple key/value store. That class can be instantiated in specific > resources where it's needed, assuming the resource is a class with a > sufficiently long lifetime. We can also instantiate a global stash, which > is created in lib/puppet/configurer.rb as part of push_context when we're > setting up a run. The Stash class could have a static member that's queried > to get the global version in push_context (if it's available); the parsed > data from /proc/mounts can be added to the context instance of the Stash. > > Andy and my discussion on #puppet-dev today: > >> [16:43:15] *<MichaelSmith>* *+zaphod42*: There's a mailing list thread >> on PUP-3116 that tries to cache the result of reading /prod/mounts >> [16:44:06] *<MichaelSmith>* I'm trying to explore whether there are any >> existing patterns for caching data we re-use during a catalog run. >> [16:45:05] *<MichaelSmith>* Puppet::Util::Storage kind of covers that, >> with the added benefit of logging the cached data, but also the cost of >> writing to PuppetDB. >> [16:46:02] *<MichaelSmith>* And also doesn't work with puppet apply, so >> that's problematic. >> [16:46:51] *<+zaphod42>* Puppet::Util::Storage writes to puppetdb? I >> thought it just wrote to a local file >> [16:47:40] *<+zaphod42>* I think henrik's concern about memory leaks >> really just is about the problems we encounter when the cache is never >> flushed >> [16:47:58] *<+zaphod42>* the data really just needs to have a clear >> lifetime >> [16:48:09] *<MichaelSmith>* Oh, I may be confused about >> Puppet::Util::Storage then. >> [16:48:31] *<+zaphod42>* and based on what I'm seeing, is this really a >> cache? or is it really just about having some "stash" where providers can >> store data during a run? >> [16:49:28] *<MichaelSmith>* It would potentially be refreshed if the >> /proc/mounts gets updated, but that's up to the provider. So just a stash >> makes sense. >> [16:49:37] *<+zaphod42>* MichaelSmith: yeah, Storage just writes to a >> local file >> https://github.com/puppetlabs/puppet/blob/master/lib/puppet/util/storage.rb#L86 >> [16:50:36] *<MichaelSmith>* Is using Storage to stash data used during a >> run something that's been discouraged in the past? >> [16:50:44] *<+zaphod42>* MichaelSmith: in which case, I would think >> about it as providing a "stash" method for providers. A very simple thing >> would be it just returns a hash that can be manipulated by the provider >> [16:50:55] *<+zaphod42>* the hash needs to be stored somewhere >> [16:51:15] *<+zaphod42>* that can be handled by the Transaction and it >> can just throw all of the contents away at the end of a run >> [16:51:54] *<MichaelSmith>* Yeah, sounds like a reasonable API to write. >> Puppet::Util::Stash, that's cleared after a run and only stored in-memory. >> [16:51:57] *<+zaphod42>* there is also the question about what is the >> scope of the data. Does just one resource get to see its own data, is it >> shared across all resources of the same provider, all of the same type, or >> all of the same run >> [16:52:45] *<MichaelSmith>* Do you have ideas how to enforce those types >> of restrictions? >> [16:53:43] *<+zaphod42>* Have different stashes for each set? So for >> every resource it has its own stash, the type has a stash, and the >> transaction has a stash and they are all accessed independently >> [16:54:14] *<+zaphod42>* the biggest problem is threading it through the >> APIs. Ideally they would be something that fits in nicely, but I have a >> feeling it will just be another global somewhere >> [16:54:52] *<MichaelSmith>* I think the tricky part becomes how to clear >> them when we have many isolated stashes. >> [16:54:59] *<MichaelSmith>* So they have to register themselves globally >> somewhere. >> [16:56:05] *<+zaphod42>* or they live as instance variables on some >> objects that get thrown away >> [16:56:18] *<+zaphod42>* so the resource stash is just an instance >> variable on a resource >> [16:56:26] *<+zaphod42>* provider stash is on a provider >> [16:56:41] *<+zaphod42>* (there is a problem there that every resource >> is an instance of a provider) >> [16:56:52] *<+zaphod42>* there isn't a shared provider instance across >> the resources >> [16:58:13] *<+zaphod42>* so one way to do it is have a Stashs object >> that is pushed into the context by the transaction and popped when the >> transaction is done >> [16:58:32] *<MichaelSmith>* This particular example is being used in a >> type, and I don't yet see where it creates a persistent instance object. >> The lifetime might be too short to be useful. >> [16:58:39] *<+zaphod42>* the stashes object holds all of the stashes for >> all of the resources, types, etc (whatever scopes are deemed correct) >> [16:59:18] *<+zaphod42>* in a type....Types are tricky because they are >> shared between the master and the agent >> [17:01:44] *<MichaelSmith>* I'm not quite sure of the implications of >> that. I guess that means lifetime on the master is different. >> [17:05:37] *<+zaphod42>* yeah, how types are used on the master versus >> the agent is different. I can't ever remember all of the details though >> [17:06:40] *<+zaphod42>* but if you put all of the stashes in a Stashes >> instance and put that instance in the Context and then use context_push (or >> better context_override), then it should be fine and not have a memory leak >> [17:07:15] *<+zaphod42>* however, it will end up holding onto data >> during a transaction longer than it may need to, thus increasing memory >> usage >> [17:07:23] *<+zaphod42>* but I'm not sure how much of a problem that >> would be >> [17:07:37] *<+zaphod42>* so long as there is some point at which the >> objects will be cleaned up >> [17:08:01] *<MichaelSmith>* Is there any advantage of having a Stashes >> instance that's added via push_context, vs just pushing your hash directly >> to it? >> [17:08:22] *<MichaelSmith>* I guess the ability to add arbitrary keys >> after starting. >> [17:08:44] *<+zaphod42>* push_context would just be where some >> collection of stashes would be held and other things can get to (a global, >> but with more control) >> [17:09:12] *<+zaphod42>* you should still provide an API on the >> resources to get to the stashes, instead of having authors go directly to >> Puppet.lookup >> [17:09:29] *<MichaelSmith>* Yeah, makes sense. >> [17:09:55] *<+zaphod42>* and the other part of the context is that it >> controls the lifetime of the stashes >> [17:10:16] *<+zaphod42>* once the context is popped, the stashes >> disappear >> [17:10:51] *<+zaphod42>* I'd much rather have instances of resources and >> such hold onto their own stashes, but it might be difficult >> [17:11:28] *<+zaphod42>* however, I think you should look into that. >> Only use the context system if there isn't a more local way of controlling >> it >> [17:11:33] *<MichaelSmith>* Yeah... not everything seems to have an >> instance. >> [17:12:13] *<+zaphod42>* which is the sad making part :( > > > On Wed, Dec 17, 2014 at 3:53 PM, Michael Smith < > [email protected]> wrote: >> >> I'm doing my own digging to figure out what seems to make sense. >> >> Josh had mentioned Puppet::push_context, set in the configurer. We push >> and pop context for each apply run; however that's a private API that >> doesn't seem to be meant for general use. Piggybacking on it looks like it >> would get messy. >> >> There's also Puppet::Util::Storage, which superficially looks appropriate >> for this kind of caching ( >> http://www.rubydoc.info/gems/puppet/Puppet/Util/Storage). I'm still >> trying to wrap my head around what side-effects might occur. >> >> >> On Tue, Dec 16, 2014 at 6:27 PM, Trevor Vaughan <[email protected]> >> wrote: >>> >>> Part of my other heartburn with using a file was revisited hard upon me >>> as I recalled the original extdata function implementation. >>> >>> In the case of extdata, one large extdata file + a lot of extlookups = >>> massive catalog compile times on the server. >>> >>> So, every time I want to call the cache, across potentially large >>> numbers of providers and/or other things requiring state, I *really* don't >>> want to read a file. Particularly, when I don't know what's going to be in >>> it. >>> >>> In this case, we would have to contend with slower client run times and >>> more CPU overhead as well as disk I/O requirements. Indicating that people >>> should change the way their OS is configured inasmuch as using tmpfs when >>> they may not have this choice does not seem ideal unless, of course, it >>> ships with puppet and doesn't require a system reboot. If, for some reason, >>> I have 50 providers that want to use this, this is 50 file reads and writes >>> that could be avoided. >>> >>> Giving people the choice of Disk vice Memory overhead would be ideal if >>> you want both for some reason. >>> >>> I'm honestly not seeing what would be so bad about scope.cache where >>> cache is some top level Puppet::Cache object that holds hashes that expire >>> at the end of a run. You would have to do things very politely in terms of >>> namespacing but you have to do that anyway. >>> >>> I am, of course, not opposed to saving cache state to disk for debugging >>> purposes, and think that should be an option when the --debug flag is used. >>> >>> Trevor >>> >>> Trevor >>> >>> On Tue, Dec 16, 2014 at 7:37 PM, Felix Frank < >>> [email protected]> wrote: >>>> >>>> Hey, >>>> >>>> good points - state retention at whatever granular level would be a >>>> good general purpose tool to have. If it's built in a pervasive fashion >>>> (i.e., any provider might use the cache for whetever it deems appropriate), >>>> it gains added visibility and becomes more opaque to the user - which is a >>>> good thing, and addresses one of the major concerns I'm having with this. >>>> The other being that it needs to be tunable for the user in some fashion. >>>> >>>> I have no qualms about disk I/O - after all, the user can choose >>>> whatever block backend they want. Users who depend on low latency or need >>>> to save IOPS can employ a tmpfs, for example. >>>> >>>> Cheers, >>>> Felix >>>> >>>> On 12/17/2014 12:56 AM, Trevor Vaughan wrote: >>>> >>>> I'm happy with catalog lifetime. >>>> >>>> I'm really not happy with doing anything that involves disk I/O. >>>> >>>> This would be key to getting providers to be able to save state in a >>>> non-hacky way as well. >>>> >>>> Trevor >>>> >>>> On Tue, Dec 16, 2014 at 6:45 PM, Michael Smith < >>>> [email protected]> wrote: >>>>> >>>>> I don't like any of the ideas I raised, but this will take some >>>>> digging. We need to determine what life-time the cache should have, and >>>>> what interface. I'm leaning towards either a cached read API in the >>>>> FileSystem utilities, or a cache tied to the catalog lifetime. >>>>> >>>> >>>> -- >>>> 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 [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/puppet-dev/5490D048.7020702%40Alumni.TU-Berlin.de >>>> <https://groups.google.com/d/msgid/puppet-dev/5490D048.7020702%40Alumni.TU-Berlin.de?utm_medium=email&utm_source=footer> >>>> . >>>> >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> >>> >>> -- >>> Trevor Vaughan >>> Vice President, Onyx Point, Inc >>> (410) 541-6699 >>> [email protected] >>> >>> -- This account not approved for unencrypted proprietary information -- >>> >>> -- >>> 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 [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/d/msgid/puppet-dev/CANs%2BFoUCo4FmT9QGk_P1kYg0CdEWA9pqhU%3D6jeXjBAr9z7fD9w%40mail.gmail.com >>> <https://groups.google.com/d/msgid/puppet-dev/CANs%2BFoUCo4FmT9QGk_P1kYg0CdEWA9pqhU%3D6jeXjBAr9z7fD9w%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- > 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 [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/puppet-dev/CABy1mMJigXCzOi1P1wD4G8kb6Ec3gS3y%2Bw_aANpkdu5s2gOWkw%40mail.gmail.com > <https://groups.google.com/d/msgid/puppet-dev/CABy1mMJigXCzOi1P1wD4G8kb6Ec3gS3y%2Bw_aANpkdu5s2gOWkw%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- Trevor Vaughan Vice President, Onyx Point, Inc (410) 541-6699 [email protected] -- This account not approved for unencrypted proprietary information -- -- 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 [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/puppet-dev/CANs%2BFoV9hwQFm8GO7Oxt8VjpDu%2BxDS24z4nSj1LPDo4hkmDTcA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
