we re getting collisions with the execution IDs of subworkfl

Question

we re getting collisions with the execution IDs of subworkflow launch plans that propeller is kicking off ```Setting the MetadataDir for StartNode <gs flyte freenome computational metadata propeller balrog production fldyczra start node data> ``` `fldyczra` was already used given that these are much shorter than what can be used is there an easy way to make these longer to avoid possible collisions

jeev · Answer

the consequence of this is that propeller is passing totally wrong values derived from the older execution to downstream tasks which is potentially very dangerous

jeev · Answer

tagging < Anna Cunningham> that first discovered this

Ketan (kumare3) · Answer

< jeev> we migrated to 20 character ids recently

jeev · Answer

< Ketan kumare3 > what version of flytepropeller

Ketan (kumare3) · Answer

Ketan (kumare3) · Answer

where did you get the subworkflow id of 8 chars

Ketan (kumare3) · Answer

actually nevermind this is 3 years ago

jeev · Answer

we re running ```image <http cr flyte org flyteorg flytepropeller v1 1 15 sha256 6630864d9adc1e6ecd980376b244f826cbee5962a36fb9b7760a840bad70447b|cr flyte org flyteorg flytepropeller v1 1 15 sha256 6630864d9adc1e6ecd980376b244f826cbee5962a36fb9b7760a840bad70447b>```

Ketan (kumare3) · Answer

Ketan (kumare3) · Answer

I think this is the problem

Ketan (kumare3) · Answer

cc < Dan Rammer hamersaw >

Ketan (kumare3) · Answer

i dont know how to make it longer

jeev · Answer

collisions seem highly unlikely just bad luck

Ketan (kumare3) · Answer

Ya

Ketan (kumare3) · Answer

With 8 characters there is higher likelihood as compared to 20 chars

jeev · Answer

right

jeev · Answer

after some major digging we found the colliding input strings `fbffih2q n2 0 n3 0 dn0 0` and `f24nu5ka n2 0 n3 0 dn4 0` hash to the same value `fldyczra` `fbffih2q n2 0 n3 0 dn1 0` and `f24nu5ka n2 0 n3 0 dn5 0` hash to `fkyq4zuy` so between these 2 executions `fbffih2q` and `f24nu5ka` we had like 7 or so collisions disappointed

Dan Rammer (hamersaw) · Answer

OK first thanks for finding this It can t have been fun to track down For an immediate fix I think it makes sense to increase the hashing algorithm to 64 bit Obviously collisions may still occur so we should probably detect launch plan execution id collisions this is a bit more involved

Ketan (kumare3) · Answer

This is very involved

jeev · Answer

we ll look into other things like object lifecycle policies and memoization ttl to reduce the likelihood of this happening but ultimately that would really just be skirting around the issue

jeev · Answer

but not 100 sure if its ok to lifecycle the propeller metadata

Ketan (kumare3) · Answer

It is ok to lifecycle

jeev · Answer

that would probably break getting execution data via remote or the console

jeev · Answer

no

Ketan (kumare3) · Answer

Just delete the db entry too

jeev · Answer

hmm

jeev · Answer

we re depending too much on flyte metadata right now but maybe this is something we should look into

Anna Cunningham · Answer

any suggestions for how we can work around this issue without making any changes in the DB or is that the only way the first time this happened I just relaunched the workflow after renaming an input file so it wouldn t try reading from the cache it happened again with another workflow today and we d prefer not to relaunch this workflow as it s very large and computationally expensive to rerun

jeev · Answer

i wonder if this says anything about the hash algorithm itself and it s usefulness here should we be worried

jeev · Answer

it s especially dangerous because the collisions we have seen are from similar positions in the DAG as such it s possible that there might be a silent failure

Ketan (kumare3) · Answer

Hmm the problem is that the generated strong and the hashing algorithm Fnv are unable to have enough entropy

Ketan (kumare3) · Answer

We should prefix the generated string with the exec id

Ketan (kumare3) · Answer

But then we have to conform

jeev · Answer

< Anna Cunningham> would you be open to coordinating a solution with the flyte team on monday unfortunately i ll still be OOO

Ketan (kumare3) · Answer

Cc < Haytham Abuelfutuh> < Dan Rammer hamersaw > this is very disturbing

Anna Cunningham · Answer

I m not as familiar with the internals but I can do my best this is definitely an urgent and serious issue for us

Ketan (kumare3) · Answer

Yes I get and we are here to support

Haytham Abuelfutuh · Answer

Apologies for the delay < Anna Cunningham> Was just made aware of this issue Is there a github issue for it yet if not do you mind filing one happy to help push a quick fix forward

Anna Cunningham · Answer

sure where should I file it not sure if this should go in `flytekit` repo or elsewhere

Haytham Abuelfutuh · Answer

<http github com flyteorg flyte issues|github com flyteorg flyte issues> plz

Anna Cunningham · Answer

submitted but please reach out if you need more information

Haytham Abuelfutuh · Answer

Thank you

Haytham Abuelfutuh · Answer

< Dan Rammer hamersaw > < Ketan kumare3 > please review Proposals If you don t have strong preferences I ll go with 475

Matheus Moreno · Answer

Hey guys just a heads up my coworker just had the same problem It was also a complex workflow comprised of many subworkflows

Ketan (kumare3) · Answer

Ohh no

Ketan (kumare3) · Answer

This fix will do it

Ketan (kumare3) · Answer

Sorry about that

Matheus Moreno · Answer

What s the current workaround for this Should we delete the project from our database

Anna Cunningham · Answer

any updates on this fix I see the github issue is still open and hasn t had much activity

Ketan (kumare3) · Answer

Cc < Haytham Abuelfutuh>

will danforth · Answer

Thanks < Ketan kumare3 > and < Haytham Abuelfutuh> for your help on this

will danforth · Answer

Hey < Ketan kumare3 > and < Haytham Abuelfutuh> do you have an update on this

Dan Rammer (hamersaw) · Answer

< will danforth> thanks for your patience on this We merged support for and should be merging with backwards compatability in FlytePropeller shortly ie hopefully later today

Ketan (kumare3) · Answer

< Rahul Mehta> this should also give you large names