On Tue, Apr 26, 2011 at 11:25, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Tue, Apr 26, 2011 at 12:35 AM, Greg Stein <gstein_at_gmail.com> wrote:
>> On Mon, Apr 25, 2011 at 21:06, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
>>>...
>>> Which kinda irks me, actually. You'd think something as simple as
>>> "ensure the permissions on disk match what's in the db" would be
>>> fairly straightforward, but it ends up have all kinds of special cases
>>> and various shortcuts. In some ways, I guess I'd settle for all of
>>> the logic being encapsulated in a single function/set-of-functions,
>>> rather than being spread across half-a-dozen functions and two files.
>>>
>>> As a first step, then, it'd be nice to know what special handling is
>>> needed for commit/update/revert, as opposed to propset. I'm happy to
>>> go digging, but any insights others have would be useful.
>>
>> I completely agree with this assessment. I ran into the same kind of
>> thing dealing with "translation" ... or "subst" ... depending on which
>> file or random function you happened to look at. Fall 2008, I narrowed
>> a lot of this down, but it still remains a bit broader of an API than
>> I would prefer. ... and setting file attributes is seemingly falling
>> into the same category. When working on that stuff, I certainly recall
>> finding stuff that would set those flags scattered around, under
>> different names, each for a specialized use.
>>
>> If you can rationalize them, then kudos. Would love to see it.
>>
>> Bert mentioned changing perms inadvertedly. Not sure that I read and
>> grok'd that entirely. But I do tend to agree with one of your
>> assessments: the file should always be sync'd with the "topmost"
>> props. Get that done first, and then worry about perf later. And
>> regarding perf: meh. I tend to believe the changes in the props
>> driving these things is *minimal*, so perf is going to be a moot point
>> in the typical case. Correctness, code understandability, and
>> determinism is the most important goal.
>
> I think I've got all the special cases and concerns isolated to a
> single function now. There are certainly a few performance
> improvements which could be made (grabbing all the data from the db in
> one go, rather than several queries), but the logic is all in the
> right spot, including a goofy special case for lock test 40.
>
> Bert's concern, as I understand it, is that before there were code
> paths in which we didn't touch the perms at all, but now we
> unconditionally clobber whatever perms were there with our own. I'm
> fine with that, actually, but I'm not sure what the performance
> implications are (I'd presume that OS caches would make the
> performance negligible, even on network filesystems).
I'm also fine with your approach. If/when perf problems are actually
found, then we can analyze the fix. Additional flags or functions,
*with a clear reason for their existence*, would be a fine solution in
my mind. I see the problem (before your changes) as having N functions
without any true clarity for why they exist. Deviating from your One
True Function with clear explanation should avoid going back to that
state.
Cheers,
-g
Received on 2011-04-26 17:31:55 CEST