[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Jeff Mesnil updated AS7-5929: ----------------------------- Description: For the patching service, the patch command leaves the server in a restart-required state {noformat} [standalone@localhost:9999 /] patch /home/jmesnil/Developer/jboss-as/build/target/jboss-as-7.2.0.Alpha1-SNAPSHOT/patch.zip { "outcome" : "success", "response-headers" : { "operation-requires-restart" : true, "process-state" : "restart-required" } } {noformat} If I try to change the AS7 configuration, the CLI warns me that a restart is required, for example: {noformat} [standalone@localhost:9999 hornetq-server=default] ./queue=foo:add(queue-address=foo) { "outcome" => "success", "response-headers" => {"process-state" => "restart-required"} } {noformat} However, when I reload the server using the /:reload command, the process-state header is cleared out: {noformat} [standalone@localhost:9999 hornetq-server=default] /:reload {"outcome" => "success"} [standalone@localhost:9999 hornetq-server=default] ./queue=bar:add(queue-address=bar) {"outcome" => "success"} {noformat} This is not correct: until the server is restarted (not reloaded), the result of my patch operation will not be taken into account (eg changing the AS7 module path). Reloading the server should not clear its process-state if it is set to "restart-required" was: For the patching service, the patch command leaves the server in a restart-required state {noformat} [standalone@localhost:9999 /] patch /home/jmesnil/Developer/jboss-as/build/target/jboss-as-7.2.0.Alpha1-SNAPSHOT/patch.zip { "outcome" : "success", "response-headers" : { "operation-requires-restart" : true, "process-state" : "restart-required" } } {noformat} If I try to change the AS7 configuration, the CLI warns me that a restart is required, for example: {codeformat} [standalone@localhost:9999 hornetq-server=default] ./queue=foo:add(queue-address=foo) { "outcome" => "success", "response-headers" => {"process-state" => "restart-required"} } {codeformat} However, when I reload the server using the /:reload command, the process-state header is cleared out: {codeformat} [standalone@localhost:9999 hornetq-server=default] /:reload {"outcome" => "success"} [standalone@localhost:9999 hornetq-server=default] ./queue=bar:add(queue-address=bar) {"outcome" => "success"} {codeformat} This is not correct: until the server is restarted (not reloaded), the result of my patch operation will not be taken into account (eg changing the AS7 module path). Reloading the server should not clear its process-state if it is set to "restart-required" -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Bartosz Baranowski reassigned AS7-5929: --------------------------------------- Assignee: Bartosz Baranowski -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Jeff Mesnil commented on AS7-5929: ---------------------------------- sorry, the issue is lacking context. The "patch" command is a work-in-progress and is not in 7.1 or master branch. The issue should be reproductible with any command that flag the server as restart-required. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Brian Stansberry commented on AS7-5929: --------------------------------------- Hehe, that's probably a bug, since there's no reason that command should require a process restart. https://issues.jboss.org/browse/AS7-6035 -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Bartosz Baranowski commented on AS7-5929: ----------------------------------------- Uh, which command needs restart than? Im quite confused by source - attributes have Flags.RESTART_*, but than there is something like: {noformat} boolean restartRequired = doMagicStuffOnCommand(....) if(restartRequired) context.reloadRequired(); {noformat} Im trying to understand in details how the whole thing works and how to write test, seems like Im in for a bumpy ride. BTW: above command with output is in some googlabe admin guide. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Brian Stansberry commented on AS7-5929: --------------------------------------- Bartosz, are you referring to the "response-headers" => undefined in the response to the 2nd :reload? That's not a huge problem but if it was introduced by a patch you wrote we should clean it up. Look for: node.get("response-headers").isDefined() If that has been added, change to node.hasDefined("response-headers") Just a guess! Basically, any call to node.get("response-headers") that doesn't subsequently add a child will result in "response-headers" => undefined. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Brian Stansberry edited comment on AS7-5929 at 11/27/12 10:10 AM: ------------------------------------------------------------------ I think the trick is probably within ControlledProcessState itself. A boolean field "restartRequired" that gets set in setRestartRequired(): {code} synchronized (service) { if (stateRef.compareAndSet(was, State.RESTART_REQUIRED, receiver[0], newStamp)) { service.stateChanged(State.RESTART_REQUIRED); restartRequired = true; break; } } {code} and cleared in revertRestartRequired(): {code} synchronized (service) { if (state.compareAndSet(State.RESTART_REQUIRED, State.RUNNING, theirStamp, this.stamp.incrementAndGet())) { service.stateChanged(State.RUNNING); restartRequired = false; } } {code} and read in setRunning(): {code} synchronized (service) { State newRunningState = restartRequired ? State.RESTART_REQUIRED : State.RUNNING; state.set(newRunningState, stamp.incrementAndGet()); service.stateChanged(newRunningState); } {code} All writes and reads to restartRequired are guarded by synchronized (service) so it is thread safe. was (Author: brian.stansberry): I think the trick is probably within ControlledProcessState itself. A boolean field "restartRequired" that gets set in setRestartRequired(): {code} synchronized (service) { if (stateRef.compareAndSet(was, State.RESTART_REQUIRED, receiver[0], newStamp)) { service.stateChanged(State.RESTART_REQUIRED); restartRequired = true; break; } } {code} and cleared in revertRestartRequired(): synchronized (service) { if (state.compareAndSet(State.RESTART_REQUIRED, State.RUNNING, theirStamp, this.stamp.incrementAndGet())) { service.stateChanged(State.RUNNING); restartRequired = false; } } and read in setRunning(): synchronized (service) { State newRunningState = restartRequired ? State.RESTART_REQUIRED : State.RUNNING; state.set(newRunningState, stamp.incrementAndGet()); service.stateChanged(newRunningState); } All writes and reads to restartRequired are guarded by synchronized (service) so it is thread safe. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Bartosz Baranowski commented on AS7-5929: ----------------------------------------- [~brian.stansberry] Well, that will work. I got a bit stuck on this(since Service.addListener is deprecated - this would simplify everything) {noformat} public Object setRestartRequired() { //.... for (;;) { State was = stateRef.get(receiver); if (was == State.STARTING || was == State.STOPPING) { break; } //... } return Integer.valueOf(newStamp); }; {noformat} Since 'was' is equal to "STARTING" cause the process is in this state according to transition, change to ReloadHandler like: {noformat} //... final State initialProcessState = processState.getState(); //... if(initialProcessState == State.RESTART_REQUIRED) service.addListener(new AbstractServiceListener<Object>() { public void listenerAdded(final ServiceController<?> controller) {} public void transition(final ServiceController<? extends Object> controller, final ServiceController.Transition transition) { if(transition == Transition.STARTING_to_UP){ controller.removeListener(this); processState.setRestartRequired(); } } }); {noformat} Does not work. If what you propse is fine, I can commit that. It looks a bit like hack to me :) -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Bartosz Baranowski commented on AS7-5929: ----------------------------------------- Yes, this is current code - this is part that actually makes my change to ReloadHandler fial. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Brian Stansberry commented on AS7-5929: --------------------------------------- The :server-set-restart-required can be used to put the server into restart-required state. This is what the HC uses when some operation on the HC (e.g. a change in the JVM config used for the server) means the server needs to be restarted to be consistent with the HC config. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Bartosz Baranowski commented on AS7-5929: ----------------------------------------- For some twisted reason my 7.1 branch does not update properly( I think ), hence merge conflict. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Bartosz Baranowski commented on AS7-5929: ----------------------------------------- master branch has been merged, 7.1 not. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5929?page=com.atlassian.jira.plugin.s... ] Bartosz Baranowski commented on AS7-5929: ----------------------------------------- Small fix to master: https://github.com/jbossas/jboss-as/pull/3573 , 7.1 has this also. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira