Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/614#discussion_r29396975
  
    --- Diff: 
usage/rest-server/src/main/java/brooklyn/rest/filter/HaHotCheckResourceFilter.java
 ---
    @@ -74,39 +89,54 @@ public ContainerResponseFilter getResponseFilter() {
                 return null;
             }
     
    +        private String lookForProblem(ContainerRequest request) {
    +            if (isSkipCheckHeaderSet(request)) 
    +                return null;
    +            
    +            if (!isHaHotStateRequired(request))
    +                return null;
    +            
    +            String problem = 
HaMasterCheckFilter.lookForProblemIfServerNotRunning(mgmt);
    +            if (Strings.isNonBlank(problem)) 
    +                return problem;
    +            
    +            if (!isHaHotStatus())
    +                return "server not in required HA hot state";
    +            if (isStateNotYetValid())
    +                return "server not yet completed loading data for required 
HA hot state";
    +            
    +            return null;
    +        }
    +        
             @Override
             public ContainerRequest filter(ContainerRequest request) {
    -            if (!isStateLoaded() && isUnsafe(request)) {
    -                log.warn("Disallowed request to standby brooklyn: 
"+request+"/"+am+" (caller should set 
'"+HaMasterCheckFilter.SKIP_CHECK_HEADER+"' to force)");
    +            String problem = lookForProblem(request);
    +            if (Strings.isNonBlank(problem)) {
    +                log.warn("Disallowing request as "+problem+": 
"+request+"/"+am+" (caller should set 
'"+HaMasterCheckFilter.SKIP_CHECK_HEADER+"' to force)");
                     throw new WebApplicationException(ApiError.builder()
    -                    .message("This request is not permitted against a 
standby Brooklyn server")
    +                    .message("This request is only permitted against an 
active hot Brooklyn server")
                         
.errorCode(Response.Status.FORBIDDEN).build().asJsonResponse());
                 }
                 return request;
             }
     
    -        private boolean isStateLoaded() {
    -            return isHaHotStatusOrDisabled() && 
!RebindTracker.isRebinding() && !recentlySwitchedState();
    +        // Maybe there should be a separate state to indicate that we have 
switched state
    +        // but still haven't finished rebinding. (Previously there was a 
time delay and an
    +        // isRebinding check, but introducing 
RebindManager#isAwaitingInitialRebind() seems cleaner.)
    +        private boolean isStateNotYetValid() {
    +            return mgmt.getRebindManager().isAwaitingInitialRebind();
             }
     
    -        // Ideally there will be a separate state to indicate that we 
switched state
    -        // but still haven't finished rebinding. There's a gap between 
changing the state
    -        // and starting rebind so add a time offset just to be sure.
    -        private boolean recentlySwitchedState() {
    -            long lastStateChange = 
mgmt.getHighAvailabilityManager().getLastStateChange();
    -            if (lastStateChange == -1) return false;
    -            return System.currentTimeMillis() - lastStateChange < 
STATE_CHANGE_SETTLE_OFFSET;
    +        private boolean isHaHotStateRequired(ContainerRequest request) {
    +            return (am.getAnnotation(HaHotStateRequired.class) != null ||
    +                    
am.getResource().getAnnotation(HaHotStateRequired.class) != null);
             }
     
    -        private boolean isUnsafe(ContainerRequest request) {
    -            boolean isOverriden = 
"true".equalsIgnoreCase(request.getHeaderValue(HaMasterCheckFilter.SKIP_CHECK_HEADER));
    -            return !isOverriden &&
    -                    (am.getAnnotation(HaHotStateRequired.class) != null ||
    -                    
am.getResource().getAnnotation(HaHotStateRequired.class) != null);
    +        private boolean isSkipCheckHeaderSet(ContainerRequest request) {
    +            return 
"true".equalsIgnoreCase(request.getHeaderValue(HaMasterCheckFilter.SKIP_CHECK_HEADER));
             }
     
    -        private boolean isHaHotStatusOrDisabled() {
    -            if (!mgmt.getHighAvailabilityManager().isRunning()) return 
true;
    --- End diff --
    
    node state is automatically master when HA mgr is disabled if node is up.  
that has been the behaviour.  i think it makes sense.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

Reply via email to