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

Adjust state logic and prune implementation #894

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

giggsoff
Copy link
Collaborator

@giggsoff giggsoff commented Sep 29, 2023

We try to calculate state of EdgeNode and objects inside of it. We construct the state based on info and metric messages from the controller. Info and metric messages are great for debugging, but in case of long time work we may hit the problem where state calculation consume more and more time.

Let's slightly refactor the state logic to be able to store it locally and reuse. We may prune objects in Adam (using novel eden prune) to keep the logic to be as fast as possible.

Copy link
Member

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

I skimmed through your code and that's really good work @giggsoff. I believe we need to do it. I think it's a nice way to solve it right now, but we need to think about more sustainable solution : making adam more useful, because eden was initially designed as testing utility and now it's slowly growing to become simple stateful cloud. IMO adam should be turned into something like EVE controller SDK and a lot of eden functionality should migrate there. Let me know your thoughts on this

P.S. will do more thorough review next week :)

@@ -117,7 +118,7 @@ func getPortMapping(appConfig *config.AppInstanceConfig, qemuPorts map[string]st
}

func (ctx *State) initApplications(ctrl controller.Cloud, dev *device.Ctx) error {
ctx.applications = make(map[string]*AppInstState)
ctx.Applications = make(map[string]*AppInstState)
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need to export Applications?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I want to store the state in file (using json functions), which requires to export fields

@giggsoff
Copy link
Collaborator Author

giggsoff commented Oct 9, 2023

IMO adam should be turned into something like EVE controller SDK and a lot of eden functionality should migrate there.

Agree with you, it would be great. But I can see several problems here:

  1. We do not want to loose ability to see every single info message from EVE-OS (for debugging/testing), those we want to keep Adam to store everything somewhere and make it available (more or less optimal).
  2. We do not want (and have no volunteers) to completely rework Adam. And actually we do not want, because it will require from us additional resources to integrate and run Adam with Kafka/Opensearch/something else.
  3. We can avoid modifications in Adam regardless of modifications in Eden (probably not true for some kind of new certificate-related tests) or EVE-OS API (probably not true in case of new endpoints, which is really rare). If we new with some odd configuration transition and results observation, we can write it in one place - in Eden.

@@ -188,16 +210,23 @@ func (ctx *State) processApplicationsByInfo(im *info.ZInfoMsg) {
app.EVEState = fmt.Sprintf("%s (%d%%)", info.ZSwState_DOWNLOAD_STARTED.String(), int(percent)/len(app.Volumes))
}
}
case info.ZInfoTypes_ZiAppInstMetaData:
Copy link
Member

Choose a reason for hiding this comment

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

Additional thanks for AppInstMetaData

Copy link
Member

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

LGTM

@uncleDecart
Copy link
Member

@giggsoff I see you force push commits, is this PR mergable?

@giggsoff
Copy link
Collaborator Author

@giggsoff I see you force push commits, is this PR mergable?

I hope so...
But I really want to see at least one attempt of CI passed and green, but cannot find any after lots of retry. Let's wait for your PR on moving onto buildjet runners and I will rebase the PR to check again.

Seems we have quite outdated format for several files

Signed-off-by: Petr Fedchenkov <[email protected]>
We try to calculate state of EdgeNode and objects inside of it.
We construct the state based on info and metric messages from the
controller. Info and metric messages are great for debugging, but in
case of long time work we may hit the problem where state
calculation consume more and more time.

Let's slightly refactor the state logic to be able to store it locally
and reuse.

Signed-off-by: Petr Fedchenkov <[email protected]>
In case of state store and load functions implemented we may prune
objects in Adam to keep the logic to be as fast as possible.

Signed-off-by: Petr Fedchenkov <[email protected]>
We want to use check-new option to check options after transition, thus
 the next state after initial

Signed-off-by: Petr Fedchenkov <[email protected]>
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.

2 participants