Open Stack

On Fri, 26 Oct 2012 16:35:38 +0900
Yamagata Yo <bi.yamagata at gmail.com> wrote:

> > necessary to have the expiration period in your patch. Why not
> > update the hashes as soon as the loss is found in any given suffix?
> 
> Without expiration period, object-auditor update try to all suffix hash values
> and it will take too long time for object-auditor, I guess.
> [...]
> But if without this functionality, patch will be more simple.
> So it may remain a matter of debate.

It seems true that expiration makes listdirs more rare (according to
set suffix_expiration_expire_age). However, the very next "for" loop
after every invocation of self.audit_location_suffix() is a loop over
every object read by pulling chunks from DiskFile. I hoped that reading
of object bodies would amortize any necessary listdirs. Although perhaps
it's not quite true if objects are small. Unfortunately, I do not have
necessary Swift experience to tell for sure. I think Darrell was going
to test this on a real cluster.

One note though: it appears that if suffix_expiration_enable is not set,
auditor never updates the hashes.pkl. audit_location_suffix effectively
turns into an equivalent of audit_location_generator with
traverse_part_only=False. The whole patch is disabled.
If this is intentional, the flag probably should be renamed into
something like 'rehash_enable', to better reflect what it does.

I've got some stylistic complaints too, like the way audit_location_generator
was split: it's a real puzzle for someone new, like me (maybe my head is
too small (笑)). Or that SuffixExpireWorker.conf looks not used for
anything outside __init__ (is it?). But those are relatively unimportant
against performance impacts, I think.

-- Pete