Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]FlytePropeller should limit the max number of node traversed / executing in one round #476

Closed
1 of 13 tasks
kumare3 opened this issue Aug 18, 2020 · 6 comments
Closed
1 of 13 tasks
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@kumare3
Copy link
Contributor

kumare3 commented Aug 18, 2020

Motivation: Why do you think this is important?
It is possible with dynamic nodes, that users could yield thousands of nodes. The upper limit that is enforced is 4k .FlytePropeller wonderously works with these high numbers, but not without impact on latency. Propeller should limit the number of nodes that can be traversed and be concurrently running.

Goal: What should the final outcome look like, ideally?
When users run large graphs, barring size of the status, flyte propeller should have predictable round latency.

Describe alternatives you've considered
Array Jobs. But, array jobs are not supported for all types.

Flyte component

  • Overall
  • Flyte Setup and Installation scripts
  • Flyte Documentation
  • Flyte communication (slack/email etc)
  • FlytePropeller
  • FlyteIDL (Flyte specification language)
  • Flytekit (Python SDK)
  • FlyteAdmin (Control Plane service)
  • FlytePlugins
  • DataCatalog
  • FlyteStdlib (common libraries)
  • FlyteConsole (UI)
  • Other

[Optional] Propose: Link/Inline
NA

@kumare3 kumare3 added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels Aug 18, 2020
@kumare3 kumare3 added this to the 0.9.0 milestone Aug 25, 2020
@datability-io
Copy link

+1

@kumare3 kumare3 modified the milestones: 0.9.0, 0.10.0 Sep 30, 2020
@EngHabu EngHabu assigned schottra and unassigned schottra Nov 4, 2020
@EngHabu EngHabu modified the milestones: 0.10.0, 0.11.0 Nov 4, 2020
@cwormsl2-zz
Copy link

+1

1 similar comment
@ganeshplyft
Copy link

+1

@EngHabu EngHabu removed this from the 0.11.0 milestone Jan 11, 2021
@kumare3 kumare3 self-assigned this Feb 16, 2021
@kumare3 kumare3 added this to the 0.11.0 milestone Feb 16, 2021
@kumare3
Copy link
Contributor Author

kumare3 commented Feb 16, 2021

I have a PR for this in progress and the tests are really promising. I am hoping to close it by End of February

@kumare3 kumare3 removed the untriaged This issues has not yet been looked at by the Maintainers label Feb 16, 2021
@kumare3
Copy link
Contributor Author

kumare3 commented Feb 28, 2021

This has been implemented but is blocked on #742. I will move it to 0.12.0

@kumare3
Copy link
Contributor Author

kumare3 commented Mar 24, 2021

This core functionality is merged, But it is not yet available to all users. This will be done in #862

@kumare3 kumare3 closed this as completed Mar 24, 2021
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
* Prefix sub-lp exec id with the parent exec-id

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

* cleanup

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

* cleanup

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

* Use a CRD-level version instead

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

* cleanup

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

* Use fnv64 to create a hash for child workflow executions

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

* Move DefinitionVersion to Status of the CRD

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

* Update to the released flyteplugins

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

* Regenerate

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

* fix unit tests

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

* more fixes

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

* fix old unit test

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

Signed-off-by: Haytham Abuelfutuh <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
eapolinario added a commit to eapolinario/flyte that referenced this issue Jul 24, 2023
…yteorg#476)

* Added race skip check

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* lint

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Fixed unit tests

Signed-off-by: pmahindrakar-oss <[email protected]>

* Moved to integration test

Signed-off-by: pmahindrakar-oss <[email protected]>

* refactored integration test

Signed-off-by: pmahindrakar-oss <[email protected]>

* nit : rename to lastTime

Signed-off-by: pmahindrakar-oss <[email protected]>

* nit : revert

Signed-off-by: pmahindrakar-oss <[email protected]>

* lastTime -> lastExecTime

Signed-off-by: pmahindrakar-oss <[email protected]>

* integration test tag

Signed-off-by: pmahindrakar-oss <[email protected]>

---------

Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: eduardo apolinario <[email protected]>
Co-authored-by: eduardo apolinario <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 9, 2023
* Prefix sub-lp exec id with the parent exec-id

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

* cleanup

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

* cleanup

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

* Use a CRD-level version instead

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

* cleanup

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

* Use fnv64 to create a hash for child workflow executions

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

* Move DefinitionVersion to Status of the CRD

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

* Update to the released flyteplugins

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

* Regenerate

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

* fix unit tests

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

* more fixes

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

* fix old unit test

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

Signed-off-by: Haytham Abuelfutuh <[email protected]>
eapolinario added a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
…yteorg#476)

* Added race skip check

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* lint

Signed-off-by: Prafulla Mahindrakar <[email protected]>

* Fixed unit tests

Signed-off-by: pmahindrakar-oss <[email protected]>

* Moved to integration test

Signed-off-by: pmahindrakar-oss <[email protected]>

* refactored integration test

Signed-off-by: pmahindrakar-oss <[email protected]>

* nit : rename to lastTime

Signed-off-by: pmahindrakar-oss <[email protected]>

* nit : revert

Signed-off-by: pmahindrakar-oss <[email protected]>

* lastTime -> lastExecTime

Signed-off-by: pmahindrakar-oss <[email protected]>

* integration test tag

Signed-off-by: pmahindrakar-oss <[email protected]>

---------

Signed-off-by: Prafulla Mahindrakar <[email protected]>
Signed-off-by: pmahindrakar-oss <[email protected]>
Signed-off-by: eduardo apolinario <[email protected]>
Co-authored-by: eduardo apolinario <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants