==> Regarding Re: [patch rfc] address regression in duplicate map entry order;
[EMAIL PROTECTED] adds:
raven> On Fri, 19 Nov 2004, Jeff Moyer wrote:
>> Hi, Ian, list,
>>
>> In my proposed fix, I meant to say:
>>
>> We could expire the cache in lookup_ghost, and then go ahead and only
>> insert new entries (instead of overwriting what was there).
>>
>> So, I've implemented this, though it isn't tested just yet. There were
>> some other bugs.
>>
>> In lookup_multi, we iterate through each map's lookup_ghost function.
>> The problem here is that you will end up doing the following:
>>
>> static int read_map(const char *root, struct lookup_context *ctxt) {
>> char key[KEY_MAX_LEN + 1]; char mapent[MAPENT_MAX_LEN + 1]; char
>> *mapname; FILE *f; int entry; time_t age = time(NULL);
>> <=================
>>
>> mapname = alloca(strlen(ctxt->mapname) + 6); sprintf(mapname, "file:%s",
>> ctxt->mapname);
>>
>> f = fopen(ctxt->mapname, "r"); if (!f) { error(MODPREFIX "could not open
>> map file %s", ctxt->mapname); return 0; }
>>
>> while(1) { entry = read_one(f, key, mapent); if (entry)
>> cache_update(root, key, mapent, age); <==========
>>
>> if (feof(f)) break; }
>>
>> fclose(f);
>>
>> /* Clean stale entries from the cache */ cache_clean(root, age);
>> <=============
>>
>> return 1; }
>>
>> Notice the lines I've pointed out. We decide what the expiration age
>> should be at the beginning of the function. We update entries and give
>> them that "age." And finally, we expire all entries that were made
>> prior to that. The problem arises in that we call the read_map routine
>> for each map in the list in order. What will happen is that each map,
>> as it is processed, will end up with a new value for age. Thus, we
>> expire all of the work done by the previous read_maps! This is bad. To
>> fix this, I've passed in the age field from the top-level.
>>
>> I've changed ldap's lookup_ghost routine (well, a function it calls,
>> anyway) to use cache_insert_unique instead of cache_add.
>>
>> Please let me know what you think. If you only like some parts, let me
>> know which ones and I'll break them out from the rest of the patch.
>> BTW, this patch is applied on top of your map expiration changes. I can
>> rediff the thing to any version of the code you want.
raven> Understand the problem.
raven> I've had a look at the patch and it looks good.
raven> I had a slightly different idea though.
raven> I think the cache_insert_unique might be a problem later as if
raven> people add multiple attributes (map entries) for multi-mount keys,
raven> instead of one attribute containing multiple mounts, in what they
raven> think is "the LDAP way" it probably won't work. I can't say we've
raven> seen this problem yet but I have been trying to allow for it in the
raven> code.
raven> The other thing that occured to me was that, together with passing
raven> the time to keep things consistent, processing the maps in reverse
raven> order in lookup_ghost of the multi map module would fix most of the
raven> issues. Along with modifying cache_add to append to any existing
raven> entries, rather than prepend.
raven> I spotted something you missed in the yp module. Have a look at the
raven> lookup_yp patch chunks in the attached map-entry-order patch.
raven> Have a look at what I've tried to do and let me know what you
raven> think. I've done some basic testing but not against the issue we are
raven> trying to fix. So let me know how it goes.
raven> Also, I found a couple of other bugs and have attached a couple of
raven> other patches as well.
Thanks, Ian! This all looks good to me, though I haven't been able to
test. What version of the tree was this applied to?
-Jeff
_______________________________________________
autofs mailing list
[EMAIL PROTECTED]
http://linux.kernel.org/mailman/listinfo/autofs
