Puppet

Luke Kanies wrote:

We currently load files into memory rather than streaming them to disk. It’s much more memory efficient to stream them directly to disk.

Brice has an initial version of this, and it’d be great to get this in quickly.

We need an equivalent feature on the server — streaming files directly from disk.

IMHO, the server part is less needed (but would be nice to have if doable, especially using some X-Sendfile or X-Accel-Redirect system), as it is relatively easy to offload the puppet master file content serving to a static dedicated HTTP server (see: http://www.masterzen.fr/2010/01/28/puppet-memory-usage-not-a-fatality/ for an example).

I started working on the server side file streaming.

One point worth making – I think you could make the overall patch a lot easier if you just did a straight REST call from the File class. Given that you specifically don’t want the abstraction here, it makes sense to retrieve the content directly with http. It’ll probably be a small amount of duplicated code, but will overall make the system much cleaner.

What do you think?

Luke Kanies wrote:

One point worth making – I think you could make the overall patch a lot easier if you just did a straight REST call from the File class. Given that you specifically don’t want the abstraction here, it makes sense to retrieve the content directly with http. It’ll probably be a small amount of duplicated code, but will overall make the system much cleaner.

What do you think?

This will be definitely easier, and won’t involve this thread game I had to put in ContentStream only to overcome Net::HTTP stupideness. I’ll see how I can rework this.

Brice Figureau wrote:

Luke Kanies wrote:

One point worth making – I think you could make the overall patch a lot easier if you just did a straight REST call from the File class. Given that you specifically don’t want the abstraction here, it makes sense to retrieve the content directly with http. It’ll probably be a small amount of duplicated code, but will overall make the system much cleaner.

What do you think?

This will be definitely easier, and won’t involve this thread game I had to put in ContentStream only to overcome Net::HTTP stupideness. I’ll see how I can rework this.

Hmm, thinking a little bit more about it, it won’t help. What I think should be better is to have a way to pass a block to the “find” indirector which will yield the model instance (ie in our case an HTTP Response). Then rewrite Puppet::Type::File#write to call the source (and/or content for unification) property with a block which will write the given content. That looks better to me.

I’ll trust your instinct on what makes more sense, but why is it easier to modify the Indirector system than to do a relatively straightforward http call? What’s complicated about making the http call?

Luke Kanies wrote:

I’ll trust your instinct on what makes more sense, but why is it easier to modify the Indirector system than to do a relatively straightforward http call? What’s complicated about making the http call?

Nothing is complicated in firing http calls. What makes thing real complex is that the response persists only in the block of the network request.

I suggest we continue the conversation on puppet-dev in this thread: http://groups.google.com/group/puppet-dev/t/d64a13f6e05f0fcb

The patch has been reviewed.

A version taking into account comments review has been pushed to the tickets/testing/3373 branch in my github repository: http://github.com/masterzen/puppet/tree/tickets/testing/3373

This version is based on the latest reductivelabs/primordial_ooze branch which look like to be a testing minus the automated refactorings.

Pushed to master in commit"2cf7222df889981313c6955cc9220ce160dd90f6"