Skip to content

Bump reporting timeout#20833

Merged
stacey-gammon merged 1 commit intoelastic:masterfrom
stacey-gammon:2018-07-16-bump-reporting-queue-timeout
Jul 24, 2018
Merged

Bump reporting timeout#20833
stacey-gammon merged 1 commit intoelastic:masterfrom
stacey-gammon:2018-07-16-bump-reporting-queue-timeout

Conversation

@stacey-gammon
Copy link

@stacey-gammon stacey-gammon commented Jul 16, 2018

Chromium takes a lot longer than Phantom to generate reports. Tests on cloud revealed 30000 was too short. Increasing by 4x, to 120000, since chromium will soon be the default.

The number was chosen pretty arbitrarily, but having it too short can create problems for our users, while having it too long merely delays the time it will take to reveal an error. IMO it's better to be on the safe side and have it too long rather than too short.

Fixes #20832

@stacey-gammon stacey-gammon added :Sharing v6.4.0 zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead labels Jul 16, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@mcpate
Copy link

mcpate commented Jul 20, 2018

Fixes #20833

I think you meant #20832?

@mcpate
Copy link

mcpate commented Jul 23, 2018

Hey there @stacey-gammon, just checking in on the progress of this and where it will eventually land. I see it is going in to master. Will it also go in to 6.4 and 6.3?

@stacey-gammon
Copy link
Author

Would not be set to go into 6.3, but could perhaps make it into 6.4 depending on review feedback. Is there a reason to push this back into 6.3? Our standard practice is to only back port critical bug fixes.

@mcpate
Copy link

mcpate commented Jul 23, 2018

Good to know about 6.3. Getting the fix in to this version doesn't seem critical to me, but I imagine that 6.4 would be that way we could test regionally (I think that was the original plan) prior to having this configuration be the default in 6.5.

@stacey-gammon
Copy link
Author

I imagine that 6.4 would be that way we could test regionally (I think that was the original plan)

Yep makes sense. Cloud has the ability to specify kibana.yml defaults though, right? Since that is how we were going to test the default to chromium? So worst case scenario, if this doesn't get in, would you be able to manually set the timeout in kibana.yml, when you default to chromium?

@mcpate
Copy link

mcpate commented Jul 23, 2018

Yes, we should be able to set a default for the timeout in the kibana.yml

Copy link
Contributor

@chrisdavies chrisdavies left a comment

Choose a reason for hiding this comment

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

LGTM. Sad that chromium is so much slower. I hope this improves with subsequent releases.

@stacey-gammon
Copy link
Author

Agreed. If I recall correctly, the chromium team did say they had it on their roadmap to improve performance. Our chromium build is also almost a year old at this point so creating a new one could help.

@stacey-gammon stacey-gammon merged commit f471aaa into elastic:master Jul 24, 2018
stacey-gammon pushed a commit to stacey-gammon/kibana that referenced this pull request Jul 24, 2018
stacey-gammon pushed a commit that referenced this pull request Jul 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v6.4.0 zDeprecated Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants