You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2017/09/19 14:24:09 UTC

[GitHub] brooklyn-server pull request #832: Add ManagementNodeStateListener support

GitHub user aledsage opened a pull request:

    https://github.com/apache/brooklyn-server/pull/832

    Add ManagementNodeStateListener support

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aledsage/brooklyn-server add-managementNodeStateListener

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/brooklyn-server/pull/832.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #832
    
----
commit cb14ecee0dbe7cd6ff5fdb8a2247b887dc06f99b
Author: Aled Sage <al...@gmail.com>
Date:   2017-09-19T14:20:02Z

    Adds TypeCoercion utility ‘registerInstanceForClassnameAdapter’

commit 32bd96535e7e6b1c43314b24c2ffa7f1b777a058
Author: Aled Sage <al...@gmail.com>
Date:   2017-09-19T14:21:58Z

    Adds ManagementNodeStateListener support

----


---

[GitHub] brooklyn-server issue #832: Add ManagementNodeStateListener support

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/832
  
    jenkins build failed with:
    ```
    [ERROR] Failed to execute goal org.apache.maven.plugins:maven-install-plugin:2.5.2:install (default-install) on project brooklyn-core: Failed to install artifact org.apache.brooklyn:brooklyn-core:jar:tests:0.13.0-SNAPSHOT: No space left on device -> [Help 1]
    ```


---

[GitHub] brooklyn-server pull request #832: Add ManagementNodeStateListener support

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/832#discussion_r139803014
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/ManagementNodeStateListenerManager.java ---
    @@ -0,0 +1,175 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.mgmt.internal;
    +
    +import static com.google.common.base.Preconditions.checkNotNull;
    +
    +import java.io.Closeable;
    +import java.io.IOException;
    +import java.util.Collection;
    +import java.util.List;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.TimeUnit;
    +import java.util.concurrent.atomic.AtomicInteger;
    +
    +import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState;
    +import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
    +import org.apache.brooklyn.core.mgmt.usage.ManagementNodeStateListener;
    +import org.apache.brooklyn.core.server.BrooklynServerConfig;
    +import org.apache.brooklyn.util.core.ClassLoaderUtils;
    +import org.apache.brooklyn.util.core.flags.TypeCoercions;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.time.Duration;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import com.google.common.base.Function;
    +import com.google.common.collect.Lists;
    +import com.google.common.util.concurrent.Futures;
    +import com.google.common.util.concurrent.ListenableFuture;
    +import com.google.common.util.concurrent.ListeningExecutorService;
    +import com.google.common.util.concurrent.MoreExecutors;
    +import com.google.common.util.concurrent.ThreadFactoryBuilder;
    +
    +/**
    + * Handles the notification of {@link ManagementNodeStateListener}s.
    + * 
    + * @see {@link BrooklynServerConfig#MANAGEMENT_NODE_STATE_LISTENERS} for configuring this.
    + * @see {@link org.apache.brooklyn.core.mgmt.ha.HighAvailabilityManagerImpl#HighAvailabilityManagerImpl(ManagementContextInternal, ManagementNodeStateListener)}
    + *      for how we get notified of the state-change.
    + */
    +public class ManagementNodeStateListenerManager implements ManagementNodeStateListener {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(ManagementNodeStateListenerManager.class);
    +
    +    private final ManagementContextInternal mgmt;
    +    
    +    private final Object mutex = new Object();
    +
    +    private final List<ManagementNodeStateListener> listeners = Lists.newCopyOnWriteArrayList();
    +    private ManagementNodeState lastPublishedVal;
    +    
    +    private final AtomicInteger listenerQueueSize = new AtomicInteger();
    +    
    +    private ListeningExecutorService listenerExecutor = MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor(new ThreadFactoryBuilder()
    +            .setNameFormat("brooklyn-managementnodestate-listener-%d")
    +            .build()));
    +
    +    public ManagementNodeStateListenerManager(ManagementContextInternal managementContext) {
    +        this.mgmt = checkNotNull(managementContext, "managementContext");
    +        
    +        // Register a coercion from String->ManagementNodeStateListener, so that MANAGEMENT_NODE_STATE_LISTENERS defined in brooklyn.cfg
    +        // will be instantiated, given their class names.
    +        TypeCoercions.BrooklynCommonAdaptorTypeCoercions.registerInstanceForClassnameAdapter(
    +                new ClassLoaderUtils(this.getClass(), managementContext), 
    +                ManagementNodeStateListener.class);
    +
    +        // Although changing listeners to Collection<ManagementNodeStateListener> is valid at compile time
    +        // the collection will contain any objects that could not be coerced by the function
    +        // declared above. Generally this means any string declared in brooklyn.properties
    +        // that is not a ManagementNodeStateListener.
    +        Collection<?> rawListeners = managementContext.getBrooklynProperties().getConfig(BrooklynServerConfig.MANAGEMENT_NODE_STATE_LISTENERS);
    +        if (rawListeners != null) {
    +            for (Object obj : rawListeners) {
    +                if (obj == null) {
    +                    throw new NullPointerException("null listener in config " + BrooklynServerConfig.MANAGEMENT_NODE_STATE_LISTENERS);
    +                } else if (!(obj instanceof ManagementNodeStateListener)) {
    +                    throw new ClassCastException("Configured object is not a "+ManagementNodeStateListener.class.getSimpleName()+". This probably means coercion failed: " + obj);
    +                } else {
    +                    ManagementNodeStateListener listener = (ManagementNodeStateListener) obj;
    +                    if (listener instanceof ManagementContextInjectable) {
    +                        ((ManagementContextInjectable) listener).setManagementContext(managementContext);
    +                    }
    +                    listeners.add((ManagementNodeStateListener)listener);
    +                }
    +            }
    +        }
    +    }
    +
    +    @Override
    +    public void onStateChange(ManagementNodeState state) {
    --- End diff --
    
    Agreed it's a bit strange. But I didn't want to complicate the `HighAvailabilityManagerImpl` more. It would only be a few lines in there (a new field to store the last value, etc) - I could move it there if you think it's better @Graeme-Miller?


---

[GitHub] brooklyn-server issue #832: Add ManagementNodeStateListener support

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/832
  
    retest this please


---

[GitHub] brooklyn-server pull request #832: Add ManagementNodeStateListener support

Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/832#discussion_r139721438
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/ManagementNodeStateListenerManager.java ---
    @@ -0,0 +1,175 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.mgmt.internal;
    +
    +import static com.google.common.base.Preconditions.checkNotNull;
    +
    +import java.io.Closeable;
    +import java.io.IOException;
    +import java.util.Collection;
    +import java.util.List;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.TimeUnit;
    +import java.util.concurrent.atomic.AtomicInteger;
    +
    +import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState;
    +import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
    +import org.apache.brooklyn.core.mgmt.usage.ManagementNodeStateListener;
    +import org.apache.brooklyn.core.server.BrooklynServerConfig;
    +import org.apache.brooklyn.util.core.ClassLoaderUtils;
    +import org.apache.brooklyn.util.core.flags.TypeCoercions;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.time.Duration;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import com.google.common.base.Function;
    +import com.google.common.collect.Lists;
    +import com.google.common.util.concurrent.Futures;
    +import com.google.common.util.concurrent.ListenableFuture;
    +import com.google.common.util.concurrent.ListeningExecutorService;
    +import com.google.common.util.concurrent.MoreExecutors;
    +import com.google.common.util.concurrent.ThreadFactoryBuilder;
    +
    +/**
    + * Handles the notification of {@link ManagementNodeStateListener}s.
    + * 
    + * @see {@link BrooklynServerConfig#MANAGEMENT_NODE_STATE_LISTENERS} for configuring this.
    + * @see {@link org.apache.brooklyn.core.mgmt.ha.HighAvailabilityManagerImpl#HighAvailabilityManagerImpl(ManagementContextInternal, ManagementNodeStateListener)}
    + *      for how we get notified of the state-change.
    + */
    +public class ManagementNodeStateListenerManager implements ManagementNodeStateListener {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(ManagementNodeStateListenerManager.class);
    +
    +    private final ManagementContextInternal mgmt;
    +    
    +    private final Object mutex = new Object();
    +
    +    private final List<ManagementNodeStateListener> listeners = Lists.newCopyOnWriteArrayList();
    +    private ManagementNodeState lastPublishedVal;
    +    
    +    private final AtomicInteger listenerQueueSize = new AtomicInteger();
    +    
    +    private ListeningExecutorService listenerExecutor = MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor(new ThreadFactoryBuilder()
    +            .setNameFormat("brooklyn-managementnodestate-listener-%d")
    +            .build()));
    +
    +    public ManagementNodeStateListenerManager(ManagementContextInternal managementContext) {
    +        this.mgmt = checkNotNull(managementContext, "managementContext");
    +        
    +        // Register a coercion from String->ManagementNodeStateListener, so that MANAGEMENT_NODE_STATE_LISTENERS defined in brooklyn.cfg
    +        // will be instantiated, given their class names.
    +        TypeCoercions.BrooklynCommonAdaptorTypeCoercions.registerInstanceForClassnameAdapter(
    +                new ClassLoaderUtils(this.getClass(), managementContext), 
    +                ManagementNodeStateListener.class);
    +
    +        // Although changing listeners to Collection<ManagementNodeStateListener> is valid at compile time
    +        // the collection will contain any objects that could not be coerced by the function
    +        // declared above. Generally this means any string declared in brooklyn.properties
    +        // that is not a ManagementNodeStateListener.
    +        Collection<?> rawListeners = managementContext.getBrooklynProperties().getConfig(BrooklynServerConfig.MANAGEMENT_NODE_STATE_LISTENERS);
    +        if (rawListeners != null) {
    +            for (Object obj : rawListeners) {
    +                if (obj == null) {
    +                    throw new NullPointerException("null listener in config " + BrooklynServerConfig.MANAGEMENT_NODE_STATE_LISTENERS);
    +                } else if (!(obj instanceof ManagementNodeStateListener)) {
    +                    throw new ClassCastException("Configured object is not a "+ManagementNodeStateListener.class.getSimpleName()+". This probably means coercion failed: " + obj);
    +                } else {
    +                    ManagementNodeStateListener listener = (ManagementNodeStateListener) obj;
    +                    if (listener instanceof ManagementContextInjectable) {
    +                        ((ManagementContextInjectable) listener).setManagementContext(managementContext);
    +                    }
    +                    listeners.add((ManagementNodeStateListener)listener);
    +                }
    +            }
    +        }
    +    }
    +
    +    @Override
    +    public void onStateChange(ManagementNodeState state) {
    --- End diff --
    
    seems a bit strange that we can call onStateChange with the same state and have the ManagementNodeStateListenerManager filter it out. Feel like it should be the responsibility of the HighAvailabilityManagerImpl, but it is neat to include it in ManagementNodeStateListenerManager


---

[GitHub] brooklyn-server pull request #832: Add ManagementNodeStateListener support

Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:

    https://github.com/apache/brooklyn-server/pull/832#discussion_r139903337
  
    --- Diff: core/src/main/java/org/apache/brooklyn/core/mgmt/internal/ManagementNodeStateListenerManager.java ---
    @@ -0,0 +1,175 @@
    +/*
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing,
    + * software distributed under the License is distributed on an
    + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    + * KIND, either express or implied.  See the License for the
    + * specific language governing permissions and limitations
    + * under the License.
    + */
    +package org.apache.brooklyn.core.mgmt.internal;
    +
    +import static com.google.common.base.Preconditions.checkNotNull;
    +
    +import java.io.Closeable;
    +import java.io.IOException;
    +import java.util.Collection;
    +import java.util.List;
    +import java.util.concurrent.Executors;
    +import java.util.concurrent.TimeUnit;
    +import java.util.concurrent.atomic.AtomicInteger;
    +
    +import org.apache.brooklyn.api.mgmt.ha.ManagementNodeState;
    +import org.apache.brooklyn.core.mgmt.ManagementContextInjectable;
    +import org.apache.brooklyn.core.mgmt.usage.ManagementNodeStateListener;
    +import org.apache.brooklyn.core.server.BrooklynServerConfig;
    +import org.apache.brooklyn.util.core.ClassLoaderUtils;
    +import org.apache.brooklyn.util.core.flags.TypeCoercions;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.apache.brooklyn.util.time.Duration;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +import com.google.common.base.Function;
    +import com.google.common.collect.Lists;
    +import com.google.common.util.concurrent.Futures;
    +import com.google.common.util.concurrent.ListenableFuture;
    +import com.google.common.util.concurrent.ListeningExecutorService;
    +import com.google.common.util.concurrent.MoreExecutors;
    +import com.google.common.util.concurrent.ThreadFactoryBuilder;
    +
    +/**
    + * Handles the notification of {@link ManagementNodeStateListener}s.
    + * 
    + * @see {@link BrooklynServerConfig#MANAGEMENT_NODE_STATE_LISTENERS} for configuring this.
    + * @see {@link org.apache.brooklyn.core.mgmt.ha.HighAvailabilityManagerImpl#HighAvailabilityManagerImpl(ManagementContextInternal, ManagementNodeStateListener)}
    + *      for how we get notified of the state-change.
    + */
    +public class ManagementNodeStateListenerManager implements ManagementNodeStateListener {
    +
    +    private static final Logger LOG = LoggerFactory.getLogger(ManagementNodeStateListenerManager.class);
    +
    +    private final ManagementContextInternal mgmt;
    +    
    +    private final Object mutex = new Object();
    +
    +    private final List<ManagementNodeStateListener> listeners = Lists.newCopyOnWriteArrayList();
    +    private ManagementNodeState lastPublishedVal;
    +    
    +    private final AtomicInteger listenerQueueSize = new AtomicInteger();
    +    
    +    private ListeningExecutorService listenerExecutor = MoreExecutors.listeningDecorator(Executors.newSingleThreadExecutor(new ThreadFactoryBuilder()
    +            .setNameFormat("brooklyn-managementnodestate-listener-%d")
    +            .build()));
    +
    +    public ManagementNodeStateListenerManager(ManagementContextInternal managementContext) {
    +        this.mgmt = checkNotNull(managementContext, "managementContext");
    +        
    +        // Register a coercion from String->ManagementNodeStateListener, so that MANAGEMENT_NODE_STATE_LISTENERS defined in brooklyn.cfg
    +        // will be instantiated, given their class names.
    +        TypeCoercions.BrooklynCommonAdaptorTypeCoercions.registerInstanceForClassnameAdapter(
    +                new ClassLoaderUtils(this.getClass(), managementContext), 
    +                ManagementNodeStateListener.class);
    +
    +        // Although changing listeners to Collection<ManagementNodeStateListener> is valid at compile time
    +        // the collection will contain any objects that could not be coerced by the function
    +        // declared above. Generally this means any string declared in brooklyn.properties
    +        // that is not a ManagementNodeStateListener.
    +        Collection<?> rawListeners = managementContext.getBrooklynProperties().getConfig(BrooklynServerConfig.MANAGEMENT_NODE_STATE_LISTENERS);
    +        if (rawListeners != null) {
    +            for (Object obj : rawListeners) {
    +                if (obj == null) {
    +                    throw new NullPointerException("null listener in config " + BrooklynServerConfig.MANAGEMENT_NODE_STATE_LISTENERS);
    +                } else if (!(obj instanceof ManagementNodeStateListener)) {
    +                    throw new ClassCastException("Configured object is not a "+ManagementNodeStateListener.class.getSimpleName()+". This probably means coercion failed: " + obj);
    +                } else {
    +                    ManagementNodeStateListener listener = (ManagementNodeStateListener) obj;
    +                    if (listener instanceof ManagementContextInjectable) {
    +                        ((ManagementContextInjectable) listener).setManagementContext(managementContext);
    +                    }
    +                    listeners.add((ManagementNodeStateListener)listener);
    +                }
    +            }
    +        }
    +    }
    +
    +    @Override
    +    public void onStateChange(ManagementNodeState state) {
    --- End diff --
    
    not sure, but I think it is fine as it is. Your call.


---

[GitHub] brooklyn-server issue #832: Add ManagementNodeStateListener support

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:

    https://github.com/apache/brooklyn-server/pull/832
  
    retest this please


---

[GitHub] brooklyn-server pull request #832: Add ManagementNodeStateListener support

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/brooklyn-server/pull/832


---