Skip to content

[WIP] refactor: Postgres instead of kafka#516

Draft
nicow-elia wants to merge 3 commits into
mainfrom
refactor/postgres-commands
Draft

[WIP] refactor: Postgres instead of kafka#516
nicow-elia wants to merge 3 commits into
mainfrom
refactor/postgres-commands

Conversation

@nicow-elia

Copy link
Copy Markdown
Collaborator

Checklist

Please check if the PR fulfills these requirements:

  • PR Title follows conventional commit messages
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • All commits in this PR are DCO signed-off (see CONTRIBUTING.md)

Does this PR already have an issue describing the problem?

Fixes #

What is the new behavior (if this is a feature change)?

Does this PR introduce a breaking change?

  • Yes
  • No


If a worker took very long during an epoch and exceeded the lease, two things might happen: either the job has not yet been
picked up - in this case it can keep working, or another worker took over the stage and it has to drop all work
and assume the job is no longer allocated to itself.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we leave the runtime check to the worker, this could potentially lead to an optimization being restarted over and over, taking way longer than the 300s allocated to it. We could also increment the time already spent on this stage in every iteration and stop optimizing once this reaches the runtime_limit

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the lease isn't the 300s - that's the time we expect an epoch to take plus a solid margin of error. The runtime limit is still something the worker would check internally. This is more related to the kafka timeout when a worker is considered dead.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i know. But if we leave it to the worker this could lead to longer than intended runtimes. For example:
Worker 1 runs for 180 of 300s and than fails
Worker 2 picks up the task and runs for 300s
-> optimization ran for 480s.

This does not break anything, but could make comparison a little tricky

into the database, but with a TRIGGERED flag. When a stage starts evaluation of a topology, it moves it to RUNNING and then
to a terminal state ACCEPTED/WARN/REJECT. Except for the terminal stage (AC), all stages have the responsibility to create
StageTopologyEvaluation objects for the next stage if they deem a topology feasible for evaluation by that stage (e.g. it is
in ACCEPTED or WARN category.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I think I would prefer the different stages being resposible to check for new accepted topologies in the previous stage, that have no StageTopologyEvaluation for the stage yet.

So:
DC <- AC fast Failing < AC

Otherwise the tables will just fill up with loads of triggered, but unevaluated topologies.
This way the workflow from get-topologies-to-evaluate -> evaluate -> store-topologies-for-stage would all be in the same place

result entry of the early (DC) or the later (AC-FAST-FAIL) stage. As the worst-k cases are a denser form of metrics, it
semantically seems to be preffered to write them in the result entry of the stage that computed them.

In contrast to StageWorkItems, StageTopologyEvaluations do not carry a lease. This makes a recovery of individual failed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, except for memory stuff. But i think thats fine

and assume the job is no longer allocated to itself.

To cancel a run, the user shall set every stage work item to CANCELLED upon which the workers will exit on their next update.
Note that setting this from None to CANCELLED is the desired flow, but once it was set and has propagated, this should

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why from None?

And i would also be fine with restarting it. So setting it from cancelled to triggered again.
or setting the AC Stage from finished to triggered to get some more topologies (probably out of scope)

transient logs when investigating repeated failures or retry exhaustion.
"""

loadflows_computed: int = 0

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the OptimizerType can be accessed via stage_work_item_id ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants