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

add 1-5 minutes of delay after maintenance mode was enabled #5872

Closed
butonic opened this issue Jun 30, 2017 · 11 comments
Closed

add 1-5 minutes of delay after maintenance mode was enabled #5872

butonic opened this issue Jun 30, 2017 · 11 comments
Assignees
Labels
ReadyToTest QA, please validate the fix/enhancement
Milestone

Comments

@butonic
Copy link
Member

butonic commented Jun 30, 2017

otherwise an a large instance all sync clients will come back at once, hitting the system ... hard

@michaelstingl
Copy link
Contributor

PR for maintenance mode detection: #5755

@guruz guruz added this to the 2.4.0 milestone Jul 3, 2017
@guruz
Copy link
Contributor

guruz commented Jul 3, 2017

@butonic btw, the web server should also be configured in a way so that it won't spawn too many PHP processes/threads and send 503 from web server layer.

@ckamm Is a random back-off easily implementable? I think maybe we don't even want this.

@ckamm
Copy link
Contributor

ckamm commented Jul 3, 2017

@guruz I think this would be easy, we just need a random "coming back up" timer. I think it should apply to maintinance mode and bare 503.

ckamm added a commit that referenced this issue Jul 4, 2017
This only applies when the server was explicitly in maintenance mode or
when it was 503-unavailable.
@ckamm
Copy link
Contributor

ckamm commented Jul 4, 2017

PR #5874
It's currently not configurable -- does it need to be in a Theme?

guruz pushed a commit that referenced this issue Jul 4, 2017
This only applies when the server was explicitly in maintenance mode or
when it was 503-unavailable.
@michaelstingl
Copy link
Contributor

@ckamm Perhaps use an environment variable to disable this for debugging purpose. Otherwise debugging can become quite time consuming… (theming could be interesting too)

@guruz
Copy link
Contributor

guruz commented Jul 5, 2017

@michaelstingl Sorry, I think that's overkill and would pollute the code. Not worth it for this kind of feature.

@michaelstingl
Copy link
Contributor

@guruz okay, then it's fine for me

@ckamm ckamm added the ReadyToTest QA, please validate the fix/enhancement label Jul 7, 2017
@SamuAlfageme
Copy link
Contributor

After quite a few tests, it seems pretty much behaving the way expected now - I'm with @guruz in this one:

the web server should also be configured in a way so that it won't spawn too many PHP processes/threads and send 503 from web server layer.

If switching off maintenance in the server was properly handled, there should be no need for this, which might affect smaller instances as collateral.

Also I'm worried, if the client could still be affected by a Byzantine 503 from the server (e.g. with some ext.sto. backends that do not behave so well like in #5088) - being fooled into a 5'-wait when maintenance mode was not activated.

Closing here as behavior is the expected, we could keep the discussion on a core ticket.

@ckamm
Copy link
Contributor

ckamm commented Nov 14, 2017

@SamuAlfageme A stay 503 won't make the client think maintenance mode is active. The server has to actively have maintenance: true in its status.php reply.

@SamuAlfageme
Copy link
Contributor

@ckamm I expressed myself wrong:

being fooled into a 5'-wait when maintenance mode was not activated.

Should say "even when maintenance mode was not activated" which was the original reason to introduce the delay. And reading:

/**
* Starts counting when the server starts being back up after 503 or
* maintenance mode. The account will only become connected once this
* timer exceeds the _maintenanceToConnectedDelay value.
*/

I was assuming both sources resulted on the application of the delay; is it wrong?

@ckamm
Copy link
Contributor

ckamm commented Nov 14, 2017

@SamuAlfageme You're right, I forgot about that. Yes, a 503 reply to the auth check in connectionvalidator would also trigger the delay. A random 503 during sync would not though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ReadyToTest QA, please validate the fix/enhancement
Projects
None yet
Development

No branches or pull requests

5 participants