On 14/03/10 08:48, Luke Kanies wrote:
> On Mar 13, 2010, at 1:57 PM, Brice Figureau wrote:
>
>> On 13/03/10 21:33, Rein Henrichs wrote:
>>> Brice,
>>>
>>> I think the "real" fix for the underlying problem is fixing the
>>> regression
>>> bug introduced in the file serving refactor.
>>
>> Indeed, but I think this patch is close to this.
>>
>>> That said, if this hot patch
>>> works around the 100% CPU usage on recursive chowns bug in the user list
>>> thread, I think it's desirable to get this into the 0.25.x branch ASAP.
>>
>> The real underlying issue is that file metadata terminus doesn't care
>> about the file {} checksum settings, it always compute the md5 checksum
>> of said files. Why? Because this is what is correct for remote files
>> metadatas.
>
> This actually shouldn't always need to be true - it's definitely
> unlikely that we wouldn't be interested in the remote md5, but it's at
> least possible.
OK, it makes sense, but I have hard time thinking (in the current puppet
system) when we wouldn't want to have remote metadatas including the
checksum?
>> Unfortunately the same code is called in the direct file serving
>> terminus system, where the checksum should not be computed (it will be
>> done by the checksum property).
>>
>> This patch is not 100% correct (but my minimal testing show that it
>> works), and I don't expect it to be merged directly in 0.25.x (on which
>> it is based).
>
> Why don't you expect that?
My sentence wasn't complete: it is missing an "as is".
When I wrote this I just had thought that directing the direct file
server to say to the fileset that they are local (and as such don't have
to compute a checksum) would be a better direction.
I must also add that I did only minimal testing of said patch (ie tried
only in the lab with various files (ie sourced, local, recursive). But
the file resource is so vast, I didn't check managing links, copying
files, etc, were still working.
>> It basically does two things:
>> * allow setting checksum => none
>> * make sure file metadata doesn't compute anything if checksum is none
>> (but only for the direct file server terminus).
>>
>> A more correct patch would be to prevent checksums to be computed in
>> local file metadata search.
>
> I suppose that would be sufficient - just have a 'local' flag,
> effectively, and an additional flag saying "i don't care about the
> checksum", but... then the latter becomes a superset of the former, so
> skip the former, use what you have here, and move on.
>
> Or am I missing something?
I don't think so, but I'm quite ignorant about the interaction between
the checksum file property and the rest of the system, so I want to make
sure I'm not introducing any new regressions.
--
Brice Figureau
My Blog: http://www.masterzen.fr/
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
