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.

Sign up for GitHub

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

Jump to bottom

[Metricbeat] Decrease timeout time of compose.EnsureUp functions #10894

Merged
merged 3 commits into from
Jun 4, 2019
Merged

[Metricbeat] Decrease timeout time of compose.EnsureUp functions #10894

merged 3 commits into from
Jun 4, 2019

Conversation

sayden
Copy link
Contributor

@sayden sayden commented Feb 22, 2019

Refer here for more info

@sayden sayden added Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Feb 22, 2019
@sayden sayden self-assigned this Feb 22, 2019
@sayden sayden requested review from a team as code owners February 22, 2019 12:22
@ruflin
Copy link
Contributor

ruflin commented Mar 4, 2019

Why not change the default and overwrite the exceptions?

@sayden
Copy link
Contributor Author

sayden commented Mar 7, 2019

Why not change the default and overwrite the exceptions?

After telling me this, I have realized that the compose package is actually ours (my bad, I thought it was the original one, I didn't checked imports). I'll change this in a commit and overwrite some exceptions like Kibana

@sayden sayden force-pushed the bugfix/mb/increase-timeouts-of-compose branch from aca1092 to 014c8c4 Compare March 7, 2019 12:15
@sayden sayden requested a review from a team as a code owner March 7, 2019 12:15
ruflin
ruflin suggested changes Mar 8, 2019
View reviewed changes
Copy link
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Seems like Logstash is now failing. Could you increase the timeout there to 300?

@sayden
Copy link
Contributor Author

sayden commented Mar 8, 2019

Done. This was my initial idea, to detect which modules needed a higher timeout and why

@sayden
Copy link
Contributor Author

sayden commented Mar 8, 2019

jenkins, test this please
ES error again, https://beats-ci.elastic.co/job/elastic+beats+pull-request+multijob-linux/beat=metricbeat,label=linux-immutable/5818/console might be related with the timeout

@ruflin
Copy link
Contributor

ruflin commented Mar 12, 2019

You didn't increase the timeout of Elasticsearch in this PR? Is that on purpose or is it already increased?

@sayden
Copy link
Contributor Author

sayden commented Mar 12, 2019

Actually I didn't increase it, I just left default time for all but Kibana and Logstash, but now that you mention it, I guess that ES need a slightly lower timeout than Kibana and Logstash to make those two work properly.

My idea was to keep triggering build in this branch before merging, just to check if the current timeouts were correct.

@sayden
Copy link
Contributor Author

sayden commented Mar 12, 2019

jenkins, test this please

@sayden sayden added the in progress Pull request is currently in progress. label Mar 13, 2019
@ruflin
Copy link
Contributor

ruflin commented Mar 13, 2019

This should be rebased as soon as #11230 is merged.

@sayden sayden force-pushed the bugfix/mb/increase-timeouts-of-compose branch from 301ed2f to c7828bb Compare March 26, 2019 16:28
@sayden sayden force-pushed the bugfix/mb/increase-timeouts-of-compose branch from c7828bb to 8a5397d Compare April 12, 2019 10:16
@sayden
Copy link
Contributor Author

sayden commented Apr 23, 2019

jenkins, test this please

2 similar comments
@sayden
Copy link
Contributor Author

sayden commented May 8, 2019

jenkins, test this please

@sayden
Copy link
Contributor Author

sayden commented May 29, 2019

jenkins, test this please

@sayden
Copy link
Contributor Author

sayden commented May 31, 2019

@ruflin I think we are safe merging this. All tests are fine and the ones that have been failing were always unrelated to timeouts. WDYT?

@sayden sayden added review and removed in progress Pull request is currently in progress. labels May 31, 2019
@ruflin
Copy link
Contributor

ruflin commented Jun 3, 2019

@sayden Overall SGTM. Would be great to get at least 1 full green build. Most PR's I approved recently were all green so I'm worried if this PR stays red, it might be related (can't see a reason at the moment that this should be true).

I think you should rebase on master again to get the "fixed" flaky tests in.

sayden added 2 commits June 4, 2019 11:19
@sayden
Change the global timeout
b868576
@sayden
Reduce overwritten timeouts from 10 minutes to 9m30s to avoid stale e…
622c674 
…rrors in Travis

(cherry picked from commit 10e6b5acf27569e9dfcb62f1c069e9c62ba9de3e)
@sayden sayden force-pushed the bugfix/mb/increase-timeouts-of-compose branch from 8a5397d to cbdc1af Compare June 4, 2019 09:20
@sayden
Copy link
Contributor Author

sayden commented Jun 4, 2019

We are green @ruflin 😄 do you want to trigger jenkins once more?

@ruflin
Copy link
Contributor

ruflin commented Jun 4, 2019

@sayden Let's get it in.

@sayden sayden merged commit 0c2b5e7 into elastic:master Jun 4, 2019
@zube zube bot added [zube]: Done and removed [zube]: Ready labels Jun 4, 2019
@sayden
Copy link
Contributor Author

sayden commented Jun 4, 2019

Thanks for the help @ruflin and @cachedout

andrewvc pushed a commit to andrewvc/beats that referenced this pull request Jun 12, 2019
@sayden @andrewvc
[Metricbeat] Decrease timeout time of compose.EnsureUp functions (ela…
c401af9 
…stic#10894)
@sayden sayden mentioned this pull request Jul 22, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ruflin ruflin ruflin requested changes

@cachedout cachedout cachedout approved these changes

Assignees

@sayden sayden

Labels
Metricbeat Metricbeat review Team:Integrations Label for the Integrations team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sayden @ruflin @cachedout @andresrc