MurmurHash3 support for fingerprint processor #70632
Conversation
danhermann
added
>enhancement
:Data Management/Ingest Node
elasticmachine
added
the
Team:Data Management
|
Pinging @elastic/es-core-features (Team:Core/Features) |
|
@elasticmachine update branch |
| if (method.equalsIgnoreCase(MurmurHasher.METHOD)) { | ||
| return MurmurHasher.getInstance(method); | ||
| } else { | ||
| return MessageDigestHasher.getInstance(method); | ||
| } |
There was a problem hiding this comment.
I think I would prefer this logic to live inside of MessageDigestHasher.getInstance, so that callers could retrieve the Murmur hasher without having to do this if check (it looks just like another built-in MessageDigestHasher).
What do you think about moving it there?
There was a problem hiding this comment.
Sure, I can do that.
| @@ -20,6 +20,8 @@ | |||
| */ | |||
| public class Murmur3Hasher { | |||
|
|
|||
| public static final String METHOD = "MurmurHash3_x64_128"; | |||
There was a problem hiding this comment.
Do we expect to add a few different murmur hash types? This is a pretty awkward name, but it makes sense if we expect to add other configurations
There was a problem hiding this comment.
I'm not aware of any plans to add additional variants of the Murmur hash, so I think we could shorten it to MurmurHash3 as it's commonly known, possibly with a note in the docs that it's the x64/128 variant that's used.
|
@elasticmachine run elasticsearch-ci/packaging-sample-windows |
|
Thanks, @dakrone! |
danhermann
changed the title
|
cc: @elastic/es-ui in case auto-complete needs to be updated to accommodate this new hash method option. |
Thanks for the heads up @danhermann! Have the docs been updated yet? To clarify, this adds |
|
Yes, the docs were updated in #70737. |
Closes #69182.