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

[Ingest] Setup gets called twice when navigating to the Ingest Manager plugin #65779

Closed
neptunian opened this issue May 7, 2020 · 12 comments
Closed
Labels
bug Fixes for quality problems that affect the customer experience impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Fleet Team label for Observability Data Collection Fleet team

Comments

@neptunian
Copy link
Contributor

Screen Shot 2020-05-07 at 5 39 52 PM

@neptunian neptunian added bug Fixes for quality problems that affect the customer experience Team:Fleet Team label for Observability Data Collection Fleet team labels May 7, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/ingest-management (Team:Ingest Management)

@jen-huang
Copy link
Contributor

jen-huang commented May 11, 2020

Not sure that this is a bug. It's called first during plugin start() and the second call is when the app component begins to load. That second call and any subsequent calls to /setup essentially verifies that setup was done. What do you think?

@ruflin
Copy link
Member

ruflin commented May 11, 2020

This is similar to the discussion in #66023 and I would assume the behaviour and available flags should be the same? @nchaulet

@nchaulet
Copy link
Member

it's kind of similar of fleet setup but there is no need of user confirmation and everything is made in the background, can we just provide the results of the start call to our application for fleet we have a FleetStatusProvider we can have the same for the rest?

@ruflin
Copy link
Member

ruflin commented May 12, 2020

@nchaulet Not sure I understand the difference between the two. Could you elaborate on how a call to the API for the two is different?

@nchaulet
Copy link
Member

Yes, the call for the fleet setup is done in two call one
GET /fleet/setup to know if the setup is already done
POST /fleet/setup to perform the setup when the user click

For the rest of the setup we just call
POST /ingest_manager/setup everytime if the setup is already done or not.

@ruflin
Copy link
Member

ruflin commented May 13, 2020

@nchaulet I get the difference from a UI perspective. I'm more thinking here about the same behaviour of the two POST API endpoints: If I run POST /fleet/setup or POST /ingest_manager/setup both should have the same behaviour in that the setup is only run once and have the same behaviour if the setup already happened before. Not that one for example would return an error and the other a success message.

@neptunian
Copy link
Contributor Author

neptunian commented May 13, 2020

@jen-huang I think its possible there could be a race condition when it's called twice in succession where extra work is being done. For example it has to install the 3 packages for the first time, but those aren't considered installed until the installation is finished. Since installing is idempotent, it's fine, but it could install the packages twice needlessly.

@jfsiii
Copy link
Contributor

jfsiii commented May 14, 2020

@neptunian I don't think there's a race condition. We await in start (and all throughout setupIngestManager) so start shouldn't complete with the installation in progress. Though it might fail/error.

I think the app code should use that success/error to see if it needs to be called again. I don't know if that's possible today. Our dependents use plugins.ingestManager.success; I'm not sure how we'd access it. I don't know if that means we follow the Fleet GET/POST split or read the value some other way.

There are at least two views which call setup. I can link to them later.

FWIW, I have looked into this a bit recently for #66125 and the initial call takes 10-15 seconds but the second doesn't even take 100ms.

I think the second call is wasteful, but not harmful. It'd be great if it could help fix something that failed earlier or alert the user about the issue but I don't think it's hurting anything.

@ph ph added impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. and removed impact:medium Addressing this issue will have a medium level of impact on the quality/strength of our product. labels May 28, 2020
@ph
Copy link
Contributor

ph commented May 28, 2020

Looking at this is this a bug or a tech debt where we could merge both call into one? If the call is idempotent, this look like a tech deb we could refactor.

@ph
Copy link
Contributor

ph commented Oct 19, 2020

Still valid?

@neptunian
Copy link
Contributor Author

No, I think we've come to the conclusion for now that we need to call it twice and @jfsiii has done some optimizations for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. Team:Fleet Team label for Observability Data Collection Fleet team
Projects
None yet
Development

No branches or pull requests

7 participants