WFCORE-693 Do not require the exclusive management lock to run the "kill" or "de… #998
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…stroy" ops on the HC
luck3y
changed the title
|
Windows Build 2005 is now running using a merge of cb90138 |
|
Linux Build 2493 is now running using a merge of cb90138 |
|
Linux Build 2493 outcome was SUCCESS using a merge of cb90138 |
|
Windows Build 2005 outcome was SUCCESS using a merge of cb90138 |
|
Core - Full Integration - Windows Build 42 outcome was FAILURE using a merge of cb90138 Failed tests |
|
WildFly pull requests Build 1913 outcome was FAILURE using a merge of cb90138 |
|
retest this please |
|
Windows Build 2006 is now running using a merge of cb90138 |
|
Linux Build 2494 is now running using a merge of cb90138 |
|
Windows Build 2006 outcome was SUCCESS using a merge of cb90138 |
|
Linux Build 2494 outcome was SUCCESS using a merge of cb90138 |
|
WildFly pull requests Build 1914 outcome was SUCCESS using a merge of cb90138 |
| context.completeStep(new OperationContext.ResultHandler() { | ||
| @Override | ||
| public void handleResult(OperationContext.ResultAction resultAction, OperationContext context, ModelNode operation) { | ||
| doExecute(serverName); |
There was a problem hiding this comment.
Feel free to ignore this comment; I'll mark this "Ready for merge" regardless.
Why the move? I'm just curious.
If this step was part of a composite, this will alter when in the sequence of events the kill/destroy happens; e.g. if the next step was then some config mod that will fail because the server isn't dead yet. But that's a silly edge case, so moving it is fine.
Since this is in a ResultHandler now, the doExecute() call can be limited to the ResultAction.KEEP case. But this op executing in a composite with something else that will trigger ResultAction.ROLLBACK is also an edge case.
There was a problem hiding this comment.
Thanks for the feedback. I ended up doing it like this as the other
examples seemed similar. Would this be more correct w.r.t. the cases you
list above:
context.authorize(operation, EnumSet.of(Action.ActionEffect.WRITE_RUNTIME));
context.completeStep(OperationContext.RollbackHandler.NOOP_ROLLBACK_HANDLER);
doExecute(serverName);
?
Thanks,
Ken
On Thu, Aug 27, 2015 at 8:51 AM, Brian Stansberry notifications@github.com
wrote:
In
host-controller/src/main/java/org/jboss/as/host/controller/operations/ServerProcessHandlers.java
#998 (comment):@@ -68,11 +71,13 @@ public void execute(final OperationContext context, final ModelNode operation) t
context.addStep(new OperationStepHandler() {
@OverRide
public void execute(OperationContext context, ModelNode operation) throws OperationFailedException {
// WFLY-2189 trigger a write-runtime authz check- context.getServiceRegistry(true);
doExecute(serverName);context.completeStep(OperationContext.RollbackHandler.NOOP_ROLLBACK_HANDLER);context.authorize(operation, EnumSet.of(Action.ActionEffect.WRITE_RUNTIME));context.completeStep(new OperationContext.ResultHandler() {@Overridepublic void handleResult(OperationContext.ResultAction resultAction, OperationContext context, ModelNode operation) {doExecute(serverName);Feel free to ignore this comment; I'll mark this "Ready for merge"
regardless.Why the move? I'm just curious.
If this step was part of a composite, this will alter when in the sequence
of events the kill/destroy happens; e.g. if the next step was then some
config mod that will fail because the server isn't dead yet. But that's a
silly edge case, so moving it is fine.Since this is in a ResultHandler now, the doExecute() call can be limited
to the ResultAction.KEEP case. But this op executing in a composite with
something else that will trigger ResultAction.ROLLBACK is also an edge case.—
Reply to this email directly or view it on GitHub
https://github.com/wildfly/wildfly-core/pull/998/files#r38096650.
bstansberry
added
the
ready-for-merge
jamezp
added a commit
to jamezp/wildfly-core
that referenced
this pull request
Aug 27, 2015
Do not require the exclusive management lock to run the "kill" or "destroy" ops on the HC
jamezp
added a commit
that referenced
this pull request
Aug 28, 2015
WFCORE-693 Do not require the exclusive management lock to run the "kill" or "de…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Don't aquire exclusive lock for server kill / shutdown ops.