On 8/2/11 4:22 PM, Jason T. Greene wrote: > Currently we have way too many read only attributes in our management > model. This makes life harder for the management tools and/or makes the > resulting UI a bit unwieldy. > > In some cases this is just because we haven't polished subsystem > management (PLEASE all subsystem authors look at this!). In other cases > though it's intentional because it is either difficult or currently > impossible to make hot changes to a particular attribute. This should be > exceedingly rare. Users love hot updates, and it's a competitive > advantage. So first we should strive to make it hot in the first place. > > However in the case that the change is not hot, and a server reboot is > too aggressive, we should still supporting making the changes by > bouncing all services involved. After some debate on IRC we finally > arrived at what is IMO a really good option for this problem: > > write-attribute (impact-services=true|false) > > The operation handler on such a non-hot attribute would then return an > error A handler should not return an error if a change cannot be applied to the runtime. An OperationStepHandler should use the relevant API on the OperationContext. E.g. for a handler registered for Stage.RUNTIME: @Override public void execute(OperationContext context, ModelNode operation) { context.runtimeUpdateSkipped(); context.reloadRequired(); // or context.restartRequired(); if (context.completeStep() == OperationContext.ResultAction.ROLLBACK) { context.revertReloadRequired(); // or context.revertRestartRequired(); } }

On 8/22/11 1:42 PM, Jason T. Greene wrote: > On 8/22/11 1:26 PM, Brian Stansberry wrote: >> On 8/2/11 4:22 PM, Jason T. Greene wrote: >>> Currently we have way too many read only attributes in our management >>> model. This makes life harder for the management tools and/or makes the >>> resulting UI a bit unwieldy. >>> >>> In some cases this is just because we haven't polished subsystem >>> management (PLEASE all subsystem authors look at this!). In other cases >>> though it's intentional because it is either difficult or currently >>> impossible to make hot changes to a particular attribute. This >>> should be >>> exceedingly rare. Users love hot updates, and it's a competitive >>> advantage. So first we should strive to make it hot in the first place. >>> >>> However in the case that the change is not hot, and a server reboot is >>> too aggressive, we should still supporting making the changes by >>> bouncing all services involved. After some debate on IRC we finally >>> arrived at what is IMO a really good option for this problem: >>> >>> write-attribute (impact-services=true|false) >>> >>> The operation handler on such a non-hot attribute would then return an >>> error >> >> A handler should not return an error if a change cannot be applied to >> the runtime. An OperationStepHandler should use the relevant API on the >> OperationContext. E.g. for a handler registered for Stage.RUNTIME: >> >> @Override >> public void execute(OperationContext context, ModelNode operation) { >> >> context.runtimeUpdateSkipped(); >> context.reloadRequired(); // or context.restartRequired(); >> if (context.completeStep() == >> OperationContext.ResultAction.ROLLBACK) >> { >> context.revertReloadRequired(); >> // or context.revertRestartRequired(); >> } >> } > > Well in my above comment I was referring to a problem where a full > server reload and/or restart is too aggressive (i.e. why bounce the web > server when all you want to do is bounce a datasource). So the issue is > that these attributes don't fit that problem. To further clarify the reason to return the error is that the change to the configuration model is NOT APPLIED. If you touch the config without the runtime, then you are out of sync and the only reasonable action to take from that point on is to restart the server. In this case we are saying that changing an attribute could have hostile consequences (bouncing a connection pool therby affecting inbound transactions), and so we need some sort of confirmation by the admin that they really intend to do this.

On 8/22/11 1:53 PM, Brian Stansberry wrote: > On 8/22/11 1:47 PM, Jason T. Greene wrote: >> On 8/22/11 1:42 PM, Jason T. Greene wrote: >>> On 8/22/11 1:26 PM, Brian Stansberry wrote: >>>> On 8/2/11 4:22 PM, Jason T. Greene wrote: >>>>> Currently we have way too many read only attributes in our management >>>>> model. This makes life harder for the management tools and/or makes >>>>> the >>>>> resulting UI a bit unwieldy. >>>>> >>>>> In some cases this is just because we haven't polished subsystem >>>>> management (PLEASE all subsystem authors look at this!). In other >>>>> cases >>>>> though it's intentional because it is either difficult or currently >>>>> impossible to make hot changes to a particular attribute. This >>>>> should be >>>>> exceedingly rare. Users love hot updates, and it's a competitive >>>>> advantage. So first we should strive to make it hot in the first >>>>> place. >>>>> >>>>> However in the case that the change is not hot, and a server >>>>> reboot is >>>>> too aggressive, we should still supporting making the changes by >>>>> bouncing all services involved. After some debate on IRC we finally >>>>> arrived at what is IMO a really good option for this problem: >>>>> >>>>> write-attribute (impact-services=true|false) >>>>> >>>>> The operation handler on such a non-hot attribute would then >>>>> return an >>>>> error >>>> >>>> A handler should not return an error if a change cannot be applied to >>>> the runtime. An OperationStepHandler should use the relevant API on >>>> the >>>> OperationContext. E.g. for a handler registered for Stage.RUNTIME: >>>> >>>> @Override >>>> public void execute(OperationContext context, ModelNode operation) { >>>> >>>> context.runtimeUpdateSkipped(); >>>> context.reloadRequired(); // or context.restartRequired(); >>>> if (context.completeStep() == >>>> OperationContext.ResultAction.ROLLBACK) >>>> { >>>> context.revertReloadRequired(); >>>> // or context.revertRestartRequired(); >>>> } >>>> } >>> >>> Well in my above comment I was referring to a problem where a full >>> server reload and/or restart is too aggressive (i.e. why bounce the web >>> server when all you want to do is bounce a datasource). So the issue is >>> that these attributes don't fit that problem. >> >> To further clarify the reason to return the error is that the change to >> the configuration model is NOT APPLIED. If you touch the config without >> the runtime, then you are out of sync and the only reasonable action to >> take from that point on is to restart the server. In this case we are >> saying that changing an attribute could have hostile consequences >> (bouncing a connection pool therby affecting inbound transactions), and >> so we need some sort of confirmation by the admin that they really >> intend to do this. >> > > I see. Seems this eliminates the ability to make a bunch of config > changes without affecting the runtime and then bouncing the server to > have them take effect. Yeah that's a good point. I didn't think of that. > > It sounds like you are looking for 3 possible values for this attribute > > true -- bounce the affected services > false -- apply to config, put server in restart-required state > undefined??? -- user made a mistake, return an error Hmmm, yes I guess you could say there are three states: ALLOW_SERVICE_RESTART (affects in-flight transactions) DELAY_RESTART_UNTIL_NEXT_BOOT REJECT_SERVICE_RESTART On the other hand REJECT_SERVICE_RESTART is really a form of safety checking / "babying" that could be pushed to the client, as long as we had some kind of metadata to tell them the change affects in-flight transactions.

