Hello Bhaskar,
On Mon, Nov 04, 2013 at 03:35:20PM -0500, Bhaskar Maddala wrote:
> Hello,
>
> Apologies for the delay in replying. I just got back to this today and
> will spend time during this week. I need a couple of clarifications to make
> sure I understood what you say before I go about making the change.
No problem, you're welcome.
> > Note that we could also consider not doubling the keywords and proceeding
> > like this instead since "avalanche" is always optional with any hash :
>
> My reading of the code in backend.c and definitions in backend.h, and I
> think you mentioned it previously, avalanche/full_hash was always applied
> prior to consistent. What does "always optional" mean in this context?
I meant from a processing point of view, not the config one. Indeed, the
avalanche hash converts a 32-bit hash to another 32-bit one in order to
improve the distribution. But with some hashes it's not necessarily desirable
(eg: it seems that you got a better distribution with djb2 without avalanche
than with).
> > hash-type {map-based|consistent} [[<algo>] avalanche]
>
> > <algo> would be "sdbm", "djb2", "wt6", "sdbm+avalanche",
> "djb2+avalanche",
> > "wt6+avalanche".
>
> Are you now saying, that we do not apply avalanche with consistent unless
> specified?
Exactly. Later, we'll be able to support more re-hash algorithms if needed,
so better start compatible with 1.4 where consistent does not imply avalanche.
> I did not test this well enough, but did do a quick test, when I do not
> apply avalanche/full_hash for consistent in backend.c, I get wildly
> different results. I looked at the code in lb_chash.c between 1.4 and 1.5
> and did not find anything difference. Am I missing something?
Indeed, in 1.4 there was no avalanche hashing. In 1.5, everywhere we call a
hash, the test starts with :
if ((px->lbprm.algo & BE_LB_HASH_TYPE) != BE_LB_HASH_MAP)
h = full_hash(h);
which means that if we're not using a map-based algo (so we're either on
"avalanche" or "consistent"), then we're applying a full hash on the
result before proceeding :
#define BE_LB_HASH_MAP 0x000000 /* map-based hash (default) */
#define BE_LB_HASH_CONS 0x100000 /* consistent hashbit to indicate a dynamic
algorithm */
#define BE_LB_HASH_AVAL 0x200000 /* run an avalanche hash before a map */
#define BE_LB_HASH_TYPE 0x300000 /* get/clear hash types */
So we would simply change the instances of the test above to something more
or less like this :
if (px->lbprm.algo & BE_LB_HASH_AVAL)
h = full_hash(h);
> Please clarify at the earliest and I will work on it immediate. Once again,
> thanks for your patience.
You're welcome, I'm quite happy that someone takes care of this because I
know it's not very fun to do and many people would like to have more options
than today for the hashes.
Take your time, there's no emergency, and feel free to ask more question if
anything is unclear to you.
Best regards,
Willy