On Mon, 22 Nov 2004, Jeff Moyer wrote:
> ==> 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?
That's a rather interesting question.
I originally made the patch against your 55.1 but couldn't get rpmbuild to
work OK under Gentoo so I added approiate patches to my Gentoo portage
install and built there. Only minor changes were needed, which I believe
would have been needed for the rpm as well.
Hopefully, it will also apply to earlier rpms without much trouble as the
changes appear to be mostly outside of the areas with lots of change.
Ian
_______________________________________________
autofs mailing list
[EMAIL PROTECTED]
http://linux.kernel.org/mailman/listinfo/autofs
