Bug 976228 - Operation which require server reload should check if something was changed
Operation which require server reload should check if something was changed
Status: CLOSED CURRENTRELEASE
Product: JBoss Enterprise Application Platform 6
Classification: JBoss
Component: Domain Management (Show other bugs)
6.1.1
Unspecified Unspecified
medium Severity medium
: ER1
: EAP 6.3.0
Assigned To: Emmanuel Hugonnet (ehsavoie)
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-20 03:28 EDT by Petr Kremensky
Modified: 2014-06-28 11:37 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Enhancement
Doc Text:
In earlier versions of JBoss EAP 6, some operations which did not effectively change the status of the server would put it in reload-required mode. The was because the system did not check if the operation would effectively change the configuration. In this release, if checks confirm that the configuration of the server was not changed by the operation, a reload is not required. This change does not cover every possible case.
Story Points: ---
Clone Of:
Environment:
Last Closed: 2014-06-28 11:37:41 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
web console after second CLI operation (23.16 KB, image/png)
2013-06-20 03:28 EDT, Petr Kremensky
no flags Details


External Trackers
Tracker ID Priority Status Summary Last Updated
JBoss Issue Tracker WFLY-3154 Major Resolved Operation which require server reload should check if something was changed 2017-09-29 03:04 EDT

  None (edit)
Description Petr Kremensky 2013-06-20 03:28:26 EDT
Created attachment 763295 [details]
web console after second CLI operation

Description of problem:
Server reload is required even if nothing was actually changed. This could have negative impact on usability of EAP.

Version-Release 
EAP 6.1.1 ER1

Steps to Reproduce:
 - start standalone
 - connect to cli
 - run: /subsystem=web/virtual-server=default-host:write-attribute(name=enable-welcome-root, value=false
 - reload server
 - run: /subsystem=web/virtual-server=default-host:write-attribute(name=enable-welcome-root, value=false

Actual results:
{
    "outcome" => "success",
    "response-headers" => {
        "operation-requires-reload" => true,
        "process-state" => "reload-required"
    }
}
Expected results:
 - reload is not required
Comment 1 Alexey Loubyansky 2013-06-20 04:38:28 EDT
The CLI is only reflecting what happened in the domain management.
Comment 5 Emmanuel Hugonnet (ehsavoie) 2014-03-25 05:05:53 EDT
Upstream PR : https://github.com/kabir/wildfly/pull/28
PR : https://github.com/jbossas/jboss-eap/pull/1121
Comment 6 Emmanuel Hugonnet (ehsavoie) 2014-03-28 04:40:21 EDT
Upstream PR : https://github.com/wildfly/wildfly/pull/6091 
PR : https://github.com/jbossas/jboss-eap/pull/1121
Comment 7 Petr Kremensky 2014-04-14 07:42:43 EDT
I found a case, where the reload is still required:
[standalone@localhost:9999 /] /subsystem=logging/periodic-rotating-file-handler=FILE:read-attribute(name=append)
{
    "outcome" => "success",
    "result" => true
}
[standalone@localhost:9999 /] /subsystem=logging/periodic-rotating-file-handler=FILE:write-attribute(name=append, value=true)
{
    "outcome" => "success",
    "response-headers" => {
        "operation-requires-reload" => true,
        "process-state" => "reload-required"
    }
}
[standalone@localhost:9999 /] reload
[standalone@localhost:9999 /] /subsystem=logging/periodic-rotating-file-handler=FILE:write-attribute(name=append, value=true)
{
    "outcome" => "success",
    "response-headers" => {
        "operation-requires-reload" => true,
        "process-state" => "reload-required"
    }
}

Is this some kind of exception we can ignore?
Comment 8 Emmanuel Hugonnet (ehsavoie) 2014-04-14 09:25:26 EDT
The logging module uses its own logic with Propery to define the state instead of relying on the ReloadRequiredWriteAttributeHandler. As it is some 'generic' process for lgger configuration switches I don't know if we should change this.
Comment 9 Petr Kremensky 2014-04-15 01:26:07 EDT
@James
Can you please look into this? Is there anything that can be done in logging module regarding this issue, or shall I close it as verified leaving logging module as it is. 
Thanks.
Comment 11 James Perkins 2014-04-15 11:54:55 EDT
Is the idea that if the value defined in the model is the same as the value being written that we just leave it? If that's the case then it should be fine and likely a simple fix.

In this particular case for append the default value is true. If the current value is UNDEFINED, e.g. the default, then it should still require a reload IMO.
Comment 12 Brian Stansberry 2014-04-15 12:10:58 EDT
@James -- the basic issue is to not trigger reload-required if the runtime value isn't actually going to change. See Emmanuel's ReloadRequiredWriteAttributeHandler fix which has some good code comments:

https://github.com/jbossas/jboss-eap/pull/1121/files

It's important to be careful about thinking about whether the value may have changed and to make a conservative decision (i.e. set reload-required if you aren't 100% certain the value is unchanged.) Emmanuel's code comment on that patch gets into that.
Comment 13 James Perkins 2014-04-15 13:10:37 EDT
Fair enough. In this case I would be inclined to leave it as it is. This is one of the few attributes in the logging subsystem that actually requires a reload. Maybe it shouldn't even require it as it doesn't really do anything unless logging is being initialized anyway. What I mean is setting append to true doesn't mean anything at runtime as the stream is already open. That is the only reason I added the reload flag.

I'd also say it's probably rare when that flag would be changed anyway. It's probably set/changed once and left. So I say we leave it as is for now.
Comment 14 Petr Kremensky 2014-04-16 01:35:01 EDT
Based on the comments above, closing this as verified.

Verified on EAP 6.3.0.ER1

Note You need to log in before you can comment on or make changes to this bug.