On 8/22/11 1:26 PM, Brian Stansberry wrote: > On 8/2/11 4:22 PM, Jason T. Greene wrote: >> Currently we have way too many read only attributes in our management >> model. This makes life harder for the management tools and/or makes the >> resulting UI a bit unwieldy. >> >> In some cases this is just because we haven't polished subsystem >> management (PLEASE all subsystem authors look at this!). In other cases >> though it's intentional because it is either difficult or currently >> impossible to make hot changes to a particular attribute. This should be >> exceedingly rare. Users love hot updates, and it's a competitive >> advantage. So first we should strive to make it hot in the first place. >> >> However in the case that the change is not hot, and a server reboot is >> too aggressive, we should still supporting making the changes by >> bouncing all services involved. After some debate on IRC we finally >> arrived at what is IMO a really good option for this problem: >> >> write-attribute (impact-services=true|false) >> >> The operation handler on such a non-hot attribute would then return an >> error > > A handler should not return an error if a change cannot be applied to > the runtime. An OperationStepHandler should use the relevant API on the > OperationContext. E.g. for a handler registered for Stage.RUNTIME: > > @Override > public void execute(OperationContext context, ModelNode operation) { > > context.runtimeUpdateSkipped(); > context.reloadRequired(); // or context.restartRequired(); > if (context.completeStep() == > OperationContext.ResultAction.ROLLBACK) > { > context.revertReloadRequired(); > // or context.revertRestartRequired(); > } > } Well in my above comment I was referring to a problem where a full server reload and/or restart is too aggressive (i.e. why bounce the web server when all you want to do is bounce a datasource). So the issue is that these attributes don't fit that problem.

On 8/22/11 1:48 PM, Brian Stansberry wrote: > On 8/22/11 1:42 PM, Jason T. Greene wrote: >> On 8/22/11 1:26 PM, Brian Stansberry wrote: >>> On 8/2/11 4:22 PM, Jason T. Greene wrote: >>>> Currently we have way too many read only attributes in our management >>>> model. This makes life harder for the management tools and/or makes >>>> the >>>> resulting UI a bit unwieldy. >>>> >>>> In some cases this is just because we haven't polished subsystem >>>> management (PLEASE all subsystem authors look at this!). In other >>>> cases >>>> though it's intentional because it is either difficult or currently >>>> impossible to make hot changes to a particular attribute. This >>>> should be >>>> exceedingly rare. Users love hot updates, and it's a competitive >>>> advantage. So first we should strive to make it hot in the first >>>> place. >>>> >>>> However in the case that the change is not hot, and a server reboot is >>>> too aggressive, we should still supporting making the changes by >>>> bouncing all services involved. After some debate on IRC we finally >>>> arrived at what is IMO a really good option for this problem: >>>> >>>> write-attribute (impact-services=true|false) >>>> >>>> The operation handler on such a non-hot attribute would then return an >>>> error >>> >>> A handler should not return an error if a change cannot be applied to >>> the runtime. An OperationStepHandler should use the relevant API on the >>> OperationContext. E.g. for a handler registered for Stage.RUNTIME: >>> >>> @Override >>> public void execute(OperationContext context, ModelNode operation) { >>> >>> context.runtimeUpdateSkipped(); >>> context.reloadRequired(); // or context.restartRequired(); >>> if (context.completeStep() == >>> OperationContext.ResultAction.ROLLBACK) >>> { >>> context.revertReloadRequired(); >>> // or context.revertRestartRequired(); >>> } >>> } >> >> Well in my above comment I was referring to a problem where a full >> server reload and/or restart is too aggressive (i.e. why bounce the web >> server when all you want to do is bounce a datasource). So the issue is >> that these attributes don't fit that problem. >> > > Sure, what you propose is the right solution for only bouncing a > datasource. But, if the operation has "impact-services" --> false, I'm > just saying the handler should not "return an error" (i.e. throw an > exception or use context.getFailureDescription().set("blah blah")). It > should properly inform the context that restart or reload is required. That was originally what I was thinking as well. The issue is that once you set that it's sticky right? (All further operations return reload/restart/etc required). Then the problem becomes how do you reset that sticky flag, and once again guarantee you are in sync.