Update hash algo to 64bit to reduce collisions

[EngHabu]

Signed-off-by: Haytham Abuelfutuh [email protected]

TL;DR

We use 64bit fnv hashing to shorten the execution id lengths, however that has lead to collisions due to its low entropy. Bumping it to 64bit to reduce such collisions.

• Figure out how to rollout the change without affecting in-flight executions.

Type

• Bug Fix
• Feature
• Plugin

Are all requirements met?

• Code completed
• Smoke tested
• Unit tests added
• Code documentation added
• Any pending items have an associated Issue

Tracking Issue

flyteorg/flyte#2778

[codecov]

Codecov Report

Merging #283 (dc3b2dc) into master (cdfc191) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #283      +/-   ##
==========================================
+ Coverage   63.26%   63.34%   +0.07%     
==========================================
  Files         145      145              
  Lines        9266     9278      +12     
==========================================
+ Hits         5862     5877      +15     
+ Misses       2870     2866       -4     
- Partials      534      535       +1     

Flag	Coverage Δ
unittests	62.77% <100.00%> (+0.09%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
go/tasks/pluginmachinery/encoding/encoder.go	100.00% <100.00%> (ø)
go/tasks/plugins/array/awsbatch/executor.go	38.05% <0.00%> (-4.93%)	⬇️
go/tasks/plugins/array/k8s/executor.go	39.65% <0.00%> (+0.15%)	⬆️
go/tasks/plugins/array/catalog.go	49.31% <0.00%> (+0.73%)	⬆️
go/tasks/plugins/array/k8s/management.go	57.21% <0.00%> (+1.43%)	⬆️
go/tasks/plugins/array/awsbatch/launcher.go	63.46% <0.00%> (+5.56%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[hamersaw]

Looks like the max execution name length just increased to 63 in this PR. Do we want to use a 128 bit version of the algorithm?

[hamersaw]

Looks like the max execution name length just increased to 63 in this PR. Do we want to use a 128 bit version of the algorithm?

On second thought, probably a bad idea. We'll start running into issues with task id length.

edit: maybe not?!?! this code is pretty complex.

[EngHabu]

@hamersaw Unfortunately we can't guarantee the Admin version propeller will be talking to. I can add support for 128 in the plugins PR though... The admin PR should have bumped the EventVersion in the propeller CRD so that propeller code and make decisions based on the updated Admin..