Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Use fnv64 to create a hash for child workflow executions #476

Merged
merged 15 commits into from
Sep 9, 2022
Merged

Conversation

EngHabu
Copy link
Contributor

@EngHabu EngHabu commented Aug 23, 2022

Signed-off-by: Haytham Abuelfutuh haytham@afutuh.com

TL;DR

Use fnv64-bit for new child workflow execution names to decrease the probability of name collisions.
Ensure backward compatibility by storing a version in the workflow CRD to be able to generate the same id for check status as we did for launch.

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

Remaining work

  • Conflict detection and failing hard; Admin can perform a better check than "execution already exists" and return the owner of the original execution. Propeller can then validate that the currently executing workflow is the owner, otherwise it should fail immediately.

Tracking Issue

fixes flyteorg/flyte#2778

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
@EngHabu EngHabu changed the base branch from crd-version to master August 31, 2022 19:53
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #476 (cf6e615) into master (a0d01ce) will increase coverage by 0.01%.
The diff coverage is 71.05%.

@EngHabu EngHabu requested a review from hamersaw September 9, 2022 04:33
@EngHabu EngHabu marked this pull request as ready for review September 9, 2022 04:33
@EngHabu EngHabu requested a review from kumare3 as a code owner September 9, 2022 04:33
@EngHabu EngHabu merged commit 9cf5368 into master Sep 9, 2022
@EngHabu EngHabu deleted the use-fnv64 branch September 9, 2022 19:49
eapolinario pushed a commit to eapolinario/flytepropeller that referenced this pull request Aug 9, 2023
* Prefix sub-lp exec id with the parent exec-id

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* cleanup

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* cleanup

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Use a CRD-level version instead

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* cleanup

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Use fnv64 to create a hash for child workflow executions

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Move DefinitionVersion to Status of the CRD

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to the released flyteplugins

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Regenerate

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* fix unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* more fixes

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* fix old unit test

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Collisions on subworkflow launchplan execution IDs
2 participants