You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2019/12/18 16:40:25 UTC

[GitHub] [lucene-solr] sigram opened a new pull request #1100: SOLR-13579: Create resource management API

sigram opened a new pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100
 
 
   Please see JIRA for details.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r360282511
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/ResourceManagerPool.java
 ##########
 @@ -0,0 +1,216 @@
+package org.apache.solr.managed;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ *
+ */
+public abstract class ResourceManagerPool<T extends ManagedComponent> implements Closeable {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  protected final String name;
+  protected final String type;
+  protected Map<String, Object> poolLimits;
+  protected final Map<String, T> components = new ConcurrentHashMap<>();
+  protected final ResourceManager resourceManager;
+  protected final Class<? extends ManagedComponent> componentClass;
+  private final Map<String, Object> poolParams;
+  protected final ResourcePoolContext poolContext = new ResourcePoolContext();
+  protected final List<ChangeListener> listeners = new ArrayList<>();
+  protected final ReentrantLock updateLock = new ReentrantLock();
+  protected int scheduleDelaySeconds;
+  protected ScheduledFuture<?> scheduledFuture;
+
+  public ResourceManagerPool(String name, String type, ResourceManager resourceManager,
+                                Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    this.name = name;
+    this.type = type;
+    this.resourceManager = resourceManager;
+    this.componentClass = resourceManager.getResourceManagerPoolFactory().getComponentClassByType(type);
+    this.poolLimits = new HashMap<>(poolLimits);
+    this.poolParams = new HashMap<>(poolParams);
+  }
+
+  /** Unique pool name. */
+  public String getName() {
+    return name;
+  }
+
+  /** Pool type. */
+  public String getType() {
+    return type;
+  }
+
+  public ResourceManager getResourceManager() {
+    return resourceManager;
+  }
+
+  /** Add component to this pool. */
+  public void registerComponent(T managedComponent) {
+    if (!componentClass.isAssignableFrom(managedComponent.getClass())) {
+      log.debug("Pool type '" + type + "' is not supported by the component " + managedComponent.getManagedComponentId());
+      return;
+    }
+    ManagedComponent existing = components.putIfAbsent(managedComponent.getManagedComponentId().toString(), managedComponent);
+    if (existing != null) {
+      throw new IllegalArgumentException("Component '" + managedComponent.getManagedComponentId() + "' already exists in pool '" + name + "' !");
+    }
+  }
+
+  /** Remove named component from this pool. */
+  public boolean unregisterComponent(String componentId) {
+    return components.remove(name) != null;
+  }
+
+  /**
+   * Check whether a named component is registered in this pool.
+   * @param componentId component id
+   * @return true if the component with this name is registered, false otherwise.
+   */
+  public boolean isRegistered(String componentId) {
+    return components.containsKey(componentId);
+  }
+
+  /** Get components managed by this pool. */
+  public Map<String, T> getComponents() {
+    return Collections.unmodifiableMap(components);
+  }
+
+  public void addChangeListener(ChangeListener listener) {
+    if (!listeners.contains(listener)) {
+      listeners.add(listener);
+    }
+  }
+
+  public void removeChangeListener(ChangeListener listener) {
+    listeners.remove(listener);
+  }
+
+
+  /**
+   * Get the current monitored values from all resources. Result is a map with resource names as keys,
+   * and param/value maps as values.
+   */
+  public Map<String, Map<String, Object>> getCurrentValues() throws InterruptedException {
+    updateLock.lockInterruptibly();
 
 Review comment:
   I'm having trouble understanding why the updateLock is necessary in this method? It is iterating over the values of a concurrent hash map (`components`) and reading the id and monitored value of the component. Both seem to be thread-safe to me.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r363875456
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/types/CacheManagerPool.java
 ##########
 @@ -0,0 +1,329 @@
+/*
+ * 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.solr.managed.types;
+
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+import org.apache.solr.managed.ChangeListener;
+import org.apache.solr.managed.ResourceManager;
+import org.apache.solr.managed.ResourceManagerPool;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.search.SolrCache;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * An implementation of {@link org.apache.solr.managed.ResourceManagerPool} specific to
+ * the management of {@link org.apache.solr.search.SolrCache} instances.
+ * <p>This plugin calculates the total size and maxRamMB of all registered cache instances
+ * and adjusts each cache's limits so that the aggregated values again fit within the pool limits.</p>
+ * <p>In order to avoid thrashing the plugin uses a dead band (by default {@link #DEFAULT_DEAD_BAND}),
+ * which can be adjusted using configuration parameter {@link #DEAD_BAND_PARAM}. If monitored values don't
+ * exceed the limits +/- the dead band then no forcible adjustment takes place.</p>
+ * <p>The management strategy consists of two distinct phases: soft optimization phase and then hard limit phase.</p>
+ * <p><b>Soft optimization</b> tries to adjust the resource consumption based on the cache hit ratio.
+ * This phase is executed only if there's no total limit exceeded. Also, hit ratio is considered a valid monitored
+ * variable only when at least N lookups occurred since the last adjustment (default value is {@link #DEFAULT_LOOKUP_DELTA}).
+ * If the hit ratio is higher than a threshold (default value is {@link #DEFAULT_TARGET_HITRATIO}) then the size
+ * of the cache can be reduced so that the resource consumption is minimized while still keeping acceptable hit
+ * ratio - and vice versa.</p>
+ * <p>This optimization phase can only adjust the limits within a {@link #DEFAULT_MAX_ADJUST_RATIO}, i.e. increased
+ * or decreased values may not be larger / smaller than this multiple / fraction of the initially configured limit.</p>
+ * <p><b>Hard limit</b> phase follows the soft optimization phase and it forcibly reduces resource consumption of all components
+ * if the total usage is still above the pool limit after the first phase has completed. Each component's limit is reduced
+ * by the same factor, regardless of the actual population or hit ratio.</p>
+ */
+public class CacheManagerPool extends ResourceManagerPool<SolrCache> {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static String TYPE = "cache";
+
+  /** Controller dead-band - changes smaller than this ratio will be ignored. */
+  public static final String DEAD_BAND_PARAM = "deadBand";
+  /** Target hit ratio - high enough to be useful, low enough to avoid excessive cache size. */
+  public static final String TARGET_HIT_RATIO_PARAM = "targetHitRatio";
+  /**
+   * Maximum allowed adjustment ratio from the initial configuration value. Adjusted value may not be
+   * higher than multiple of this factor, and not lower than divided by this factor.
+   */
+  public static final String MAX_ADJUST_RATIO_PARAM = "maxAdjustRatio";
+  /**
+   * Minimum number of lookups since last adjustment to consider the reported hitRatio
+   *  to be statistically valid.
+   */
+  public static final String MIN_LOOKUP_DELTA_PARAM = "minLookupDelta";
+  /** Default value of dead band (10%). */
+  public static final double DEFAULT_DEAD_BAND = 0.1;
+  /** Default target hit ratio - a compromise between usefulness and limited resource usage. */
+  public static final double DEFAULT_TARGET_HITRATIO = 0.8;
+  /**
+   * Default minimum number of lookups since the last adjustment. This can be treated as Bernoulli trials
+   * that give a 5% confidence about the statistical validity of hit ratio (<code>0.5 / sqrt(lookups)</code>).
+   */
+  public static final long DEFAULT_LOOKUP_DELTA = 100;
+  /**
+   * Default maximum adjustment ratio from the initially configured values.
+   */
+  public static final double DEFAULT_MAX_ADJUST_RATIO = 2.0;
+
+  protected static final Map<String, Function<Map<String, Object>, Double>> controlledToMonitored = new HashMap<>();
+
+  static {
+    controlledToMonitored.put(SolrCache.MAX_RAM_MB_PARAM, values -> {
+      Number ramBytes = (Number) values.get(SolrCache.RAM_BYTES_USED_PARAM);
+      return ramBytes != null ? ramBytes.doubleValue() / SolrCache.MB : 0.0;
+    });
+    controlledToMonitored.put(SolrCache.MAX_SIZE_PARAM, values ->
+        ((Number)values.getOrDefault(SolrCache.SIZE_PARAM, -1.0)).doubleValue());
+  }
+
+  protected double deadBand = DEFAULT_DEAD_BAND;
+  protected double targetHitRatio = DEFAULT_TARGET_HITRATIO;
+  protected long lookupDelta = DEFAULT_LOOKUP_DELTA;
+  protected double maxAdjustRatio = DEFAULT_MAX_ADJUST_RATIO;
+  protected Map<String, Long> lookups = new HashMap<>();
+  protected Map<String, Map<String, Object>> initialComponentLimits = new HashMap<>();
+
+  public CacheManagerPool(String name, String type, ResourceManager resourceManager, Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    super(name, type, resourceManager, poolLimits, poolParams);
+    String str = String.valueOf(poolParams.getOrDefault(DEAD_BAND_PARAM, DEFAULT_DEAD_BAND));
+    try {
+      deadBand = Double.parseDouble(str);
+    } catch (Exception e) {
+      log.warn("Invalid deadBand parameter value '" + str + "', using default " + DEFAULT_DEAD_BAND);
+    }
+  }
+
+  @Override
+  public void registerComponent(SolrCache component) {
+    super.registerComponent(component);
+    initialComponentLimits.put(component.getManagedComponentId().toString(), getResourceLimits(component));
+  }
+
+  @Override
+  public boolean unregisterComponent(String componentId) {
+    lookups.remove(componentId);
+    initialComponentLimits.remove(componentId);
+    return super.unregisterComponent(componentId);
+  }
+
+  @Override
+  public Object doSetResourceLimit(SolrCache component, String limitName, Object val) {
+    if (!(val instanceof Number)) {
+      try {
+        val = Long.parseLong(String.valueOf(val));
+      } catch (Exception e) {
+        throw new IllegalArgumentException("Unsupported value type (not a number) for limit '" + limitName + "': " + val + " (" + val.getClass().getName() + ")");
+      }
+    }
+    Number value = (Number)val;
+    if (value.longValue() > Integer.MAX_VALUE) {
+      throw new IllegalArgumentException("Invalid new value for limit '" + limitName +"': " + value);
+    }
+    switch (limitName) {
+      case SolrCache.MAX_SIZE_PARAM:
+        component.setMaxSize(value.intValue());
+        break;
+      case SolrCache.MAX_RAM_MB_PARAM:
+        component.setMaxRamMB(value.intValue());
+        break;
+      default:
+        throw new IllegalArgumentException("Unsupported limit name '" + limitName + "'");
+    }
+    return value.intValue();
+  }
+
+  @Override
+  public Map<String, Object> getResourceLimits(SolrCache component) {
+    Map<String, Object> limits = new HashMap<>();
+    limits.put(SolrCache.MAX_SIZE_PARAM, component.getMaxSize());
+    limits.put(SolrCache.MAX_RAM_MB_PARAM, component.getMaxRamMB());
+    return limits;
+  }
+
+  @Override
+  public Map<String, Object> getMonitoredValues(SolrCache component) throws Exception {
+    Map<String, Object> values = new HashMap<>();
+    values.put(SolrCache.SIZE_PARAM, component.size());
+    values.put(SolrCache.RAM_BYTES_USED_PARAM, component.ramBytesUsed());
+    SolrMetricsContext metricsContext = component.getSolrMetricsContext();
+    if (metricsContext != null) {
+      Map<String, Object> metrics = metricsContext.getMetricsSnapshot();
+      String key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.HIT_RATIO_PARAM;
+      values.put(SolrCache.HIT_RATIO_PARAM, metrics.get(key));
+      key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.LOOKUPS_PARAM;
+      values.put(SolrCache.LOOKUPS_PARAM, metrics.get(key));
+    }
+    return values;
+  }
+
+  @Override
+  protected void doManage() throws Exception {
+    Map<String, Map<String, Object>> currentValues = getCurrentValues();
+    Map<String, Object> totalValues = aggregateTotalValues(currentValues);
+    // pool limits are defined using controlled tags
+    poolLimits.forEach((poolLimitName, value) -> {
+      // only numeric limits are supported
+      if (value == null || !(value instanceof Number)) {
+        return;
+      }
+      double poolLimitValue = ((Number)value).doubleValue();
+      if (poolLimitValue <= 0) {
+        return;
+      }
+      Function<Map<String, Object>, Double> func = controlledToMonitored.get(poolLimitName);
+      if (func == null) {
+        return;
+      }
+      Double totalValue = func.apply(totalValues);
+      if (totalValue.doubleValue() <= 0.0) {
+        return;
+      }
+      double totalDelta = poolLimitValue - totalValue.doubleValue();
+
+      // dead band to avoid thrashing
+      if (Math.abs(totalDelta / poolLimitValue) < deadBand) {
+        return;
+      }
+
+      List<SolrCache> adjustableComponents = new ArrayList<>();
+      components.forEach((name, component) -> {
+        Map<String, Object> resourceLimits = getResourceLimits((SolrCache) component);
+        Object limit = resourceLimits.get(poolLimitName);
+        // XXX we could attempt here to control eg. ramBytesUsed by adjusting maxSize limit
+        // XXX and vice versa if the current limit is undefined or unsupported
+        if (limit == null || !(limit instanceof Number)) {
+          return;
+        }
+        double currentResourceLimit = ((Number)limit).doubleValue();
+        if (currentResourceLimit <= 0) { // undefined or unsupported
+          return;
+        }
+        adjustableComponents.add(component);
+      });
+      optimize(adjustableComponents, currentValues, poolLimitName, poolLimitValue, totalValue.doubleValue());
+    });
+  }
+
+  /**
+   * Manage all eligible components that support this pool limit.
+   */
+  private void optimize(List<SolrCache> components, Map<String, Map<String, Object>> currentValues, String limitName,
+                        double poolLimitValue, double totalValue) {
+    // changeRatio > 1.0 means there are available free resources
+    // changeRatio < 1.0 means there's shortage of resources
+    final AtomicReference<Double> changeRatio = new AtomicReference<>(poolLimitValue / totalValue);
+
+    // ========================== OPTIMIZATION ==============================
+    // if the situation is not critical (ie. total consumption is less than max)
+    // try to proactively optimize by reducing the size of caches with too high hitRatio
+    // (because a lower hit ratio is still acceptable if it means saving resources) and
+    // expand the size of caches with too low hitRatio
+    final AtomicReference<Double> newTotalValue = new AtomicReference<>(totalValue);
+    components.forEach(component -> {
+      long currentLookups = ((Number)currentValues.get(component.getManagedComponentId().toString()).get(SolrCache.LOOKUPS_PARAM)).longValue();
+      long lastLookups = lookups.computeIfAbsent(component.getManagedComponentId().toString(), k -> 0L);
+      if (currentLookups < lastLookups + lookupDelta) {
+        // too little data, skip the optimization
+        return;
+      }
+      Map<String, Object> resourceLimits = getResourceLimits(component);
+      double currentLimit = ((Number)resourceLimits.get(limitName)).doubleValue();
+      double currentHitRatio = ((Number)currentValues.get(component.getManagedComponentId().toString()).get(SolrCache.HIT_RATIO_PARAM)).doubleValue();
+      Number initialLimit = (Number)initialComponentLimits.get(component.getManagedComponentId().toString()).get(limitName);
+      if (initialLimit == null) {
+        // can't optimize because we don't know how far off we are from the initial setting
+        return;
+      }
+      if (currentHitRatio < targetHitRatio) {
+        if (changeRatio.get() < 1.0) {
+          // don't expand if we're already short on resources
+          return;
+        }
+        // expand to increase the hitRatio, but not more than maxAdjustRatio from the initialLimit
+        double newLimit = currentLimit * changeRatio.get();
 
 Review comment:
   Good point, fixed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359739291
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
+
+  protected ResourceManagerPoolFactory resourceManagerPoolFactory;
+  protected SolrResourceLoader loader;
+
+
+  public DefaultResourceManager(SolrResourceLoader loader, TimeSource timeSource) {
+    this.loader = loader;
+    this.timeSource = timeSource;
+  }
+
+  protected void doInit() throws Exception {
+    scheduledThreadPoolExecutor = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(maxNumPools,
 
 Review comment:
   Using maxNumPools as the core pool size is a bit excessive because this will keep that many threads running even when idle. We should either use a small number as the core pool size and set max pool size to `maxNumPools` or we set `allowCoreThreadTimeOut=true` and set a positive `keepAliveTime`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r360285223
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/ResourceManagerPool.java
 ##########
 @@ -0,0 +1,216 @@
+package org.apache.solr.managed;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ *
+ */
+public abstract class ResourceManagerPool<T extends ManagedComponent> implements Closeable {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  protected final String name;
+  protected final String type;
+  protected Map<String, Object> poolLimits;
+  protected final Map<String, T> components = new ConcurrentHashMap<>();
+  protected final ResourceManager resourceManager;
+  protected final Class<? extends ManagedComponent> componentClass;
+  private final Map<String, Object> poolParams;
+  protected final ResourcePoolContext poolContext = new ResourcePoolContext();
+  protected final List<ChangeListener> listeners = new ArrayList<>();
+  protected final ReentrantLock updateLock = new ReentrantLock();
+  protected int scheduleDelaySeconds;
+  protected ScheduledFuture<?> scheduledFuture;
+
+  public ResourceManagerPool(String name, String type, ResourceManager resourceManager,
+                                Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    this.name = name;
+    this.type = type;
+    this.resourceManager = resourceManager;
+    this.componentClass = resourceManager.getResourceManagerPoolFactory().getComponentClassByType(type);
+    this.poolLimits = new HashMap<>(poolLimits);
+    this.poolParams = new HashMap<>(poolParams);
+  }
+
+  /** Unique pool name. */
+  public String getName() {
+    return name;
+  }
+
+  /** Pool type. */
+  public String getType() {
+    return type;
+  }
+
+  public ResourceManager getResourceManager() {
+    return resourceManager;
+  }
+
+  /** Add component to this pool. */
+  public void registerComponent(T managedComponent) {
+    if (!componentClass.isAssignableFrom(managedComponent.getClass())) {
+      log.debug("Pool type '" + type + "' is not supported by the component " + managedComponent.getManagedComponentId());
+      return;
+    }
+    ManagedComponent existing = components.putIfAbsent(managedComponent.getManagedComponentId().toString(), managedComponent);
+    if (existing != null) {
+      throw new IllegalArgumentException("Component '" + managedComponent.getManagedComponentId() + "' already exists in pool '" + name + "' !");
+    }
+  }
+
+  /** Remove named component from this pool. */
+  public boolean unregisterComponent(String componentId) {
+    return components.remove(name) != null;
+  }
+
+  /**
+   * Check whether a named component is registered in this pool.
+   * @param componentId component id
+   * @return true if the component with this name is registered, false otherwise.
+   */
+  public boolean isRegistered(String componentId) {
+    return components.containsKey(componentId);
+  }
+
+  /** Get components managed by this pool. */
+  public Map<String, T> getComponents() {
+    return Collections.unmodifiableMap(components);
+  }
+
+  public void addChangeListener(ChangeListener listener) {
+    if (!listeners.contains(listener)) {
+      listeners.add(listener);
+    }
+  }
+
+  public void removeChangeListener(ChangeListener listener) {
+    listeners.remove(listener);
+  }
+
+
+  /**
+   * Get the current monitored values from all resources. Result is a map with resource names as keys,
+   * and param/value maps as values.
+   */
+  public Map<String, Map<String, Object>> getCurrentValues() throws InterruptedException {
+    updateLock.lockInterruptibly();
 
 Review comment:
   The updateLock is acquired only in getCurrentValues() (called by doManage and ResourceManagerHandler.STATUS) and in manage() but manage() is called only by a scheduled executor which guarantees a happens-before relationship to previous executions. I think we can get rid of the lock entirely?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359740809
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
+
+  protected ResourceManagerPoolFactory resourceManagerPoolFactory;
+  protected SolrResourceLoader loader;
+
+
+  public DefaultResourceManager(SolrResourceLoader loader, TimeSource timeSource) {
+    this.loader = loader;
+    this.timeSource = timeSource;
+  }
+
+  protected void doInit() throws Exception {
+    scheduledThreadPoolExecutor = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(maxNumPools,
+        new DefaultSolrThreadFactory(getClass().getSimpleName()));
+    scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
+    scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+    // TODO: make configurable based on plugin info
+    resourceManagerPoolFactory = new DefaultResourceManagerPoolFactory(loader,
+        pluginInfo != null ?
+            (Map<String, Object>)pluginInfo.initArgs.toMap(new HashMap<>()).getOrDefault("plugins", Collections.emptyMap()) :
+            Collections.emptyMap());
+    log.info("Resource manager initialized.");
+  }
+
+  public void setMaxNumPools(Integer maxNumPools) {
+    if (maxNumPools != null) {
+      this.maxNumPools = maxNumPools;
+    } else {
+      this.maxNumPools = DEFAULT_MAX_POOLS;
+    }
+  }
+
+  @Override
+  public PluginInfo getPluginInfo() {
+    return pluginInfo;
+  }
+
+  @Override
+  public ResourceManagerPoolFactory getResourceManagerPoolFactory() {
+    return resourceManagerPoolFactory;
+  }
+
+  @Override
+  public ResourceManagerPool createPool(String name, String type, Map<String, Object> poolLimits, Map<String, Object> args) throws Exception {
+    ensureActive();
+    if (resourcePools.containsKey(name)) {
+      throw new IllegalArgumentException("Pool '" + name + "' already exists.");
+    }
+    if (resourcePools.size() >= maxNumPools) {
+      throw new IllegalArgumentException("Maximum number of pools (" + maxNumPools + ") reached.");
+    }
+    ResourceManagerPool newPool = resourceManagerPoolFactory.create(name, type, this, poolLimits, args);
+    newPool.scheduleDelaySeconds = Integer.parseInt(String.valueOf(args.getOrDefault(SCHEDULE_DELAY_SECONDS_PARAM, DEFAULT_SCHEDULE_DELAY_SECONDS)));
+    resourcePools.putIfAbsent(name, newPool);
 
 Review comment:
   Do a null check on the return value of putIfAbsent or we might leak the newPool object

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r368990483
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/ResourceManager.java
 ##########
 @@ -0,0 +1,305 @@
+/*
+ * 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.solr.managed;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.util.SolrPluginUtils;
+import org.apache.solr.util.plugin.PluginInfoInitialized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Base class for resource management. It uses a flat model where there are named
+ * resource pools of a given type, each pool with its own defined resource limits. Components can be added
+ * to a pool for the management of a specific aspect of that component.
+ */
+public abstract class ResourceManager implements PluginInfoInitialized, SolrMetricProducer {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String RESOURCE_MANAGER_PARAM = "resourceManager";
+  public static final String POOL_CONFIGS_PARAM = "poolConfigs";
+  public static final String POOL_LIMITS_PARAM = "poolLimits";
+  public static final String POOL_PARAMS_PARAM = "poolParams";
+
+  protected PluginInfo pluginInfo;
+  protected SolrMetricsContext metricsContext;
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
+  protected boolean initialized = false;
+
+  /**
+   * Create a resource manager and optionally create configured pools.
+   * @param loader SolrResourceLoader instance
+   * @param timeSource time source instance
+   * @param resourceManagerClass implementation class for the resource manager
+   * @param pluginInfo resource manager plugin info
+   * @param config resource manager and pool configurations
+   */
+  public static ResourceManager load(SolrResourceLoader loader, SolrMetricManager metricManager, TimeSource timeSource,
+                                     Class<? extends ResourceManager> resourceManagerClass, PluginInfo pluginInfo,
+                                     Map<String, Object> config) throws Exception {
+    Map<String, Object> managerOverrides = (Map<String, Object>)config.getOrDefault(RESOURCE_MANAGER_PARAM, Collections.emptyMap());
+    if (!managerOverrides.isEmpty()) {
+      Map<String, Object> pluginMap = new HashMap<>();
+      pluginInfo.toMap(pluginMap);
+      pluginMap.putAll(managerOverrides);
+      if (pluginMap.containsKey(FieldType.CLASS_NAME)) {
+        resourceManagerClass = loader.findClass((String)pluginMap.get(FieldType.CLASS_NAME), ResourceManager.class);
+      }
+      pluginInfo = new PluginInfo(pluginInfo.type, pluginMap);
+    }
+
+    ResourceManager resourceManager = loader.newInstance(
+        resourceManagerClass.getName(),
+        resourceManagerClass,
+        null,
+        new Class[]{SolrResourceLoader.class, SolrMetricManager.class, TimeSource.class},
+        new Object[]{loader, metricManager, timeSource});
+    SolrPluginUtils.invokeSetters(resourceManager, pluginInfo.initArgs);
+    resourceManager.init(pluginInfo);
+    resourceManager.initializeMetrics(new SolrMetricsContext(metricManager, "node", SolrMetricProducer.getUniqueMetricTag(loader, null)), null);
+    Map<String, Object> poolConfigs = (Map<String, Object>)config.get(POOL_CONFIGS_PARAM);
+    if (poolConfigs != null) {
+      for (String poolName : poolConfigs.keySet()) {
+        Map<String, Object> params = (Map<String, Object>)poolConfigs.get(poolName);
+        if (params == null || params.isEmpty()) {
+          throw new IllegalArgumentException("Pool '" + poolName + "' configuration missing: " + poolConfigs);
+        }
+        String type = (String)params.get(CommonParams.TYPE);
+        if (type == null || type.isBlank()) {
+          throw new IllegalArgumentException("Pool '" + poolName + "' type is missing: " + params);
+        }
+        Map<String, Object> poolLimits = (Map<String, Object>)params.getOrDefault(POOL_LIMITS_PARAM, Collections.emptyMap());
+        Map<String, Object> poolParams = (Map<String, Object>)params.getOrDefault(POOL_PARAMS_PARAM, Collections.emptyMap());
+        try {
+          resourceManager.createPool(poolName, type, poolLimits, poolParams);
+        } catch (Exception e) {
+          log.warn("Failed to create resource manager pool '" + poolName + "'", e);
+        }
+      }
+    }
+    return resourceManager;
+  }
+
+  @Override
+  public void init(PluginInfo info) {
+    if (info != null) {
+      this.pluginInfo = info.copy();
+    }
+    if (!enabled) {
+      log.debug("Resource manager " + getClass().getSimpleName() + " disabled.");
+      return;
+    }
+    try {
+      doInit();
+      initialized = true;
+    } catch (Exception e) {
+      log.warn("Exception initializing resource manager " + getClass().getSimpleName() + ", disabling!");
+      try {
+        close();
+      } catch (Exception e1) {
+        log.warn("Exception closing resource manager " + getClass().getSimpleName(), e1);
+      }
+    }
+  }
+
+  /**
+   * Enable resource management, defaults to true. {@link #init(PluginInfo)} checks
+   * this flag before calling {@link #doInit()}.
+   * @param enabled - whether or not resource management is to be enabled
+   */
+  public void setEnabled(Boolean enabled) {
+    if (enabled != null) {
+      this.enabled = enabled;
+    }
+  }
+
+  public PluginInfo getPluginInfo() {
+    return pluginInfo;
+  }
+
+  /**
+   * This method is called after plugin info and setters are invoked, but before metrics are initialized.
+   * @throws Exception on fatal errors during initialization
+   */
+  protected abstract void doInit() throws Exception;
+
+  @Override
+  public void initializeMetrics(SolrMetricsContext parentContext, String childScope) {
+    metricsContext = parentContext.getChildContext(this, "manager");
+  }
+
+  @Override
+  public SolrMetricsContext getSolrMetricsContext() {
+    return metricsContext;
+  }
+
+  /**
+   * Update pool limits for existing pools.
+   * @param newPoolLimits a map of pool names to a map of updated limits. Only existing pools will be affected.
+   */
+  public void updatePoolLimits(Map<String, Object> newPoolLimits) {
+    if (newPoolLimits == null || newPoolLimits.isEmpty()) {
+      return;
+    }
+
+    for (Map.Entry<String, Object> entry : newPoolLimits.entrySet()) {
+      String poolName = entry.getKey();
+      Map<String, Object> params = (Map<String, Object>)entry.getValue();
+      ResourceManagerPool pool = getPool(poolName);
+      if (pool == null) {
+        log.warn("Cannot update config - pool '" + poolName + "' not found.");
+        continue;
+      }
+      Map<String, Object> poolLimits = (Map<String, Object>)params.getOrDefault(POOL_LIMITS_PARAM, Collections.emptyMap());
+      pool.setPoolLimits(poolLimits);
+    }
+  }
+
+  /**
+   * Return a factory for creating specialized pools.
+   */
+  public abstract ResourceManagerPoolFactory getResourceManagerPoolFactory();
+
+  /**
+   * Create a named resource management pool.
+   * @param name pool name (must not be empty)
+   * @param type pool type (one of the supported {@link ResourceManagerPool} types)
+   * @param poolLimits pool limits (must not be null)
+   * @param poolParams other parameters (must not be null).
+   * @return newly created and scheduled resource pool
+   */
+  public abstract ResourceManagerPool createPool(String name, String type, Map<String, Object> poolLimits, Map<String, Object> poolParams) throws Exception;
+
+  /**
+   * List all currently existing pool names.
+   */
+  public abstract Collection<String> listPools();
+
+  /** Return a named pool or null if no such pool exists. */
+  public abstract ResourceManagerPool getPool(String name);
+
+  /** Returns true if a pool with this name exists, false otherwise. */
+  public boolean hasPool(String name) {
+    return getPool(name) != null;
+  }
+  /**
+   * Modify pool limits of an existing pool.
+   * @param name existing pool name
+   * @param poolLimits new pool limits. By convention only the values present in this map will be modified,
+   *                   all other limits will remain unchanged. In order to remove a limit use null value.
+   */
+  public abstract void setPoolLimits(String name, Map<String, Object> poolLimits) throws Exception;
+
+  /**
+   * Modify parameters of an existing pool.
+   * @param name existing pool name
+   * @param params new parameter values.
+   * @throws Exception when an invalid value or unsupported parameter is requested, or the parameter
+   * value cannot be changed after creation
+   */
+  public abstract void setPoolParams(String name, Map<String, Object> params) throws Exception;
+
+  /**
+   * Remove pool. This also stops the management of resources registered with that pool.
+   * @param name existing pool name
+   */
+  public abstract void removePool(String name) throws Exception;
+
+  /**
+   * Add managed components to a pool.
+   * @param pool existing pool name.
+   * @param managedComponents components to add
+   */
+  public void registerComponents(String pool, Collection<ManagedComponent> managedComponents) {
+    ensureActive();
+    for (ManagedComponent managedComponent : managedComponents) {
+      registerComponent(pool, managedComponent);
+    }
+  }
+
+  /**
+   * Add a managed component to a pool.
+   * @param pool existing pool name.
+   * @param managedComponent managed component. The component must support the management type
+   *                        used in the selected pool. The component must not be already managed by
+   *                        another pool of the same type.
+   */
+  public abstract void registerComponent(String pool, ManagedComponent managedComponent);
+
+  /**
+   * Remove a managed component from a pool.
+   * @param pool existing pool name.
+   * @param componentId component id to remove
+   * @return true if a component was actually registered and has been removed
+   */
+  public boolean unregisterComponent(String pool, ManagedComponentId componentId) {
+    return unregisterComponent(pool, componentId.toString());
+  }
+
+  /**
+   * Unregister component from all pools.
+   * @param componentId component id
+   * @return true if a component was actually registered and removed from at least one pool.
+   */
+  public boolean unregisterComponent(String componentId) {
+    boolean removed = false;
+    for (String pool : listPools()) {
+      removed = removed || unregisterComponent(pool, componentId);
+    }
+    return removed;
+  }
+
+  /**
+   * Remove a managed component from a pool.
+   * @param pool existing pool name.
+   * @param componentId component id to remove
+   * @return true if a component was actually registered and has been removed
+   */
+  public abstract boolean unregisterComponent(String pool, String componentId);
+
+  protected void ensureActive() {
+    if (isClosed) {
+      throw new IllegalStateException("Already closed.");
+    }
+    if (!initialized) {
+      throw new IllegalStateException("Not initialized.");
+    }
+  }
+
+  @Override
+  public void close() throws Exception {
+    synchronized (this) {
+      isClosed = true;
+      SolrMetricProducer.super.close();
+    }
 
 Review comment:
   The usage of isClosed and initialized in this class is not thread safe. Different threads read and set these values so the latest values are not guaranteed to be visible. Both can be made AtomicBoolean instances and then the synchronized(this) block here is not needed at all.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359744696
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
+
+  protected ResourceManagerPoolFactory resourceManagerPoolFactory;
+  protected SolrResourceLoader loader;
+
+
+  public DefaultResourceManager(SolrResourceLoader loader, TimeSource timeSource) {
+    this.loader = loader;
+    this.timeSource = timeSource;
+  }
+
+  protected void doInit() throws Exception {
+    scheduledThreadPoolExecutor = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(maxNumPools,
+        new DefaultSolrThreadFactory(getClass().getSimpleName()));
+    scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
+    scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+    // TODO: make configurable based on plugin info
+    resourceManagerPoolFactory = new DefaultResourceManagerPoolFactory(loader,
+        pluginInfo != null ?
+            (Map<String, Object>)pluginInfo.initArgs.toMap(new HashMap<>()).getOrDefault("plugins", Collections.emptyMap()) :
+            Collections.emptyMap());
+    log.info("Resource manager initialized.");
+  }
+
+  public void setMaxNumPools(Integer maxNumPools) {
+    if (maxNumPools != null) {
+      this.maxNumPools = maxNumPools;
+    } else {
+      this.maxNumPools = DEFAULT_MAX_POOLS;
+    }
+  }
+
+  @Override
+  public PluginInfo getPluginInfo() {
+    return pluginInfo;
+  }
+
+  @Override
+  public ResourceManagerPoolFactory getResourceManagerPoolFactory() {
+    return resourceManagerPoolFactory;
+  }
+
+  @Override
+  public ResourceManagerPool createPool(String name, String type, Map<String, Object> poolLimits, Map<String, Object> args) throws Exception {
+    ensureActive();
+    if (resourcePools.containsKey(name)) {
+      throw new IllegalArgumentException("Pool '" + name + "' already exists.");
+    }
+    if (resourcePools.size() >= maxNumPools) {
+      throw new IllegalArgumentException("Maximum number of pools (" + maxNumPools + ") reached.");
+    }
+    ResourceManagerPool newPool = resourceManagerPoolFactory.create(name, type, this, poolLimits, args);
+    newPool.scheduleDelaySeconds = Integer.parseInt(String.valueOf(args.getOrDefault(SCHEDULE_DELAY_SECONDS_PARAM, DEFAULT_SCHEDULE_DELAY_SECONDS)));
+    resourcePools.putIfAbsent(name, newPool);
+    if (timeSource != null) {
+      newPool.scheduledFuture = scheduledThreadPoolExecutor.scheduleWithFixedDelay(() -> {
+            log.info("- running pool " + newPool.getName() + " / " + newPool.getType());
+            newPool.manage();
+          }, 0,
+          timeSource.convertDelay(TimeUnit.SECONDS, newPool.scheduleDelaySeconds, TimeUnit.MILLISECONDS),
+          TimeUnit.MILLISECONDS);
+    }
+    log.info("- created pool " + newPool.getName() + " / " + newPool.getType());
+    return newPool;
+  }
+
+  @Override
+  public Collection<String> listPools() {
+    return Collections.unmodifiableSet(resourcePools.keySet());
+  }
+
+  @Override
+  public ResourceManagerPool getPool(String name) {
+    return resourcePools.get(name);
+  }
+
+  @Override
+  public void setPoolLimits(String name, Map<String, Object> poolLimits) throws Exception {
+    ensureActive();
+    ResourceManagerPool pool = resourcePools.get(name);
+    if (pool == null) {
+      throw new IllegalArgumentException("Pool '" + name + "' doesn't exist.");
+    }
+    pool.setPoolLimits(poolLimits);
+    log.info("- modified pool limits " + pool.getName() + " / " + pool.getType() + ": " + poolLimits);
+
+  }
+
+  @Override
+  public void removePool(String name) throws Exception {
+    ensureActive();
+    ResourceManagerPool pool = resourcePools.remove(name);
+    if (pool == null) {
+      throw new IllegalArgumentException("Pool '" + name + "' doesn't exist.");
+    }
+    IOUtils.closeQuietly(pool);
+    log.info("- removed pool " + pool.getName() + " / " + pool.getType());
+  }
+
+  @Override
+  public void registerComponent(String name, ManagedComponent managedComponent) {
+    ensureActive();
+    ResourceManagerPool pool = resourcePools.get(name);
+    if (pool == null) {
+      throw new IllegalArgumentException("Pool '" + name + "' doesn't exist.");
+    }
+    String poolType = pool.getType();
+    resourcePools.forEach((poolName, otherPool) -> {
+      if (otherPool == pool) {
+        return;
+      }
+      if (otherPool.isRegistered(managedComponent.getManagedComponentId().toString()) &&
+          otherPool.getType().equals(poolType)) {
+        throw new IllegalArgumentException("Resource " + managedComponent.getManagedComponentId() +
+            " is already managed in another pool (" +
+            otherPool.getName() + ") of the same type " + poolType);
+      }
+    });
+    pool.registerComponent(managedComponent);
+  }
+
+  @Override
+  public boolean unregisterComponent(String poolName, String resourceId) {
+    ensureActive();
+    ResourceManagerPool pool = resourcePools.get(poolName);
+    if (pool == null) {
+      throw new IllegalArgumentException("Pool '" + poolName + "' doesn't exist.");
+    }
+    return pool.unregisterComponent(resourceId);
+  }
+
+  @Override
+  public void close() throws IOException {
+    synchronized (this) {
+      isClosed = true;
+      log.debug("Closing all pools.");
+      for (ResourceManagerPool pool : resourcePools.values()) {
+        IOUtils.closeQuietly(pool);
+      }
+      resourcePools.clear();
 
 Review comment:
   This doesn't need to be inside the synchronized block I think.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r365806337
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
 
 Review comment:
   Fixed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r363873784
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/types/CacheManagerPool.java
 ##########
 @@ -0,0 +1,329 @@
+/*
+ * 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.solr.managed.types;
+
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+import org.apache.solr.managed.ChangeListener;
+import org.apache.solr.managed.ResourceManager;
+import org.apache.solr.managed.ResourceManagerPool;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.search.SolrCache;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * An implementation of {@link org.apache.solr.managed.ResourceManagerPool} specific to
+ * the management of {@link org.apache.solr.search.SolrCache} instances.
+ * <p>This plugin calculates the total size and maxRamMB of all registered cache instances
+ * and adjusts each cache's limits so that the aggregated values again fit within the pool limits.</p>
+ * <p>In order to avoid thrashing the plugin uses a dead band (by default {@link #DEFAULT_DEAD_BAND}),
+ * which can be adjusted using configuration parameter {@link #DEAD_BAND_PARAM}. If monitored values don't
+ * exceed the limits +/- the dead band then no forcible adjustment takes place.</p>
+ * <p>The management strategy consists of two distinct phases: soft optimization phase and then hard limit phase.</p>
+ * <p><b>Soft optimization</b> tries to adjust the resource consumption based on the cache hit ratio.
+ * This phase is executed only if there's no total limit exceeded. Also, hit ratio is considered a valid monitored
+ * variable only when at least N lookups occurred since the last adjustment (default value is {@link #DEFAULT_LOOKUP_DELTA}).
+ * If the hit ratio is higher than a threshold (default value is {@link #DEFAULT_TARGET_HITRATIO}) then the size
+ * of the cache can be reduced so that the resource consumption is minimized while still keeping acceptable hit
+ * ratio - and vice versa.</p>
+ * <p>This optimization phase can only adjust the limits within a {@link #DEFAULT_MAX_ADJUST_RATIO}, i.e. increased
+ * or decreased values may not be larger / smaller than this multiple / fraction of the initially configured limit.</p>
+ * <p><b>Hard limit</b> phase follows the soft optimization phase and it forcibly reduces resource consumption of all components
+ * if the total usage is still above the pool limit after the first phase has completed. Each component's limit is reduced
+ * by the same factor, regardless of the actual population or hit ratio.</p>
+ */
+public class CacheManagerPool extends ResourceManagerPool<SolrCache> {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static String TYPE = "cache";
+
+  /** Controller dead-band - changes smaller than this ratio will be ignored. */
+  public static final String DEAD_BAND_PARAM = "deadBand";
+  /** Target hit ratio - high enough to be useful, low enough to avoid excessive cache size. */
+  public static final String TARGET_HIT_RATIO_PARAM = "targetHitRatio";
+  /**
+   * Maximum allowed adjustment ratio from the initial configuration value. Adjusted value may not be
+   * higher than multiple of this factor, and not lower than divided by this factor.
+   */
+  public static final String MAX_ADJUST_RATIO_PARAM = "maxAdjustRatio";
+  /**
+   * Minimum number of lookups since last adjustment to consider the reported hitRatio
+   *  to be statistically valid.
+   */
+  public static final String MIN_LOOKUP_DELTA_PARAM = "minLookupDelta";
+  /** Default value of dead band (10%). */
+  public static final double DEFAULT_DEAD_BAND = 0.1;
+  /** Default target hit ratio - a compromise between usefulness and limited resource usage. */
+  public static final double DEFAULT_TARGET_HITRATIO = 0.8;
+  /**
+   * Default minimum number of lookups since the last adjustment. This can be treated as Bernoulli trials
+   * that give a 5% confidence about the statistical validity of hit ratio (<code>0.5 / sqrt(lookups)</code>).
+   */
+  public static final long DEFAULT_LOOKUP_DELTA = 100;
+  /**
+   * Default maximum adjustment ratio from the initially configured values.
+   */
+  public static final double DEFAULT_MAX_ADJUST_RATIO = 2.0;
+
+  protected static final Map<String, Function<Map<String, Object>, Double>> controlledToMonitored = new HashMap<>();
+
+  static {
+    controlledToMonitored.put(SolrCache.MAX_RAM_MB_PARAM, values -> {
+      Number ramBytes = (Number) values.get(SolrCache.RAM_BYTES_USED_PARAM);
+      return ramBytes != null ? ramBytes.doubleValue() / SolrCache.MB : 0.0;
+    });
+    controlledToMonitored.put(SolrCache.MAX_SIZE_PARAM, values ->
+        ((Number)values.getOrDefault(SolrCache.SIZE_PARAM, -1.0)).doubleValue());
+  }
+
+  protected double deadBand = DEFAULT_DEAD_BAND;
+  protected double targetHitRatio = DEFAULT_TARGET_HITRATIO;
+  protected long lookupDelta = DEFAULT_LOOKUP_DELTA;
+  protected double maxAdjustRatio = DEFAULT_MAX_ADJUST_RATIO;
+  protected Map<String, Long> lookups = new HashMap<>();
+  protected Map<String, Map<String, Object>> initialComponentLimits = new HashMap<>();
+
+  public CacheManagerPool(String name, String type, ResourceManager resourceManager, Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    super(name, type, resourceManager, poolLimits, poolParams);
+    String str = String.valueOf(poolParams.getOrDefault(DEAD_BAND_PARAM, DEFAULT_DEAD_BAND));
+    try {
+      deadBand = Double.parseDouble(str);
+    } catch (Exception e) {
+      log.warn("Invalid deadBand parameter value '" + str + "', using default " + DEFAULT_DEAD_BAND);
+    }
+  }
+
+  @Override
+  public void registerComponent(SolrCache component) {
+    super.registerComponent(component);
+    initialComponentLimits.put(component.getManagedComponentId().toString(), getResourceLimits(component));
+  }
+
+  @Override
+  public boolean unregisterComponent(String componentId) {
+    lookups.remove(componentId);
+    initialComponentLimits.remove(componentId);
+    return super.unregisterComponent(componentId);
+  }
+
+  @Override
+  public Object doSetResourceLimit(SolrCache component, String limitName, Object val) {
+    if (!(val instanceof Number)) {
+      try {
+        val = Long.parseLong(String.valueOf(val));
+      } catch (Exception e) {
+        throw new IllegalArgumentException("Unsupported value type (not a number) for limit '" + limitName + "': " + val + " (" + val.getClass().getName() + ")");
+      }
+    }
+    Number value = (Number)val;
+    if (value.longValue() > Integer.MAX_VALUE) {
+      throw new IllegalArgumentException("Invalid new value for limit '" + limitName +"': " + value);
+    }
+    switch (limitName) {
+      case SolrCache.MAX_SIZE_PARAM:
+        component.setMaxSize(value.intValue());
+        break;
+      case SolrCache.MAX_RAM_MB_PARAM:
+        component.setMaxRamMB(value.intValue());
+        break;
+      default:
+        throw new IllegalArgumentException("Unsupported limit name '" + limitName + "'");
+    }
+    return value.intValue();
+  }
+
+  @Override
+  public Map<String, Object> getResourceLimits(SolrCache component) {
+    Map<String, Object> limits = new HashMap<>();
+    limits.put(SolrCache.MAX_SIZE_PARAM, component.getMaxSize());
+    limits.put(SolrCache.MAX_RAM_MB_PARAM, component.getMaxRamMB());
+    return limits;
+  }
+
+  @Override
+  public Map<String, Object> getMonitoredValues(SolrCache component) throws Exception {
+    Map<String, Object> values = new HashMap<>();
+    values.put(SolrCache.SIZE_PARAM, component.size());
+    values.put(SolrCache.RAM_BYTES_USED_PARAM, component.ramBytesUsed());
+    SolrMetricsContext metricsContext = component.getSolrMetricsContext();
+    if (metricsContext != null) {
+      Map<String, Object> metrics = metricsContext.getMetricsSnapshot();
+      String key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.HIT_RATIO_PARAM;
+      values.put(SolrCache.HIT_RATIO_PARAM, metrics.get(key));
+      key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.LOOKUPS_PARAM;
+      values.put(SolrCache.LOOKUPS_PARAM, metrics.get(key));
+    }
+    return values;
+  }
+
+  @Override
+  protected void doManage() throws Exception {
+    Map<String, Map<String, Object>> currentValues = getCurrentValues();
+    Map<String, Object> totalValues = aggregateTotalValues(currentValues);
+    // pool limits are defined using controlled tags
+    poolLimits.forEach((poolLimitName, value) -> {
+      // only numeric limits are supported
+      if (value == null || !(value instanceof Number)) {
+        return;
+      }
+      double poolLimitValue = ((Number)value).doubleValue();
+      if (poolLimitValue <= 0) {
+        return;
+      }
+      Function<Map<String, Object>, Double> func = controlledToMonitored.get(poolLimitName);
+      if (func == null) {
+        return;
+      }
+      Double totalValue = func.apply(totalValues);
+      if (totalValue.doubleValue() <= 0.0) {
+        return;
+      }
+      double totalDelta = poolLimitValue - totalValue.doubleValue();
+
+      // dead band to avoid thrashing
+      if (Math.abs(totalDelta / poolLimitValue) < deadBand) {
+        return;
+      }
+
+      List<SolrCache> adjustableComponents = new ArrayList<>();
+      components.forEach((name, component) -> {
+        Map<String, Object> resourceLimits = getResourceLimits((SolrCache) component);
+        Object limit = resourceLimits.get(poolLimitName);
+        // XXX we could attempt here to control eg. ramBytesUsed by adjusting maxSize limit
+        // XXX and vice versa if the current limit is undefined or unsupported
+        if (limit == null || !(limit instanceof Number)) {
+          return;
+        }
+        double currentResourceLimit = ((Number)limit).doubleValue();
+        if (currentResourceLimit <= 0) { // undefined or unsupported
+          return;
+        }
+        adjustableComponents.add(component);
+      });
+      optimize(adjustableComponents, currentValues, poolLimitName, poolLimitValue, totalValue.doubleValue());
+    });
+  }
+
+  /**
+   * Manage all eligible components that support this pool limit.
+   */
+  private void optimize(List<SolrCache> components, Map<String, Map<String, Object>> currentValues, String limitName,
+                        double poolLimitValue, double totalValue) {
+    // changeRatio > 1.0 means there are available free resources
+    // changeRatio < 1.0 means there's shortage of resources
+    final AtomicReference<Double> changeRatio = new AtomicReference<>(poolLimitValue / totalValue);
+
+    // ========================== OPTIMIZATION ==============================
+    // if the situation is not critical (ie. total consumption is less than max)
+    // try to proactively optimize by reducing the size of caches with too high hitRatio
+    // (because a lower hit ratio is still acceptable if it means saving resources) and
+    // expand the size of caches with too low hitRatio
+    final AtomicReference<Double> newTotalValue = new AtomicReference<>(totalValue);
+    components.forEach(component -> {
+      long currentLookups = ((Number)currentValues.get(component.getManagedComponentId().toString()).get(SolrCache.LOOKUPS_PARAM)).longValue();
+      long lastLookups = lookups.computeIfAbsent(component.getManagedComponentId().toString(), k -> 0L);
+      if (currentLookups < lastLookups + lookupDelta) {
+        // too little data, skip the optimization
+        return;
+      }
 
 Review comment:
   If the lookups have been reset due to commit we should simply skip any optimization - because we really don't know how useful is the content of the cache at the moment.
   
   Using cumulative stats is a double-edged sword, specifically because they are never reset... Take for example cumulative hit ratio - it's calculated using cumulative_lookups and cumulative_hits. This means that short-term fluctuations are averaged, but it also means that short-term effects of any adjustments are also averaged (ie. they don't seem to affect the hit ratio in short term), which during thee next round of optimization will cause more and more adjustments, and the controller will eventually grossly overshoot its target. This issue becomes more and more serious as the absolute numbers increase because it's more and more difficult to sway this average.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r363811813
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/handler/admin/ResourceManagerHandler.java
 ##########
 @@ -0,0 +1,334 @@
+/*
+ * 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.solr.handler.admin;
+
+import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.RequestHandlerBase;
+import org.apache.solr.managed.ChangeListener;
+import org.apache.solr.managed.ManagedComponent;
+import org.apache.solr.managed.ResourceManager;
+import org.apache.solr.managed.ResourceManagerPool;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.security.AuthorizationContext;
+import org.apache.solr.security.PermissionNameProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Request handler to access and modify pools and their resource limits.
+ */
+public class ResourceManagerHandler extends RequestHandlerBase implements PermissionNameProvider {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String POOL_PARAM = "pool";
+  public static final String LIMIT_PREFIX_PARAM = "limit.";
+  public static final String ARG_PREFIX_PARAM = "arg.";
+  public static final String POOL_ACTION_PARAM = "poolAction";
+  public static final String RES_ACTION_PARAM = "resAction";
+
+  public enum PoolOp {
+    LIST,
+    STATUS,
+    CREATE,
+    DELETE,
+    SETLIMITS;
+
+    public static PoolOp get(String p) {
+      if (p != null) {
+        try {
+          return PoolOp.valueOf(p.toUpperCase(Locale.ROOT));
+        } catch (Exception e) {
+          return null;
+        }
+      }
+      return null;
+    }
+  }
+
+  public enum ResOp {
+    LIST,
+    STATUS,
+    DELETE,
+    GETLIMITS,
+    SETLIMITS;
+
+    public static ResOp get(String p) {
+      if (p != null) {
+        try {
+          return ResOp.valueOf(p.toUpperCase(Locale.ROOT));
+        } catch (Exception e) {
+          return null;
+        }
+      }
+      return null;
+    }
+  }
+
+  private final ResourceManager resourceManager;
+
+  public ResourceManagerHandler(ResourceManager resourceManager) {
+    this.resourceManager = resourceManager;
+  }
+
+  @Override
+  public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    if (resourceManager == null) {
+      throw new SolrException(SolrException.ErrorCode.INVALID_STATE, "ResourceManager instance not initialized.");
+    }
+    String poolAction = req.getParams().get(POOL_ACTION_PARAM);
+    String resAction = req.getParams().get(RES_ACTION_PARAM);
+    if (poolAction != null) {
+      handlePoolRequest(req, rsp);
+    } else if (resAction != null) {
+      handleResourceRequest(req, rsp);
+    } else {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Missing " + POOL_ACTION_PARAM + " and " + RES_ACTION_PARAM + ": " + req.getParams());
+    }
 
 Review comment:
   Good point, I think we should throw an exception.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r371203081
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/ResourceManager.java
 ##########
 @@ -0,0 +1,305 @@
+/*
+ * 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.solr.managed;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.metrics.SolrMetricManager;
+import org.apache.solr.metrics.SolrMetricProducer;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.schema.FieldType;
+import org.apache.solr.util.SolrPluginUtils;
+import org.apache.solr.util.plugin.PluginInfoInitialized;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Base class for resource management. It uses a flat model where there are named
+ * resource pools of a given type, each pool with its own defined resource limits. Components can be added
+ * to a pool for the management of a specific aspect of that component.
+ */
+public abstract class ResourceManager implements PluginInfoInitialized, SolrMetricProducer {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String RESOURCE_MANAGER_PARAM = "resourceManager";
+  public static final String POOL_CONFIGS_PARAM = "poolConfigs";
+  public static final String POOL_LIMITS_PARAM = "poolLimits";
+  public static final String POOL_PARAMS_PARAM = "poolParams";
+
+  protected PluginInfo pluginInfo;
+  protected SolrMetricsContext metricsContext;
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
+  protected boolean initialized = false;
+
+  /**
+   * Create a resource manager and optionally create configured pools.
+   * @param loader SolrResourceLoader instance
+   * @param timeSource time source instance
+   * @param resourceManagerClass implementation class for the resource manager
+   * @param pluginInfo resource manager plugin info
+   * @param config resource manager and pool configurations
+   */
+  public static ResourceManager load(SolrResourceLoader loader, SolrMetricManager metricManager, TimeSource timeSource,
+                                     Class<? extends ResourceManager> resourceManagerClass, PluginInfo pluginInfo,
+                                     Map<String, Object> config) throws Exception {
+    Map<String, Object> managerOverrides = (Map<String, Object>)config.getOrDefault(RESOURCE_MANAGER_PARAM, Collections.emptyMap());
+    if (!managerOverrides.isEmpty()) {
+      Map<String, Object> pluginMap = new HashMap<>();
+      pluginInfo.toMap(pluginMap);
+      pluginMap.putAll(managerOverrides);
+      if (pluginMap.containsKey(FieldType.CLASS_NAME)) {
+        resourceManagerClass = loader.findClass((String)pluginMap.get(FieldType.CLASS_NAME), ResourceManager.class);
+      }
+      pluginInfo = new PluginInfo(pluginInfo.type, pluginMap);
+    }
+
+    ResourceManager resourceManager = loader.newInstance(
+        resourceManagerClass.getName(),
+        resourceManagerClass,
+        null,
+        new Class[]{SolrResourceLoader.class, SolrMetricManager.class, TimeSource.class},
+        new Object[]{loader, metricManager, timeSource});
+    SolrPluginUtils.invokeSetters(resourceManager, pluginInfo.initArgs);
+    resourceManager.init(pluginInfo);
+    resourceManager.initializeMetrics(new SolrMetricsContext(metricManager, "node", SolrMetricProducer.getUniqueMetricTag(loader, null)), null);
+    Map<String, Object> poolConfigs = (Map<String, Object>)config.get(POOL_CONFIGS_PARAM);
+    if (poolConfigs != null) {
+      for (String poolName : poolConfigs.keySet()) {
+        Map<String, Object> params = (Map<String, Object>)poolConfigs.get(poolName);
+        if (params == null || params.isEmpty()) {
+          throw new IllegalArgumentException("Pool '" + poolName + "' configuration missing: " + poolConfigs);
+        }
+        String type = (String)params.get(CommonParams.TYPE);
+        if (type == null || type.isBlank()) {
+          throw new IllegalArgumentException("Pool '" + poolName + "' type is missing: " + params);
+        }
+        Map<String, Object> poolLimits = (Map<String, Object>)params.getOrDefault(POOL_LIMITS_PARAM, Collections.emptyMap());
+        Map<String, Object> poolParams = (Map<String, Object>)params.getOrDefault(POOL_PARAMS_PARAM, Collections.emptyMap());
+        try {
+          resourceManager.createPool(poolName, type, poolLimits, poolParams);
+        } catch (Exception e) {
+          log.warn("Failed to create resource manager pool '" + poolName + "'", e);
+        }
+      }
+    }
+    return resourceManager;
+  }
+
+  @Override
+  public void init(PluginInfo info) {
+    if (info != null) {
+      this.pluginInfo = info.copy();
+    }
+    if (!enabled) {
+      log.debug("Resource manager " + getClass().getSimpleName() + " disabled.");
+      return;
+    }
+    try {
+      doInit();
+      initialized = true;
+    } catch (Exception e) {
+      log.warn("Exception initializing resource manager " + getClass().getSimpleName() + ", disabling!");
+      try {
+        close();
+      } catch (Exception e1) {
+        log.warn("Exception closing resource manager " + getClass().getSimpleName(), e1);
+      }
+    }
+  }
+
+  /**
+   * Enable resource management, defaults to true. {@link #init(PluginInfo)} checks
+   * this flag before calling {@link #doInit()}.
+   * @param enabled - whether or not resource management is to be enabled
+   */
+  public void setEnabled(Boolean enabled) {
+    if (enabled != null) {
+      this.enabled = enabled;
+    }
+  }
+
+  public PluginInfo getPluginInfo() {
+    return pluginInfo;
+  }
+
+  /**
+   * This method is called after plugin info and setters are invoked, but before metrics are initialized.
+   * @throws Exception on fatal errors during initialization
+   */
+  protected abstract void doInit() throws Exception;
+
+  @Override
+  public void initializeMetrics(SolrMetricsContext parentContext, String childScope) {
+    metricsContext = parentContext.getChildContext(this, "manager");
+  }
+
+  @Override
+  public SolrMetricsContext getSolrMetricsContext() {
+    return metricsContext;
+  }
+
+  /**
+   * Update pool limits for existing pools.
+   * @param newPoolLimits a map of pool names to a map of updated limits. Only existing pools will be affected.
+   */
+  public void updatePoolLimits(Map<String, Object> newPoolLimits) {
+    if (newPoolLimits == null || newPoolLimits.isEmpty()) {
+      return;
+    }
+
+    for (Map.Entry<String, Object> entry : newPoolLimits.entrySet()) {
+      String poolName = entry.getKey();
+      Map<String, Object> params = (Map<String, Object>)entry.getValue();
+      ResourceManagerPool pool = getPool(poolName);
+      if (pool == null) {
+        log.warn("Cannot update config - pool '" + poolName + "' not found.");
+        continue;
+      }
+      Map<String, Object> poolLimits = (Map<String, Object>)params.getOrDefault(POOL_LIMITS_PARAM, Collections.emptyMap());
+      pool.setPoolLimits(poolLimits);
+    }
+  }
+
+  /**
+   * Return a factory for creating specialized pools.
+   */
+  public abstract ResourceManagerPoolFactory getResourceManagerPoolFactory();
+
+  /**
+   * Create a named resource management pool.
+   * @param name pool name (must not be empty)
+   * @param type pool type (one of the supported {@link ResourceManagerPool} types)
+   * @param poolLimits pool limits (must not be null)
+   * @param poolParams other parameters (must not be null).
+   * @return newly created and scheduled resource pool
+   */
+  public abstract ResourceManagerPool createPool(String name, String type, Map<String, Object> poolLimits, Map<String, Object> poolParams) throws Exception;
+
+  /**
+   * List all currently existing pool names.
+   */
+  public abstract Collection<String> listPools();
+
+  /** Return a named pool or null if no such pool exists. */
+  public abstract ResourceManagerPool getPool(String name);
+
+  /** Returns true if a pool with this name exists, false otherwise. */
+  public boolean hasPool(String name) {
+    return getPool(name) != null;
+  }
+  /**
+   * Modify pool limits of an existing pool.
+   * @param name existing pool name
+   * @param poolLimits new pool limits. By convention only the values present in this map will be modified,
+   *                   all other limits will remain unchanged. In order to remove a limit use null value.
+   */
+  public abstract void setPoolLimits(String name, Map<String, Object> poolLimits) throws Exception;
+
+  /**
+   * Modify parameters of an existing pool.
+   * @param name existing pool name
+   * @param params new parameter values.
+   * @throws Exception when an invalid value or unsupported parameter is requested, or the parameter
+   * value cannot be changed after creation
+   */
+  public abstract void setPoolParams(String name, Map<String, Object> params) throws Exception;
+
+  /**
+   * Remove pool. This also stops the management of resources registered with that pool.
+   * @param name existing pool name
+   */
+  public abstract void removePool(String name) throws Exception;
+
+  /**
+   * Add managed components to a pool.
+   * @param pool existing pool name.
+   * @param managedComponents components to add
+   */
+  public void registerComponents(String pool, Collection<ManagedComponent> managedComponents) {
+    ensureActive();
+    for (ManagedComponent managedComponent : managedComponents) {
+      registerComponent(pool, managedComponent);
+    }
+  }
+
+  /**
+   * Add a managed component to a pool.
+   * @param pool existing pool name.
+   * @param managedComponent managed component. The component must support the management type
+   *                        used in the selected pool. The component must not be already managed by
+   *                        another pool of the same type.
+   */
+  public abstract void registerComponent(String pool, ManagedComponent managedComponent);
+
+  /**
+   * Remove a managed component from a pool.
+   * @param pool existing pool name.
+   * @param componentId component id to remove
+   * @return true if a component was actually registered and has been removed
+   */
+  public boolean unregisterComponent(String pool, ManagedComponentId componentId) {
+    return unregisterComponent(pool, componentId.toString());
+  }
+
+  /**
+   * Unregister component from all pools.
+   * @param componentId component id
+   * @return true if a component was actually registered and removed from at least one pool.
+   */
+  public boolean unregisterComponent(String componentId) {
+    boolean removed = false;
+    for (String pool : listPools()) {
+      removed = removed || unregisterComponent(pool, componentId);
+    }
+    return removed;
+  }
+
+  /**
+   * Remove a managed component from a pool.
+   * @param pool existing pool name.
+   * @param componentId component id to remove
+   * @return true if a component was actually registered and has been removed
+   */
+  public abstract boolean unregisterComponent(String pool, String componentId);
+
+  protected void ensureActive() {
+    if (isClosed) {
+      throw new IllegalStateException("Already closed.");
+    }
+    if (!initialized) {
+      throw new IllegalStateException("Not initialized.");
+    }
+  }
+
+  @Override
+  public void close() throws Exception {
+    synchronized (this) {
+      isClosed = true;
+      SolrMetricProducer.super.close();
+    }
 
 Review comment:
   Fixed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359744215
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
 
 Review comment:
   The `isClosed` is also defined in the parent class and it appears to be unused except in the close method. Can we get rid of it?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r368989051
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
+
+  protected ResourceManagerPoolFactory resourceManagerPoolFactory;
+  protected SolrResourceLoader loader;
+
+
+  public DefaultResourceManager(SolrResourceLoader loader, TimeSource timeSource) {
+    this.loader = loader;
+    this.timeSource = timeSource;
+  }
+
+  protected void doInit() throws Exception {
+    scheduledThreadPoolExecutor = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(maxNumPools,
+        new DefaultSolrThreadFactory(getClass().getSimpleName()));
+    scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
+    scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+    // TODO: make configurable based on plugin info
+    resourceManagerPoolFactory = new DefaultResourceManagerPoolFactory(loader,
+        pluginInfo != null ?
+            (Map<String, Object>)pluginInfo.initArgs.toMap(new HashMap<>()).getOrDefault("plugins", Collections.emptyMap()) :
+            Collections.emptyMap());
+    log.info("Resource manager initialized.");
+  }
+
+  public void setMaxNumPools(Integer maxNumPools) {
+    if (maxNumPools != null) {
+      this.maxNumPools = maxNumPools;
+    } else {
+      this.maxNumPools = DEFAULT_MAX_POOLS;
+    }
+  }
+
+  @Override
+  public PluginInfo getPluginInfo() {
+    return pluginInfo;
+  }
+
+  @Override
+  public ResourceManagerPoolFactory getResourceManagerPoolFactory() {
+    return resourceManagerPoolFactory;
+  }
+
+  @Override
+  public ResourceManagerPool createPool(String name, String type, Map<String, Object> poolLimits, Map<String, Object> args) throws Exception {
+    ensureActive();
+    if (resourcePools.containsKey(name)) {
+      throw new IllegalArgumentException("Pool '" + name + "' already exists.");
+    }
+    if (resourcePools.size() >= maxNumPools) {
+      throw new IllegalArgumentException("Maximum number of pools (" + maxNumPools + ") reached.");
+    }
+    ResourceManagerPool newPool = resourceManagerPoolFactory.create(name, type, this, poolLimits, args);
+    newPool.scheduleDelaySeconds = Integer.parseInt(String.valueOf(args.getOrDefault(SCHEDULE_DELAY_SECONDS_PARAM, DEFAULT_SCHEDULE_DELAY_SECONDS)));
+    resourcePools.putIfAbsent(name, newPool);
+    if (timeSource != null) {
+      newPool.scheduledFuture = scheduledThreadPoolExecutor.scheduleWithFixedDelay(() -> {
+            log.info("- running pool " + newPool.getName() + " / " + newPool.getType());
+            newPool.manage();
+          }, 0,
+          timeSource.convertDelay(TimeUnit.SECONDS, newPool.scheduleDelaySeconds, TimeUnit.MILLISECONDS),
+          TimeUnit.MILLISECONDS);
+    }
+    log.info("- created pool " + newPool.getName() + " / " + newPool.getType());
+    return newPool;
+  }
+
+  @Override
+  public Collection<String> listPools() {
+    return Collections.unmodifiableSet(resourcePools.keySet());
+  }
+
+  @Override
+  public ResourceManagerPool getPool(String name) {
+    return resourcePools.get(name);
+  }
+
+  @Override
+  public void setPoolLimits(String name, Map<String, Object> poolLimits) throws Exception {
+    ensureActive();
+    ResourceManagerPool pool = resourcePools.get(name);
+    if (pool == null) {
+      throw new IllegalArgumentException("Pool '" + name + "' doesn't exist.");
+    }
+    pool.setPoolLimits(poolLimits);
+    log.info("- modified pool limits " + pool.getName() + " / " + pool.getType() + ": " + poolLimits);
+
+  }
+
+  @Override
+  public void removePool(String name) throws Exception {
+    ensureActive();
+    ResourceManagerPool pool = resourcePools.remove(name);
+    if (pool == null) {
+      throw new IllegalArgumentException("Pool '" + name + "' doesn't exist.");
+    }
+    IOUtils.closeQuietly(pool);
+    log.info("- removed pool " + pool.getName() + " / " + pool.getType());
+  }
+
+  @Override
+  public void registerComponent(String name, ManagedComponent managedComponent) {
+    ensureActive();
+    ResourceManagerPool pool = resourcePools.get(name);
+    if (pool == null) {
+      throw new IllegalArgumentException("Pool '" + name + "' doesn't exist.");
+    }
+    String poolType = pool.getType();
+    resourcePools.forEach((poolName, otherPool) -> {
+      if (otherPool == pool) {
+        return;
+      }
+      if (otherPool.isRegistered(managedComponent.getManagedComponentId().toString()) &&
+          otherPool.getType().equals(poolType)) {
+        throw new IllegalArgumentException("Resource " + managedComponent.getManagedComponentId() +
+            " is already managed in another pool (" +
+            otherPool.getName() + ") of the same type " + poolType);
+      }
+    });
+    pool.registerComponent(managedComponent);
+  }
+
+  @Override
+  public boolean unregisterComponent(String poolName, String resourceId) {
+    ensureActive();
+    ResourceManagerPool pool = resourcePools.get(poolName);
+    if (pool == null) {
+      throw new IllegalArgumentException("Pool '" + poolName + "' doesn't exist.");
+    }
+    return pool.unregisterComponent(resourceId);
+  }
+
+  @Override
+  public void close() throws IOException {
+    synchronized (this) {
+      isClosed = true;
+      log.debug("Closing all pools.");
+      for (ResourceManagerPool pool : resourcePools.values()) {
+        IOUtils.closeQuietly(pool);
+      }
+      resourcePools.clear();
 
 Review comment:
   This is still not addressed. Why is the synchronized block required here?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359713817
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/handler/admin/ResourceManagerHandler.java
 ##########
 @@ -0,0 +1,334 @@
+/*
+ * 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.solr.handler.admin;
+
+import java.lang.invoke.MethodHandles;
+import java.util.HashMap;
+import java.util.Locale;
+import java.util.Map;
+
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.params.SolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.RequestHandlerBase;
+import org.apache.solr.managed.ChangeListener;
+import org.apache.solr.managed.ManagedComponent;
+import org.apache.solr.managed.ResourceManager;
+import org.apache.solr.managed.ResourceManagerPool;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+import org.apache.solr.security.AuthorizationContext;
+import org.apache.solr.security.PermissionNameProvider;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Request handler to access and modify pools and their resource limits.
+ */
+public class ResourceManagerHandler extends RequestHandlerBase implements PermissionNameProvider {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String POOL_PARAM = "pool";
+  public static final String LIMIT_PREFIX_PARAM = "limit.";
+  public static final String ARG_PREFIX_PARAM = "arg.";
+  public static final String POOL_ACTION_PARAM = "poolAction";
+  public static final String RES_ACTION_PARAM = "resAction";
+
+  public enum PoolOp {
+    LIST,
+    STATUS,
+    CREATE,
+    DELETE,
+    SETLIMITS;
+
+    public static PoolOp get(String p) {
+      if (p != null) {
+        try {
+          return PoolOp.valueOf(p.toUpperCase(Locale.ROOT));
+        } catch (Exception e) {
+          return null;
+        }
+      }
+      return null;
+    }
+  }
+
+  public enum ResOp {
+    LIST,
+    STATUS,
+    DELETE,
+    GETLIMITS,
+    SETLIMITS;
+
+    public static ResOp get(String p) {
+      if (p != null) {
+        try {
+          return ResOp.valueOf(p.toUpperCase(Locale.ROOT));
+        } catch (Exception e) {
+          return null;
+        }
+      }
+      return null;
+    }
+  }
+
+  private final ResourceManager resourceManager;
+
+  public ResourceManagerHandler(ResourceManager resourceManager) {
+    this.resourceManager = resourceManager;
+  }
+
+  @Override
+  public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    if (resourceManager == null) {
+      throw new SolrException(SolrException.ErrorCode.INVALID_STATE, "ResourceManager instance not initialized.");
+    }
+    String poolAction = req.getParams().get(POOL_ACTION_PARAM);
+    String resAction = req.getParams().get(RES_ACTION_PARAM);
+    if (poolAction != null) {
+      handlePoolRequest(req, rsp);
+    } else if (resAction != null) {
+      handleResourceRequest(req, rsp);
+    } else {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Missing " + POOL_ACTION_PARAM + " and " + RES_ACTION_PARAM + ": " + req.getParams());
+    }
 
 Review comment:
   What if both `poolAction` and `resAction` are specified together in one request? We should either throw an exception or handle both. Current impl will execute only `poolAction`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359741713
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
+
+  protected ResourceManagerPoolFactory resourceManagerPoolFactory;
+  protected SolrResourceLoader loader;
+
+
+  public DefaultResourceManager(SolrResourceLoader loader, TimeSource timeSource) {
+    this.loader = loader;
+    this.timeSource = timeSource;
+  }
+
+  protected void doInit() throws Exception {
+    scheduledThreadPoolExecutor = (ScheduledThreadPoolExecutor) Executors.newScheduledThreadPool(maxNumPools,
+        new DefaultSolrThreadFactory(getClass().getSimpleName()));
+    scheduledThreadPoolExecutor.setRemoveOnCancelPolicy(true);
+    scheduledThreadPoolExecutor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false);
+    // TODO: make configurable based on plugin info
+    resourceManagerPoolFactory = new DefaultResourceManagerPoolFactory(loader,
+        pluginInfo != null ?
+            (Map<String, Object>)pluginInfo.initArgs.toMap(new HashMap<>()).getOrDefault("plugins", Collections.emptyMap()) :
+            Collections.emptyMap());
+    log.info("Resource manager initialized.");
+  }
+
+  public void setMaxNumPools(Integer maxNumPools) {
+    if (maxNumPools != null) {
+      this.maxNumPools = maxNumPools;
+    } else {
+      this.maxNumPools = DEFAULT_MAX_POOLS;
+    }
+  }
+
+  @Override
+  public PluginInfo getPluginInfo() {
+    return pluginInfo;
+  }
+
+  @Override
+  public ResourceManagerPoolFactory getResourceManagerPoolFactory() {
+    return resourceManagerPoolFactory;
+  }
+
+  @Override
+  public ResourceManagerPool createPool(String name, String type, Map<String, Object> poolLimits, Map<String, Object> args) throws Exception {
+    ensureActive();
+    if (resourcePools.containsKey(name)) {
+      throw new IllegalArgumentException("Pool '" + name + "' already exists.");
+    }
+    if (resourcePools.size() >= maxNumPools) {
+      throw new IllegalArgumentException("Maximum number of pools (" + maxNumPools + ") reached.");
+    }
+    ResourceManagerPool newPool = resourceManagerPoolFactory.create(name, type, this, poolLimits, args);
+    newPool.scheduleDelaySeconds = Integer.parseInt(String.valueOf(args.getOrDefault(SCHEDULE_DELAY_SECONDS_PARAM, DEFAULT_SCHEDULE_DELAY_SECONDS)));
+    resourcePools.putIfAbsent(name, newPool);
+    if (timeSource != null) {
+      newPool.scheduledFuture = scheduledThreadPoolExecutor.scheduleWithFixedDelay(() -> {
+            log.info("- running pool " + newPool.getName() + " / " + newPool.getType());
+            newPool.manage();
 
 Review comment:
   Perhaps expose metrics for how long it took to the manage() call? Since this is scheduled at fixed delay and not fixed rate, we should track how long it took and log a warning if it is longer than the `scheduleDelaySeconds`

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359773422
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
 
 Review comment:
   In fact, this is quite trappy. The close() method sets this `isClosed=true` but the `ensureActive()` method checks the parent's isClosed which is always false because the close() method does not set super.isClosed=true and there is no super.close() method too

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r360290771
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/types/CacheManagerPool.java
 ##########
 @@ -0,0 +1,329 @@
+/*
+ * 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.solr.managed.types;
+
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+import org.apache.solr.managed.ChangeListener;
+import org.apache.solr.managed.ResourceManager;
+import org.apache.solr.managed.ResourceManagerPool;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.search.SolrCache;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * An implementation of {@link org.apache.solr.managed.ResourceManagerPool} specific to
+ * the management of {@link org.apache.solr.search.SolrCache} instances.
+ * <p>This plugin calculates the total size and maxRamMB of all registered cache instances
+ * and adjusts each cache's limits so that the aggregated values again fit within the pool limits.</p>
+ * <p>In order to avoid thrashing the plugin uses a dead band (by default {@link #DEFAULT_DEAD_BAND}),
+ * which can be adjusted using configuration parameter {@link #DEAD_BAND_PARAM}. If monitored values don't
+ * exceed the limits +/- the dead band then no forcible adjustment takes place.</p>
+ * <p>The management strategy consists of two distinct phases: soft optimization phase and then hard limit phase.</p>
+ * <p><b>Soft optimization</b> tries to adjust the resource consumption based on the cache hit ratio.
+ * This phase is executed only if there's no total limit exceeded. Also, hit ratio is considered a valid monitored
+ * variable only when at least N lookups occurred since the last adjustment (default value is {@link #DEFAULT_LOOKUP_DELTA}).
+ * If the hit ratio is higher than a threshold (default value is {@link #DEFAULT_TARGET_HITRATIO}) then the size
+ * of the cache can be reduced so that the resource consumption is minimized while still keeping acceptable hit
+ * ratio - and vice versa.</p>
+ * <p>This optimization phase can only adjust the limits within a {@link #DEFAULT_MAX_ADJUST_RATIO}, i.e. increased
+ * or decreased values may not be larger / smaller than this multiple / fraction of the initially configured limit.</p>
+ * <p><b>Hard limit</b> phase follows the soft optimization phase and it forcibly reduces resource consumption of all components
+ * if the total usage is still above the pool limit after the first phase has completed. Each component's limit is reduced
+ * by the same factor, regardless of the actual population or hit ratio.</p>
+ */
+public class CacheManagerPool extends ResourceManagerPool<SolrCache> {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static String TYPE = "cache";
+
+  /** Controller dead-band - changes smaller than this ratio will be ignored. */
+  public static final String DEAD_BAND_PARAM = "deadBand";
+  /** Target hit ratio - high enough to be useful, low enough to avoid excessive cache size. */
+  public static final String TARGET_HIT_RATIO_PARAM = "targetHitRatio";
+  /**
+   * Maximum allowed adjustment ratio from the initial configuration value. Adjusted value may not be
+   * higher than multiple of this factor, and not lower than divided by this factor.
+   */
+  public static final String MAX_ADJUST_RATIO_PARAM = "maxAdjustRatio";
+  /**
+   * Minimum number of lookups since last adjustment to consider the reported hitRatio
+   *  to be statistically valid.
+   */
+  public static final String MIN_LOOKUP_DELTA_PARAM = "minLookupDelta";
+  /** Default value of dead band (10%). */
+  public static final double DEFAULT_DEAD_BAND = 0.1;
+  /** Default target hit ratio - a compromise between usefulness and limited resource usage. */
+  public static final double DEFAULT_TARGET_HITRATIO = 0.8;
+  /**
+   * Default minimum number of lookups since the last adjustment. This can be treated as Bernoulli trials
+   * that give a 5% confidence about the statistical validity of hit ratio (<code>0.5 / sqrt(lookups)</code>).
+   */
+  public static final long DEFAULT_LOOKUP_DELTA = 100;
+  /**
+   * Default maximum adjustment ratio from the initially configured values.
+   */
+  public static final double DEFAULT_MAX_ADJUST_RATIO = 2.0;
+
+  protected static final Map<String, Function<Map<String, Object>, Double>> controlledToMonitored = new HashMap<>();
+
+  static {
+    controlledToMonitored.put(SolrCache.MAX_RAM_MB_PARAM, values -> {
+      Number ramBytes = (Number) values.get(SolrCache.RAM_BYTES_USED_PARAM);
+      return ramBytes != null ? ramBytes.doubleValue() / SolrCache.MB : 0.0;
+    });
+    controlledToMonitored.put(SolrCache.MAX_SIZE_PARAM, values ->
+        ((Number)values.getOrDefault(SolrCache.SIZE_PARAM, -1.0)).doubleValue());
+  }
+
+  protected double deadBand = DEFAULT_DEAD_BAND;
+  protected double targetHitRatio = DEFAULT_TARGET_HITRATIO;
+  protected long lookupDelta = DEFAULT_LOOKUP_DELTA;
+  protected double maxAdjustRatio = DEFAULT_MAX_ADJUST_RATIO;
+  protected Map<String, Long> lookups = new HashMap<>();
+  protected Map<String, Map<String, Object>> initialComponentLimits = new HashMap<>();
+
+  public CacheManagerPool(String name, String type, ResourceManager resourceManager, Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    super(name, type, resourceManager, poolLimits, poolParams);
+    String str = String.valueOf(poolParams.getOrDefault(DEAD_BAND_PARAM, DEFAULT_DEAD_BAND));
+    try {
+      deadBand = Double.parseDouble(str);
+    } catch (Exception e) {
+      log.warn("Invalid deadBand parameter value '" + str + "', using default " + DEFAULT_DEAD_BAND);
+    }
+  }
+
+  @Override
+  public void registerComponent(SolrCache component) {
+    super.registerComponent(component);
+    initialComponentLimits.put(component.getManagedComponentId().toString(), getResourceLimits(component));
+  }
+
+  @Override
+  public boolean unregisterComponent(String componentId) {
+    lookups.remove(componentId);
+    initialComponentLimits.remove(componentId);
+    return super.unregisterComponent(componentId);
+  }
+
+  @Override
+  public Object doSetResourceLimit(SolrCache component, String limitName, Object val) {
+    if (!(val instanceof Number)) {
+      try {
+        val = Long.parseLong(String.valueOf(val));
+      } catch (Exception e) {
+        throw new IllegalArgumentException("Unsupported value type (not a number) for limit '" + limitName + "': " + val + " (" + val.getClass().getName() + ")");
+      }
+    }
+    Number value = (Number)val;
+    if (value.longValue() > Integer.MAX_VALUE) {
+      throw new IllegalArgumentException("Invalid new value for limit '" + limitName +"': " + value);
+    }
+    switch (limitName) {
+      case SolrCache.MAX_SIZE_PARAM:
+        component.setMaxSize(value.intValue());
+        break;
+      case SolrCache.MAX_RAM_MB_PARAM:
+        component.setMaxRamMB(value.intValue());
+        break;
+      default:
+        throw new IllegalArgumentException("Unsupported limit name '" + limitName + "'");
+    }
+    return value.intValue();
+  }
+
+  @Override
+  public Map<String, Object> getResourceLimits(SolrCache component) {
+    Map<String, Object> limits = new HashMap<>();
+    limits.put(SolrCache.MAX_SIZE_PARAM, component.getMaxSize());
+    limits.put(SolrCache.MAX_RAM_MB_PARAM, component.getMaxRamMB());
+    return limits;
+  }
+
+  @Override
+  public Map<String, Object> getMonitoredValues(SolrCache component) throws Exception {
+    Map<String, Object> values = new HashMap<>();
+    values.put(SolrCache.SIZE_PARAM, component.size());
+    values.put(SolrCache.RAM_BYTES_USED_PARAM, component.ramBytesUsed());
+    SolrMetricsContext metricsContext = component.getSolrMetricsContext();
+    if (metricsContext != null) {
+      Map<String, Object> metrics = metricsContext.getMetricsSnapshot();
+      String key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.HIT_RATIO_PARAM;
+      values.put(SolrCache.HIT_RATIO_PARAM, metrics.get(key));
+      key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.LOOKUPS_PARAM;
+      values.put(SolrCache.LOOKUPS_PARAM, metrics.get(key));
+    }
+    return values;
+  }
+
+  @Override
+  protected void doManage() throws Exception {
+    Map<String, Map<String, Object>> currentValues = getCurrentValues();
+    Map<String, Object> totalValues = aggregateTotalValues(currentValues);
+    // pool limits are defined using controlled tags
+    poolLimits.forEach((poolLimitName, value) -> {
+      // only numeric limits are supported
+      if (value == null || !(value instanceof Number)) {
+        return;
+      }
+      double poolLimitValue = ((Number)value).doubleValue();
+      if (poolLimitValue <= 0) {
+        return;
+      }
+      Function<Map<String, Object>, Double> func = controlledToMonitored.get(poolLimitName);
+      if (func == null) {
+        return;
+      }
+      Double totalValue = func.apply(totalValues);
+      if (totalValue.doubleValue() <= 0.0) {
+        return;
+      }
+      double totalDelta = poolLimitValue - totalValue.doubleValue();
+
+      // dead band to avoid thrashing
+      if (Math.abs(totalDelta / poolLimitValue) < deadBand) {
+        return;
+      }
+
+      List<SolrCache> adjustableComponents = new ArrayList<>();
+      components.forEach((name, component) -> {
+        Map<String, Object> resourceLimits = getResourceLimits((SolrCache) component);
+        Object limit = resourceLimits.get(poolLimitName);
+        // XXX we could attempt here to control eg. ramBytesUsed by adjusting maxSize limit
+        // XXX and vice versa if the current limit is undefined or unsupported
+        if (limit == null || !(limit instanceof Number)) {
+          return;
+        }
+        double currentResourceLimit = ((Number)limit).doubleValue();
+        if (currentResourceLimit <= 0) { // undefined or unsupported
+          return;
+        }
+        adjustableComponents.add(component);
+      });
+      optimize(adjustableComponents, currentValues, poolLimitName, poolLimitValue, totalValue.doubleValue());
+    });
+  }
+
+  /**
+   * Manage all eligible components that support this pool limit.
+   */
+  private void optimize(List<SolrCache> components, Map<String, Map<String, Object>> currentValues, String limitName,
+                        double poolLimitValue, double totalValue) {
+    // changeRatio > 1.0 means there are available free resources
+    // changeRatio < 1.0 means there's shortage of resources
+    final AtomicReference<Double> changeRatio = new AtomicReference<>(poolLimitValue / totalValue);
+
+    // ========================== OPTIMIZATION ==============================
+    // if the situation is not critical (ie. total consumption is less than max)
+    // try to proactively optimize by reducing the size of caches with too high hitRatio
+    // (because a lower hit ratio is still acceptable if it means saving resources) and
+    // expand the size of caches with too low hitRatio
+    final AtomicReference<Double> newTotalValue = new AtomicReference<>(totalValue);
+    components.forEach(component -> {
+      long currentLookups = ((Number)currentValues.get(component.getManagedComponentId().toString()).get(SolrCache.LOOKUPS_PARAM)).longValue();
+      long lastLookups = lookups.computeIfAbsent(component.getManagedComponentId().toString(), k -> 0L);
+      if (currentLookups < lastLookups + lookupDelta) {
+        // too little data, skip the optimization
+        return;
+      }
+      Map<String, Object> resourceLimits = getResourceLimits(component);
+      double currentLimit = ((Number)resourceLimits.get(limitName)).doubleValue();
+      double currentHitRatio = ((Number)currentValues.get(component.getManagedComponentId().toString()).get(SolrCache.HIT_RATIO_PARAM)).doubleValue();
+      Number initialLimit = (Number)initialComponentLimits.get(component.getManagedComponentId().toString()).get(limitName);
+      if (initialLimit == null) {
+        // can't optimize because we don't know how far off we are from the initial setting
+        return;
+      }
+      if (currentHitRatio < targetHitRatio) {
+        if (changeRatio.get() < 1.0) {
+          // don't expand if we're already short on resources
+          return;
+        }
+        // expand to increase the hitRatio, but not more than maxAdjustRatio from the initialLimit
+        double newLimit = currentLimit * changeRatio.get();
 
 Review comment:
   Shouldn't the limit be adjusted only if there are evictions because of the old limit? Otherwise we are increasing the limits for no benefit

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r363873784
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/types/CacheManagerPool.java
 ##########
 @@ -0,0 +1,329 @@
+/*
+ * 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.solr.managed.types;
+
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+import org.apache.solr.managed.ChangeListener;
+import org.apache.solr.managed.ResourceManager;
+import org.apache.solr.managed.ResourceManagerPool;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.search.SolrCache;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * An implementation of {@link org.apache.solr.managed.ResourceManagerPool} specific to
+ * the management of {@link org.apache.solr.search.SolrCache} instances.
+ * <p>This plugin calculates the total size and maxRamMB of all registered cache instances
+ * and adjusts each cache's limits so that the aggregated values again fit within the pool limits.</p>
+ * <p>In order to avoid thrashing the plugin uses a dead band (by default {@link #DEFAULT_DEAD_BAND}),
+ * which can be adjusted using configuration parameter {@link #DEAD_BAND_PARAM}. If monitored values don't
+ * exceed the limits +/- the dead band then no forcible adjustment takes place.</p>
+ * <p>The management strategy consists of two distinct phases: soft optimization phase and then hard limit phase.</p>
+ * <p><b>Soft optimization</b> tries to adjust the resource consumption based on the cache hit ratio.
+ * This phase is executed only if there's no total limit exceeded. Also, hit ratio is considered a valid monitored
+ * variable only when at least N lookups occurred since the last adjustment (default value is {@link #DEFAULT_LOOKUP_DELTA}).
+ * If the hit ratio is higher than a threshold (default value is {@link #DEFAULT_TARGET_HITRATIO}) then the size
+ * of the cache can be reduced so that the resource consumption is minimized while still keeping acceptable hit
+ * ratio - and vice versa.</p>
+ * <p>This optimization phase can only adjust the limits within a {@link #DEFAULT_MAX_ADJUST_RATIO}, i.e. increased
+ * or decreased values may not be larger / smaller than this multiple / fraction of the initially configured limit.</p>
+ * <p><b>Hard limit</b> phase follows the soft optimization phase and it forcibly reduces resource consumption of all components
+ * if the total usage is still above the pool limit after the first phase has completed. Each component's limit is reduced
+ * by the same factor, regardless of the actual population or hit ratio.</p>
+ */
+public class CacheManagerPool extends ResourceManagerPool<SolrCache> {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static String TYPE = "cache";
+
+  /** Controller dead-band - changes smaller than this ratio will be ignored. */
+  public static final String DEAD_BAND_PARAM = "deadBand";
+  /** Target hit ratio - high enough to be useful, low enough to avoid excessive cache size. */
+  public static final String TARGET_HIT_RATIO_PARAM = "targetHitRatio";
+  /**
+   * Maximum allowed adjustment ratio from the initial configuration value. Adjusted value may not be
+   * higher than multiple of this factor, and not lower than divided by this factor.
+   */
+  public static final String MAX_ADJUST_RATIO_PARAM = "maxAdjustRatio";
+  /**
+   * Minimum number of lookups since last adjustment to consider the reported hitRatio
+   *  to be statistically valid.
+   */
+  public static final String MIN_LOOKUP_DELTA_PARAM = "minLookupDelta";
+  /** Default value of dead band (10%). */
+  public static final double DEFAULT_DEAD_BAND = 0.1;
+  /** Default target hit ratio - a compromise between usefulness and limited resource usage. */
+  public static final double DEFAULT_TARGET_HITRATIO = 0.8;
+  /**
+   * Default minimum number of lookups since the last adjustment. This can be treated as Bernoulli trials
+   * that give a 5% confidence about the statistical validity of hit ratio (<code>0.5 / sqrt(lookups)</code>).
+   */
+  public static final long DEFAULT_LOOKUP_DELTA = 100;
+  /**
+   * Default maximum adjustment ratio from the initially configured values.
+   */
+  public static final double DEFAULT_MAX_ADJUST_RATIO = 2.0;
+
+  protected static final Map<String, Function<Map<String, Object>, Double>> controlledToMonitored = new HashMap<>();
+
+  static {
+    controlledToMonitored.put(SolrCache.MAX_RAM_MB_PARAM, values -> {
+      Number ramBytes = (Number) values.get(SolrCache.RAM_BYTES_USED_PARAM);
+      return ramBytes != null ? ramBytes.doubleValue() / SolrCache.MB : 0.0;
+    });
+    controlledToMonitored.put(SolrCache.MAX_SIZE_PARAM, values ->
+        ((Number)values.getOrDefault(SolrCache.SIZE_PARAM, -1.0)).doubleValue());
+  }
+
+  protected double deadBand = DEFAULT_DEAD_BAND;
+  protected double targetHitRatio = DEFAULT_TARGET_HITRATIO;
+  protected long lookupDelta = DEFAULT_LOOKUP_DELTA;
+  protected double maxAdjustRatio = DEFAULT_MAX_ADJUST_RATIO;
+  protected Map<String, Long> lookups = new HashMap<>();
+  protected Map<String, Map<String, Object>> initialComponentLimits = new HashMap<>();
+
+  public CacheManagerPool(String name, String type, ResourceManager resourceManager, Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    super(name, type, resourceManager, poolLimits, poolParams);
+    String str = String.valueOf(poolParams.getOrDefault(DEAD_BAND_PARAM, DEFAULT_DEAD_BAND));
+    try {
+      deadBand = Double.parseDouble(str);
+    } catch (Exception e) {
+      log.warn("Invalid deadBand parameter value '" + str + "', using default " + DEFAULT_DEAD_BAND);
+    }
+  }
+
+  @Override
+  public void registerComponent(SolrCache component) {
+    super.registerComponent(component);
+    initialComponentLimits.put(component.getManagedComponentId().toString(), getResourceLimits(component));
+  }
+
+  @Override
+  public boolean unregisterComponent(String componentId) {
+    lookups.remove(componentId);
+    initialComponentLimits.remove(componentId);
+    return super.unregisterComponent(componentId);
+  }
+
+  @Override
+  public Object doSetResourceLimit(SolrCache component, String limitName, Object val) {
+    if (!(val instanceof Number)) {
+      try {
+        val = Long.parseLong(String.valueOf(val));
+      } catch (Exception e) {
+        throw new IllegalArgumentException("Unsupported value type (not a number) for limit '" + limitName + "': " + val + " (" + val.getClass().getName() + ")");
+      }
+    }
+    Number value = (Number)val;
+    if (value.longValue() > Integer.MAX_VALUE) {
+      throw new IllegalArgumentException("Invalid new value for limit '" + limitName +"': " + value);
+    }
+    switch (limitName) {
+      case SolrCache.MAX_SIZE_PARAM:
+        component.setMaxSize(value.intValue());
+        break;
+      case SolrCache.MAX_RAM_MB_PARAM:
+        component.setMaxRamMB(value.intValue());
+        break;
+      default:
+        throw new IllegalArgumentException("Unsupported limit name '" + limitName + "'");
+    }
+    return value.intValue();
+  }
+
+  @Override
+  public Map<String, Object> getResourceLimits(SolrCache component) {
+    Map<String, Object> limits = new HashMap<>();
+    limits.put(SolrCache.MAX_SIZE_PARAM, component.getMaxSize());
+    limits.put(SolrCache.MAX_RAM_MB_PARAM, component.getMaxRamMB());
+    return limits;
+  }
+
+  @Override
+  public Map<String, Object> getMonitoredValues(SolrCache component) throws Exception {
+    Map<String, Object> values = new HashMap<>();
+    values.put(SolrCache.SIZE_PARAM, component.size());
+    values.put(SolrCache.RAM_BYTES_USED_PARAM, component.ramBytesUsed());
+    SolrMetricsContext metricsContext = component.getSolrMetricsContext();
+    if (metricsContext != null) {
+      Map<String, Object> metrics = metricsContext.getMetricsSnapshot();
+      String key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.HIT_RATIO_PARAM;
+      values.put(SolrCache.HIT_RATIO_PARAM, metrics.get(key));
+      key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.LOOKUPS_PARAM;
+      values.put(SolrCache.LOOKUPS_PARAM, metrics.get(key));
+    }
+    return values;
+  }
+
+  @Override
+  protected void doManage() throws Exception {
+    Map<String, Map<String, Object>> currentValues = getCurrentValues();
+    Map<String, Object> totalValues = aggregateTotalValues(currentValues);
+    // pool limits are defined using controlled tags
+    poolLimits.forEach((poolLimitName, value) -> {
+      // only numeric limits are supported
+      if (value == null || !(value instanceof Number)) {
+        return;
+      }
+      double poolLimitValue = ((Number)value).doubleValue();
+      if (poolLimitValue <= 0) {
+        return;
+      }
+      Function<Map<String, Object>, Double> func = controlledToMonitored.get(poolLimitName);
+      if (func == null) {
+        return;
+      }
+      Double totalValue = func.apply(totalValues);
+      if (totalValue.doubleValue() <= 0.0) {
+        return;
+      }
+      double totalDelta = poolLimitValue - totalValue.doubleValue();
+
+      // dead band to avoid thrashing
+      if (Math.abs(totalDelta / poolLimitValue) < deadBand) {
+        return;
+      }
+
+      List<SolrCache> adjustableComponents = new ArrayList<>();
+      components.forEach((name, component) -> {
+        Map<String, Object> resourceLimits = getResourceLimits((SolrCache) component);
+        Object limit = resourceLimits.get(poolLimitName);
+        // XXX we could attempt here to control eg. ramBytesUsed by adjusting maxSize limit
+        // XXX and vice versa if the current limit is undefined or unsupported
+        if (limit == null || !(limit instanceof Number)) {
+          return;
+        }
+        double currentResourceLimit = ((Number)limit).doubleValue();
+        if (currentResourceLimit <= 0) { // undefined or unsupported
+          return;
+        }
+        adjustableComponents.add(component);
+      });
+      optimize(adjustableComponents, currentValues, poolLimitName, poolLimitValue, totalValue.doubleValue());
+    });
+  }
+
+  /**
+   * Manage all eligible components that support this pool limit.
+   */
+  private void optimize(List<SolrCache> components, Map<String, Map<String, Object>> currentValues, String limitName,
+                        double poolLimitValue, double totalValue) {
+    // changeRatio > 1.0 means there are available free resources
+    // changeRatio < 1.0 means there's shortage of resources
+    final AtomicReference<Double> changeRatio = new AtomicReference<>(poolLimitValue / totalValue);
+
+    // ========================== OPTIMIZATION ==============================
+    // if the situation is not critical (ie. total consumption is less than max)
+    // try to proactively optimize by reducing the size of caches with too high hitRatio
+    // (because a lower hit ratio is still acceptable if it means saving resources) and
+    // expand the size of caches with too low hitRatio
+    final AtomicReference<Double> newTotalValue = new AtomicReference<>(totalValue);
+    components.forEach(component -> {
+      long currentLookups = ((Number)currentValues.get(component.getManagedComponentId().toString()).get(SolrCache.LOOKUPS_PARAM)).longValue();
+      long lastLookups = lookups.computeIfAbsent(component.getManagedComponentId().toString(), k -> 0L);
+      if (currentLookups < lastLookups + lookupDelta) {
+        // too little data, skip the optimization
+        return;
+      }
 
 Review comment:
   If the lookups has been reset due to commit we should simply skip any optimization - because we really don't know how useful is the content of the cache at the moment.
   
   Using cumulative stats is a double-edged sword, specifically because they are never reset... Take for example cumulative hit ratio - it's calculated using cumulative_lookups and cumulative_hits. This means that short-term fluctuations are averaged, but it also means that short-term effects of any adjustments are also averaged (ie. they don't seem to affect the hit ratio in short term), which during thee next round of optimization will cause more and more adjustments, and the controller will eventually grossly overshoot its target. This issue becomes more and more serious as the absolute numbers increase because it's more and more difficult to sway this average.
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r360271832
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/ResourceManagerPool.java
 ##########
 @@ -0,0 +1,216 @@
+package org.apache.solr.managed;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ *
+ */
+public abstract class ResourceManagerPool<T extends ManagedComponent> implements Closeable {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  protected final String name;
+  protected final String type;
+  protected Map<String, Object> poolLimits;
+  protected final Map<String, T> components = new ConcurrentHashMap<>();
+  protected final ResourceManager resourceManager;
+  protected final Class<? extends ManagedComponent> componentClass;
+  private final Map<String, Object> poolParams;
+  protected final ResourcePoolContext poolContext = new ResourcePoolContext();
+  protected final List<ChangeListener> listeners = new ArrayList<>();
+  protected final ReentrantLock updateLock = new ReentrantLock();
+  protected int scheduleDelaySeconds;
+  protected ScheduledFuture<?> scheduledFuture;
+
+  public ResourceManagerPool(String name, String type, ResourceManager resourceManager,
+                                Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    this.name = name;
+    this.type = type;
+    this.resourceManager = resourceManager;
+    this.componentClass = resourceManager.getResourceManagerPoolFactory().getComponentClassByType(type);
+    this.poolLimits = new HashMap<>(poolLimits);
+    this.poolParams = new HashMap<>(poolParams);
+  }
+
+  /** Unique pool name. */
+  public String getName() {
+    return name;
+  }
+
+  /** Pool type. */
+  public String getType() {
+    return type;
+  }
+
+  public ResourceManager getResourceManager() {
+    return resourceManager;
+  }
+
+  /** Add component to this pool. */
+  public void registerComponent(T managedComponent) {
+    if (!componentClass.isAssignableFrom(managedComponent.getClass())) {
+      log.debug("Pool type '" + type + "' is not supported by the component " + managedComponent.getManagedComponentId());
+      return;
+    }
+    ManagedComponent existing = components.putIfAbsent(managedComponent.getManagedComponentId().toString(), managedComponent);
+    if (existing != null) {
+      throw new IllegalArgumentException("Component '" + managedComponent.getManagedComponentId() + "' already exists in pool '" + name + "' !");
+    }
+  }
+
+  /** Remove named component from this pool. */
+  public boolean unregisterComponent(String componentId) {
+    return components.remove(name) != null;
+  }
+
+  /**
+   * Check whether a named component is registered in this pool.
+   * @param componentId component id
+   * @return true if the component with this name is registered, false otherwise.
+   */
+  public boolean isRegistered(String componentId) {
+    return components.containsKey(componentId);
+  }
+
+  /** Get components managed by this pool. */
+  public Map<String, T> getComponents() {
+    return Collections.unmodifiableMap(components);
+  }
+
+  public void addChangeListener(ChangeListener listener) {
+    if (!listeners.contains(listener)) {
+      listeners.add(listener);
+    }
+  }
+
+  public void removeChangeListener(ChangeListener listener) {
+    listeners.remove(listener);
+  }
+
+
+  /**
+   * Get the current monitored values from all resources. Result is a map with resource names as keys,
+   * and param/value maps as values.
+   */
+  public Map<String, Map<String, Object>> getCurrentValues() throws InterruptedException {
+    updateLock.lockInterruptibly();
+    try {
+      // collect the current values
+      Map<String, Map<String, Object>> currentValues = new HashMap<>();
+      for (T managedComponent : components.values()) {
+        try {
+          currentValues.put(managedComponent.getManagedComponentId().toString(), getMonitoredValues(managedComponent));
+        } catch (Exception e) {
+          log.warn("Error getting managed values from " + managedComponent.getManagedComponentId(), e);
+        }
+      }
+      return Collections.unmodifiableMap(currentValues);
+    } finally {
+      updateLock.unlock();
+    }
+  }
+
+  public abstract Map<String, Object> getMonitoredValues(T component) throws Exception;
+
+  public void setResourceLimits(T component, Map<String, Object> limits, ChangeListener.Reason reason) throws Exception {
+    if (limits == null || limits.isEmpty()) {
+      return;
+    }
+    for (Map.Entry<String, Object> entry : limits.entrySet()) {
+      setResourceLimit(component, entry.getKey(), entry.getValue(), reason);
+    }
+  }
+
+  public Object setResourceLimit(T component, String limitName, Object value, ChangeListener.Reason reason) throws Exception {
+    Object newActualLimit = doSetResourceLimit(component, limitName, value);
+    for (ChangeListener listener : listeners) {
+      listener.changedLimit(getName(), component, limitName, value, newActualLimit, reason);
+    }
+    return newActualLimit;
+  }
+
+  protected abstract Object doSetResourceLimit(T component, String limitName, Object value) throws Exception;
+
+  public abstract Map<String, Object> getResourceLimits(T component) throws Exception;
+
+  /**
+   * Calculate aggregated monitored values.
+   * <p>Default implementation of this method simply sums up all non-negative numeric values across
+   * components and ignores any non-numeric values.</p>
+   */
+  public Map<String, Object> aggregateTotalValues(Map<String, Map<String, Object>> perComponentValues) {
+    // calculate the totals
+    Map<String, Object> newTotalValues = new HashMap<>();
+    perComponentValues.values().forEach(map -> map.forEach((k, v) -> {
+      // only calculate totals for numbers
+      if (!(v instanceof Number)) {
+        return;
+      }
+      Double val = ((Number)v).doubleValue();
+      // -1 and MAX_VALUE are our special guard values
+      if (val < 0 || val.longValue() == Long.MAX_VALUE || val.longValue() == Integer.MAX_VALUE) {
+        return;
+      }
+      newTotalValues.merge(k, val, (v1, v2) -> ((Number)v1).doubleValue() + ((Number)v2).doubleValue());
+    }));
+    return newTotalValues;
+  }
+
+  /** Get current pool limits. */
+  public Map<String, Object> getPoolLimits() {
+    return Collections.unmodifiableMap(poolLimits);
+  }
+
+  /**
+   * Pool limits are defined using controlled tags.
+   */
+  public void setPoolLimits(Map<String, Object> poolLimits) {
+    this.poolLimits = new HashMap(poolLimits);
+  }
+
+  /** Get parameters specified during creation. */
+  public Map<String, Object> getParams() {
+    return Collections.unmodifiableMap(poolParams);
+  }
+
+  /**
+   * Pool context used for managing additional pool state.
+   */
+  public ResourcePoolContext getResourcePoolContext() {
+    return poolContext;
+  }
+
+  public void manage() {
+    updateLock.lock();
+    try {
+      doManage();
+    } catch (Exception e) {
+      log.warn("Exception caught managing pool " + getName(), e);
+    } finally {
+      updateLock.unlock();
+    }
+  }
+
+  protected abstract void doManage() throws Exception;
+
+  public void close() throws IOException {
+    if (scheduledFuture != null) {
+      scheduledFuture.cancel(true);
+      scheduledFuture = null;
+    }
+    components.clear();
+    poolContext.clear();
 
 Review comment:
   We should add a listeners.clear() as well.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r360290106
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/types/CacheManagerPool.java
 ##########
 @@ -0,0 +1,329 @@
+/*
+ * 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.solr.managed.types;
+
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+import org.apache.solr.managed.ChangeListener;
+import org.apache.solr.managed.ResourceManager;
+import org.apache.solr.managed.ResourceManagerPool;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.search.SolrCache;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * An implementation of {@link org.apache.solr.managed.ResourceManagerPool} specific to
+ * the management of {@link org.apache.solr.search.SolrCache} instances.
+ * <p>This plugin calculates the total size and maxRamMB of all registered cache instances
+ * and adjusts each cache's limits so that the aggregated values again fit within the pool limits.</p>
+ * <p>In order to avoid thrashing the plugin uses a dead band (by default {@link #DEFAULT_DEAD_BAND}),
+ * which can be adjusted using configuration parameter {@link #DEAD_BAND_PARAM}. If monitored values don't
+ * exceed the limits +/- the dead band then no forcible adjustment takes place.</p>
+ * <p>The management strategy consists of two distinct phases: soft optimization phase and then hard limit phase.</p>
+ * <p><b>Soft optimization</b> tries to adjust the resource consumption based on the cache hit ratio.
+ * This phase is executed only if there's no total limit exceeded. Also, hit ratio is considered a valid monitored
+ * variable only when at least N lookups occurred since the last adjustment (default value is {@link #DEFAULT_LOOKUP_DELTA}).
+ * If the hit ratio is higher than a threshold (default value is {@link #DEFAULT_TARGET_HITRATIO}) then the size
+ * of the cache can be reduced so that the resource consumption is minimized while still keeping acceptable hit
+ * ratio - and vice versa.</p>
+ * <p>This optimization phase can only adjust the limits within a {@link #DEFAULT_MAX_ADJUST_RATIO}, i.e. increased
+ * or decreased values may not be larger / smaller than this multiple / fraction of the initially configured limit.</p>
+ * <p><b>Hard limit</b> phase follows the soft optimization phase and it forcibly reduces resource consumption of all components
+ * if the total usage is still above the pool limit after the first phase has completed. Each component's limit is reduced
+ * by the same factor, regardless of the actual population or hit ratio.</p>
+ */
+public class CacheManagerPool extends ResourceManagerPool<SolrCache> {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static String TYPE = "cache";
+
+  /** Controller dead-band - changes smaller than this ratio will be ignored. */
+  public static final String DEAD_BAND_PARAM = "deadBand";
+  /** Target hit ratio - high enough to be useful, low enough to avoid excessive cache size. */
+  public static final String TARGET_HIT_RATIO_PARAM = "targetHitRatio";
+  /**
+   * Maximum allowed adjustment ratio from the initial configuration value. Adjusted value may not be
+   * higher than multiple of this factor, and not lower than divided by this factor.
+   */
+  public static final String MAX_ADJUST_RATIO_PARAM = "maxAdjustRatio";
+  /**
+   * Minimum number of lookups since last adjustment to consider the reported hitRatio
+   *  to be statistically valid.
+   */
+  public static final String MIN_LOOKUP_DELTA_PARAM = "minLookupDelta";
+  /** Default value of dead band (10%). */
+  public static final double DEFAULT_DEAD_BAND = 0.1;
+  /** Default target hit ratio - a compromise between usefulness and limited resource usage. */
+  public static final double DEFAULT_TARGET_HITRATIO = 0.8;
+  /**
+   * Default minimum number of lookups since the last adjustment. This can be treated as Bernoulli trials
+   * that give a 5% confidence about the statistical validity of hit ratio (<code>0.5 / sqrt(lookups)</code>).
+   */
+  public static final long DEFAULT_LOOKUP_DELTA = 100;
+  /**
+   * Default maximum adjustment ratio from the initially configured values.
+   */
+  public static final double DEFAULT_MAX_ADJUST_RATIO = 2.0;
+
+  protected static final Map<String, Function<Map<String, Object>, Double>> controlledToMonitored = new HashMap<>();
+
+  static {
+    controlledToMonitored.put(SolrCache.MAX_RAM_MB_PARAM, values -> {
+      Number ramBytes = (Number) values.get(SolrCache.RAM_BYTES_USED_PARAM);
+      return ramBytes != null ? ramBytes.doubleValue() / SolrCache.MB : 0.0;
+    });
+    controlledToMonitored.put(SolrCache.MAX_SIZE_PARAM, values ->
+        ((Number)values.getOrDefault(SolrCache.SIZE_PARAM, -1.0)).doubleValue());
+  }
+
+  protected double deadBand = DEFAULT_DEAD_BAND;
+  protected double targetHitRatio = DEFAULT_TARGET_HITRATIO;
+  protected long lookupDelta = DEFAULT_LOOKUP_DELTA;
+  protected double maxAdjustRatio = DEFAULT_MAX_ADJUST_RATIO;
+  protected Map<String, Long> lookups = new HashMap<>();
+  protected Map<String, Map<String, Object>> initialComponentLimits = new HashMap<>();
+
+  public CacheManagerPool(String name, String type, ResourceManager resourceManager, Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    super(name, type, resourceManager, poolLimits, poolParams);
+    String str = String.valueOf(poolParams.getOrDefault(DEAD_BAND_PARAM, DEFAULT_DEAD_BAND));
+    try {
+      deadBand = Double.parseDouble(str);
+    } catch (Exception e) {
+      log.warn("Invalid deadBand parameter value '" + str + "', using default " + DEFAULT_DEAD_BAND);
+    }
+  }
+
+  @Override
+  public void registerComponent(SolrCache component) {
+    super.registerComponent(component);
+    initialComponentLimits.put(component.getManagedComponentId().toString(), getResourceLimits(component));
+  }
+
+  @Override
+  public boolean unregisterComponent(String componentId) {
+    lookups.remove(componentId);
+    initialComponentLimits.remove(componentId);
+    return super.unregisterComponent(componentId);
+  }
+
+  @Override
+  public Object doSetResourceLimit(SolrCache component, String limitName, Object val) {
+    if (!(val instanceof Number)) {
+      try {
+        val = Long.parseLong(String.valueOf(val));
+      } catch (Exception e) {
+        throw new IllegalArgumentException("Unsupported value type (not a number) for limit '" + limitName + "': " + val + " (" + val.getClass().getName() + ")");
+      }
+    }
+    Number value = (Number)val;
+    if (value.longValue() > Integer.MAX_VALUE) {
+      throw new IllegalArgumentException("Invalid new value for limit '" + limitName +"': " + value);
+    }
+    switch (limitName) {
+      case SolrCache.MAX_SIZE_PARAM:
+        component.setMaxSize(value.intValue());
+        break;
+      case SolrCache.MAX_RAM_MB_PARAM:
+        component.setMaxRamMB(value.intValue());
+        break;
+      default:
+        throw new IllegalArgumentException("Unsupported limit name '" + limitName + "'");
+    }
+    return value.intValue();
+  }
+
+  @Override
+  public Map<String, Object> getResourceLimits(SolrCache component) {
+    Map<String, Object> limits = new HashMap<>();
+    limits.put(SolrCache.MAX_SIZE_PARAM, component.getMaxSize());
+    limits.put(SolrCache.MAX_RAM_MB_PARAM, component.getMaxRamMB());
+    return limits;
+  }
+
+  @Override
+  public Map<String, Object> getMonitoredValues(SolrCache component) throws Exception {
+    Map<String, Object> values = new HashMap<>();
+    values.put(SolrCache.SIZE_PARAM, component.size());
+    values.put(SolrCache.RAM_BYTES_USED_PARAM, component.ramBytesUsed());
+    SolrMetricsContext metricsContext = component.getSolrMetricsContext();
+    if (metricsContext != null) {
+      Map<String, Object> metrics = metricsContext.getMetricsSnapshot();
+      String key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.HIT_RATIO_PARAM;
+      values.put(SolrCache.HIT_RATIO_PARAM, metrics.get(key));
+      key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.LOOKUPS_PARAM;
+      values.put(SolrCache.LOOKUPS_PARAM, metrics.get(key));
+    }
+    return values;
+  }
+
+  @Override
+  protected void doManage() throws Exception {
+    Map<String, Map<String, Object>> currentValues = getCurrentValues();
+    Map<String, Object> totalValues = aggregateTotalValues(currentValues);
+    // pool limits are defined using controlled tags
+    poolLimits.forEach((poolLimitName, value) -> {
+      // only numeric limits are supported
+      if (value == null || !(value instanceof Number)) {
+        return;
+      }
+      double poolLimitValue = ((Number)value).doubleValue();
+      if (poolLimitValue <= 0) {
+        return;
+      }
+      Function<Map<String, Object>, Double> func = controlledToMonitored.get(poolLimitName);
+      if (func == null) {
+        return;
+      }
+      Double totalValue = func.apply(totalValues);
+      if (totalValue.doubleValue() <= 0.0) {
+        return;
+      }
+      double totalDelta = poolLimitValue - totalValue.doubleValue();
+
+      // dead band to avoid thrashing
+      if (Math.abs(totalDelta / poolLimitValue) < deadBand) {
+        return;
+      }
+
+      List<SolrCache> adjustableComponents = new ArrayList<>();
+      components.forEach((name, component) -> {
+        Map<String, Object> resourceLimits = getResourceLimits((SolrCache) component);
+        Object limit = resourceLimits.get(poolLimitName);
+        // XXX we could attempt here to control eg. ramBytesUsed by adjusting maxSize limit
+        // XXX and vice versa if the current limit is undefined or unsupported
+        if (limit == null || !(limit instanceof Number)) {
+          return;
+        }
+        double currentResourceLimit = ((Number)limit).doubleValue();
+        if (currentResourceLimit <= 0) { // undefined or unsupported
+          return;
+        }
+        adjustableComponents.add(component);
+      });
+      optimize(adjustableComponents, currentValues, poolLimitName, poolLimitValue, totalValue.doubleValue());
+    });
+  }
+
+  /**
+   * Manage all eligible components that support this pool limit.
+   */
+  private void optimize(List<SolrCache> components, Map<String, Map<String, Object>> currentValues, String limitName,
+                        double poolLimitValue, double totalValue) {
+    // changeRatio > 1.0 means there are available free resources
+    // changeRatio < 1.0 means there's shortage of resources
+    final AtomicReference<Double> changeRatio = new AtomicReference<>(poolLimitValue / totalValue);
+
+    // ========================== OPTIMIZATION ==============================
+    // if the situation is not critical (ie. total consumption is less than max)
+    // try to proactively optimize by reducing the size of caches with too high hitRatio
+    // (because a lower hit ratio is still acceptable if it means saving resources) and
+    // expand the size of caches with too low hitRatio
+    final AtomicReference<Double> newTotalValue = new AtomicReference<>(totalValue);
+    components.forEach(component -> {
+      long currentLookups = ((Number)currentValues.get(component.getManagedComponentId().toString()).get(SolrCache.LOOKUPS_PARAM)).longValue();
+      long lastLookups = lookups.computeIfAbsent(component.getManagedComponentId().toString(), k -> 0L);
+      if (currentLookups < lastLookups + lookupDelta) {
+        // too little data, skip the optimization
+        return;
+      }
 
 Review comment:
   Perhaps we can use cumulative lookups to check delta calculations but use lookups to make sure a minimum number of lookups have actually happened to give us meaningful hit ratio data

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r368993331
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/ResourceManagerPool.java
 ##########
 @@ -0,0 +1,294 @@
+/*
+ * 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.solr.managed;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReentrantLock;
+
+import com.codahale.metrics.Timer;
+import org.apache.solr.core.SolrInfoBean;
+import org.apache.solr.metrics.MetricsMap;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ *
+ */
+public abstract class ResourceManagerPool<T extends ManagedComponent> implements SolrInfoBean, Closeable {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  protected final String name;
+  protected final String type;
+  protected Map<String, Object> poolLimits;
+  protected final Map<String, T> components = new ConcurrentHashMap<>();
+  protected final ResourceManager resourceManager;
+  protected final Class<? extends ManagedComponent> componentClass;
+  protected Map<String, Object> poolParams;
+  protected final ResourcePoolContext poolContext = new ResourcePoolContext();
+  protected final Set<ChangeListener> listeners = new CopyOnWriteArraySet<>();
+  protected final ReentrantLock manageLock = new ReentrantLock();
+  protected int scheduleDelaySeconds;
+  protected ScheduledFuture<?> scheduledFuture;
+  protected SolrMetricsContext solrMetricsContext;
+  protected Timer manageTimer;
+  protected Map<ChangeListener.Reason, AtomicLong> changeCounts = new ConcurrentHashMap<>();
+
+  public ResourceManagerPool(String name, String type, ResourceManager resourceManager,
+                                Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    this.name = name;
+    this.type = type;
+    this.resourceManager = resourceManager;
+    this.componentClass = resourceManager.getResourceManagerPoolFactory().getComponentClassByType(type);
+    setPoolLimits(poolLimits);
+    setPoolParams(poolParams);
+  }
+
+  /** Unique pool name. */
+  public String getName() {
+    return name;
+  }
+
+  /** Pool type. */
+  public String getType() {
+    return type;
+  }
+
+  public Category getCategory() {
+    return Category.RESOURCE;
+  }
+
+  public String getDescription() {
+    return getName() + "/" + getType() + " (" + getClass().getSimpleName() + ")";
+  }
+
+  @Override
+  public void initializeMetrics(SolrMetricsContext parentContext, String childScope) {
+    solrMetricsContext = parentContext.getChildContext(this, childScope);
+    manageTimer = solrMetricsContext.timer("manageTimes", getCategory().toString(), "pool", getType());
+    MetricsMap changeMap = new MetricsMap((detailed, map) -> {
+      changeCounts.forEach((k, v) -> map.put(k.toString(), v.get()));
+    });
+    solrMetricsContext.gauge(changeMap, true, "changes", getCategory().toString(), "pool", getType());
+    solrMetricsContext.gauge(new MetricsMap((detailed, map) -> map.putAll(poolLimits)), true, "poolLimits", getCategory().toString(), "pool", getType());
+    solrMetricsContext.gauge(new MetricsMap((detailed, map) -> {
+      try {
+        Map<String, Object> totalValues = aggregateTotalValues(getCurrentValues());
+        map.putAll(totalValues);
+      } catch (Exception e) {
+        log.warn("Exception retrieving current values in pool {} / {}: {}", getName(), getType(), e);
+      }
+    }), true, "totalValues", getCategory().toString(), "pool", getType());
+  }
+
+  @Override
+  public SolrMetricsContext getSolrMetricsContext() {
+    return solrMetricsContext;
+  }
+
+  public ResourceManager getResourceManager() {
+    return resourceManager;
+  }
+
+  /** Add component to this pool. */
+  public void registerComponent(T managedComponent) {
+    if (!componentClass.isAssignableFrom(managedComponent.getClass())) {
+      log.debug("Pool type '" + type + "' is not supported by the component " + managedComponent.getManagedComponentId());
+      return;
+    }
+    ManagedComponent existing = components.putIfAbsent(managedComponent.getManagedComponentId().toString(), managedComponent);
+    if (existing != null) {
+      throw new IllegalArgumentException("Component '" + managedComponent.getManagedComponentId() + "' already exists in pool '" + name + "' !");
+    }
+  }
+
+  /** Remove named component from this pool. */
+  public boolean unregisterComponent(String componentId) {
+    return components.remove(componentId) != null;
+  }
+
+  /**
+   * Check whether a named component is registered in this pool.
+   * @param componentId component id
+   * @return true if the component with this name is registered, false otherwise.
+   */
+  public boolean isRegistered(String componentId) {
+    return components.containsKey(componentId);
+  }
+
+  /** Get components managed by this pool. */
+  public Map<String, T> getComponents() {
+    return Collections.unmodifiableMap(components);
+  }
+
+  public void addChangeListener(ChangeListener listener) {
+    listeners.add(listener);
+  }
+
+  public void removeChangeListener(ChangeListener listener) {
+    listeners.remove(listener);
+  }
+
+
+  /**
+   * Get the current monitored values from all resources. Result is a map with resource names as keys,
+   * and param/value maps as values.
+   */
+  public Map<String, Map<String, Object>> getCurrentValues() throws InterruptedException {
+    // collect the current values
+    Map<String, Map<String, Object>> currentValues = new HashMap<>();
+    for (T managedComponent : components.values()) {
+      try {
+        currentValues.put(managedComponent.getManagedComponentId().toString(), getMonitoredValues(managedComponent));
+      } catch (Exception e) {
+        log.warn("Error getting managed values from " + managedComponent.getManagedComponentId(), e);
+      }
+    }
+    return Collections.unmodifiableMap(currentValues);
+  }
+
+  public abstract Map<String, Object> getMonitoredValues(T component) throws Exception;
+
+  public void setResourceLimits(T component, Map<String, Object> limits, ChangeListener.Reason reason) throws Exception {
+    if (limits == null || limits.isEmpty()) {
+      return;
+    }
+    for (Map.Entry<String, Object> entry : limits.entrySet()) {
+      setResourceLimit(component, entry.getKey(), entry.getValue(), reason);
+    }
+  }
+
+  public Object setResourceLimit(T component, String limitName, Object value, ChangeListener.Reason reason) throws Exception {
+    try {
+      Object newActualLimit = doSetResourceLimit(component, limitName, value);
+      changeCounts.computeIfAbsent(reason, r -> new AtomicLong()).incrementAndGet();
+      for (ChangeListener listener : listeners) {
+        listener.changedLimit(getName(), component, limitName, value, newActualLimit, reason);
+      }
+      return newActualLimit;
+    } catch (Throwable t) {
+      for (ChangeListener listener : listeners) {
+        listener.onError(getName(), component, limitName, value, reason, t);
+      }
+      throw t;
+    }
+  }
+
+  protected abstract Object doSetResourceLimit(T component, String limitName, Object value) throws Exception;
+
+  public abstract Map<String, Object> getResourceLimits(T component) throws Exception;
+
+  /**
+   * Calculate aggregated monitored values.
+   * <p>Default implementation of this method simply sums up all non-negative numeric values across
+   * components and ignores any non-numeric values.</p>
+   */
+  public Map<String, Object> aggregateTotalValues(Map<String, Map<String, Object>> perComponentValues) {
+    // calculate the totals
+    Map<String, Object> newTotalValues = new HashMap<>();
+    perComponentValues.values().forEach(map -> map.forEach((k, v) -> {
+      // only calculate totals for numbers
+      if (!(v instanceof Number)) {
+        return;
+      }
+      Double val = ((Number)v).doubleValue();
+      // -1 and MAX_VALUE are our special guard values
+      if (val < 0 || val.longValue() == Long.MAX_VALUE || val.longValue() == Integer.MAX_VALUE) {
+        return;
+      }
+      newTotalValues.merge(k, val, (v1, v2) -> ((Number)v1).doubleValue() + ((Number)v2).doubleValue());
+    }));
+    return newTotalValues;
+  }
+
+  /** Get current pool limits. */
+  public Map<String, Object> getPoolLimits() {
+    return Collections.unmodifiableMap(poolLimits);
+  }
+
+  /**
+   * Pool limits are defined using controlled tags.
+   */
+  public void setPoolLimits(Map<String, Object> poolLimits) {
+    if (poolLimits != null) {
+      this.poolLimits = new HashMap(poolLimits);
+    }
+  }
+
+  /** Get parameters specified during creation. */
+  public Map<String, Object> getPoolParams() {
+    return Collections.unmodifiableMap(poolParams);
+  }
+
+  public void setPoolParams(Map<String, Object> poolParams) {
+    if (poolParams != null) {
+      this.poolParams = new HashMap<>(poolParams);
+      this.scheduleDelaySeconds = Integer.parseInt(String.valueOf(poolParams.getOrDefault(SCHEDULE_DELAY_SECONDS_PARAM, DEFAULT_SCHEDULE_DELAY_SECONDS)));
+    }
+  }
+
+  /**
+   * Pool context used for managing additional pool state.
+   */
+  public ResourcePoolContext getResourcePoolContext() {
+    return poolContext;
+  }
+
+  public void manage() {
+    manageLock.lock();
 
 Review comment:
   I think we can get rid of the manageLock completely. The manage method is only called by the scheduled executor which guarantees that the effect of previous executions happens-before the subsequent ones.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r365719144
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
 
 Review comment:
   This is modified in superclass `setEnabled(Boolean)`, which in turn is invoked by `PluginUtils.invokeSetters`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359743404
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/DefaultResourceManager.java
 ##########
 @@ -0,0 +1,231 @@
+/*
+ * 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.solr.managed;
+
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledThreadPoolExecutor;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.solr.common.params.CommonParams;
+import org.apache.solr.common.util.ExecutorUtil;
+import org.apache.solr.common.util.IOUtils;
+import org.apache.solr.common.util.TimeSource;
+import org.apache.solr.core.PluginInfo;
+import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.managed.types.CacheManagerPool;
+import org.apache.solr.search.SolrCache;
+import org.apache.solr.util.DefaultSolrThreadFactory;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Default implementation of {@link ResourceManager}.
+ * <p>Resource pools managed by this implementation are run periodically, each according to
+ * its schedule defined by {@link #SCHEDULE_DELAY_SECONDS_PARAM} parameter during the pool creation.</p>
+ */
+public class DefaultResourceManager extends ResourceManager {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+
+  public static final String SCHEDULE_DELAY_SECONDS_PARAM = "scheduleDelaySeconds";
+  public static final String MAX_NUM_POOLS_PARAM = "maxNumPools";
+
+  public static final int DEFAULT_MAX_POOLS = 100;
+  public static final int DEFAULT_SCHEDULE_DELAY_SECONDS = 10;
+
+  public static final String NODE_SEARCHER_CACHE_POOL = "nodeSearcherCachePool";
+
+  public static final Map<String, Map<String, Object>> DEFAULT_NODE_POOLS = new HashMap<>();
+
+  static {
+    Map<String, Object> params = new HashMap<>();
+    params.put(CommonParams.TYPE, CacheManagerPool.TYPE);
+    // unlimited RAM
+    params.put(SolrCache.MAX_RAM_MB_PARAM, -1L);
+    DEFAULT_NODE_POOLS.put(NODE_SEARCHER_CACHE_POOL, params);
+  }
+
+
+  protected int maxNumPools = DEFAULT_MAX_POOLS;
+
+  protected Map<String, ResourceManagerPool> resourcePools = new ConcurrentHashMap<>();
+
+
+  private TimeSource timeSource;
+
+  /**
+   * Thread pool for scheduling the pool runs.
+   */
+  private ScheduledThreadPoolExecutor scheduledThreadPoolExecutor;
+
+  protected boolean isClosed = false;
+  protected boolean enabled = true;
 
 Review comment:
   The `enabled` appears to be unused?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r360289452
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/types/CacheManagerPool.java
 ##########
 @@ -0,0 +1,329 @@
+/*
+ * 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.solr.managed.types;
+
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
+
+import org.apache.solr.managed.ChangeListener;
+import org.apache.solr.managed.ResourceManager;
+import org.apache.solr.managed.ResourceManagerPool;
+import org.apache.solr.metrics.SolrMetricsContext;
+import org.apache.solr.search.SolrCache;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * An implementation of {@link org.apache.solr.managed.ResourceManagerPool} specific to
+ * the management of {@link org.apache.solr.search.SolrCache} instances.
+ * <p>This plugin calculates the total size and maxRamMB of all registered cache instances
+ * and adjusts each cache's limits so that the aggregated values again fit within the pool limits.</p>
+ * <p>In order to avoid thrashing the plugin uses a dead band (by default {@link #DEFAULT_DEAD_BAND}),
+ * which can be adjusted using configuration parameter {@link #DEAD_BAND_PARAM}. If monitored values don't
+ * exceed the limits +/- the dead band then no forcible adjustment takes place.</p>
+ * <p>The management strategy consists of two distinct phases: soft optimization phase and then hard limit phase.</p>
+ * <p><b>Soft optimization</b> tries to adjust the resource consumption based on the cache hit ratio.
+ * This phase is executed only if there's no total limit exceeded. Also, hit ratio is considered a valid monitored
+ * variable only when at least N lookups occurred since the last adjustment (default value is {@link #DEFAULT_LOOKUP_DELTA}).
+ * If the hit ratio is higher than a threshold (default value is {@link #DEFAULT_TARGET_HITRATIO}) then the size
+ * of the cache can be reduced so that the resource consumption is minimized while still keeping acceptable hit
+ * ratio - and vice versa.</p>
+ * <p>This optimization phase can only adjust the limits within a {@link #DEFAULT_MAX_ADJUST_RATIO}, i.e. increased
+ * or decreased values may not be larger / smaller than this multiple / fraction of the initially configured limit.</p>
+ * <p><b>Hard limit</b> phase follows the soft optimization phase and it forcibly reduces resource consumption of all components
+ * if the total usage is still above the pool limit after the first phase has completed. Each component's limit is reduced
+ * by the same factor, regardless of the actual population or hit ratio.</p>
+ */
+public class CacheManagerPool extends ResourceManagerPool<SolrCache> {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static String TYPE = "cache";
+
+  /** Controller dead-band - changes smaller than this ratio will be ignored. */
+  public static final String DEAD_BAND_PARAM = "deadBand";
+  /** Target hit ratio - high enough to be useful, low enough to avoid excessive cache size. */
+  public static final String TARGET_HIT_RATIO_PARAM = "targetHitRatio";
+  /**
+   * Maximum allowed adjustment ratio from the initial configuration value. Adjusted value may not be
+   * higher than multiple of this factor, and not lower than divided by this factor.
+   */
+  public static final String MAX_ADJUST_RATIO_PARAM = "maxAdjustRatio";
+  /**
+   * Minimum number of lookups since last adjustment to consider the reported hitRatio
+   *  to be statistically valid.
+   */
+  public static final String MIN_LOOKUP_DELTA_PARAM = "minLookupDelta";
+  /** Default value of dead band (10%). */
+  public static final double DEFAULT_DEAD_BAND = 0.1;
+  /** Default target hit ratio - a compromise between usefulness and limited resource usage. */
+  public static final double DEFAULT_TARGET_HITRATIO = 0.8;
+  /**
+   * Default minimum number of lookups since the last adjustment. This can be treated as Bernoulli trials
+   * that give a 5% confidence about the statistical validity of hit ratio (<code>0.5 / sqrt(lookups)</code>).
+   */
+  public static final long DEFAULT_LOOKUP_DELTA = 100;
+  /**
+   * Default maximum adjustment ratio from the initially configured values.
+   */
+  public static final double DEFAULT_MAX_ADJUST_RATIO = 2.0;
+
+  protected static final Map<String, Function<Map<String, Object>, Double>> controlledToMonitored = new HashMap<>();
+
+  static {
+    controlledToMonitored.put(SolrCache.MAX_RAM_MB_PARAM, values -> {
+      Number ramBytes = (Number) values.get(SolrCache.RAM_BYTES_USED_PARAM);
+      return ramBytes != null ? ramBytes.doubleValue() / SolrCache.MB : 0.0;
+    });
+    controlledToMonitored.put(SolrCache.MAX_SIZE_PARAM, values ->
+        ((Number)values.getOrDefault(SolrCache.SIZE_PARAM, -1.0)).doubleValue());
+  }
+
+  protected double deadBand = DEFAULT_DEAD_BAND;
+  protected double targetHitRatio = DEFAULT_TARGET_HITRATIO;
+  protected long lookupDelta = DEFAULT_LOOKUP_DELTA;
+  protected double maxAdjustRatio = DEFAULT_MAX_ADJUST_RATIO;
+  protected Map<String, Long> lookups = new HashMap<>();
+  protected Map<String, Map<String, Object>> initialComponentLimits = new HashMap<>();
+
+  public CacheManagerPool(String name, String type, ResourceManager resourceManager, Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    super(name, type, resourceManager, poolLimits, poolParams);
+    String str = String.valueOf(poolParams.getOrDefault(DEAD_BAND_PARAM, DEFAULT_DEAD_BAND));
+    try {
+      deadBand = Double.parseDouble(str);
+    } catch (Exception e) {
+      log.warn("Invalid deadBand parameter value '" + str + "', using default " + DEFAULT_DEAD_BAND);
+    }
+  }
+
+  @Override
+  public void registerComponent(SolrCache component) {
+    super.registerComponent(component);
+    initialComponentLimits.put(component.getManagedComponentId().toString(), getResourceLimits(component));
+  }
+
+  @Override
+  public boolean unregisterComponent(String componentId) {
+    lookups.remove(componentId);
+    initialComponentLimits.remove(componentId);
+    return super.unregisterComponent(componentId);
+  }
+
+  @Override
+  public Object doSetResourceLimit(SolrCache component, String limitName, Object val) {
+    if (!(val instanceof Number)) {
+      try {
+        val = Long.parseLong(String.valueOf(val));
+      } catch (Exception e) {
+        throw new IllegalArgumentException("Unsupported value type (not a number) for limit '" + limitName + "': " + val + " (" + val.getClass().getName() + ")");
+      }
+    }
+    Number value = (Number)val;
+    if (value.longValue() > Integer.MAX_VALUE) {
+      throw new IllegalArgumentException("Invalid new value for limit '" + limitName +"': " + value);
+    }
+    switch (limitName) {
+      case SolrCache.MAX_SIZE_PARAM:
+        component.setMaxSize(value.intValue());
+        break;
+      case SolrCache.MAX_RAM_MB_PARAM:
+        component.setMaxRamMB(value.intValue());
+        break;
+      default:
+        throw new IllegalArgumentException("Unsupported limit name '" + limitName + "'");
+    }
+    return value.intValue();
+  }
+
+  @Override
+  public Map<String, Object> getResourceLimits(SolrCache component) {
+    Map<String, Object> limits = new HashMap<>();
+    limits.put(SolrCache.MAX_SIZE_PARAM, component.getMaxSize());
+    limits.put(SolrCache.MAX_RAM_MB_PARAM, component.getMaxRamMB());
+    return limits;
+  }
+
+  @Override
+  public Map<String, Object> getMonitoredValues(SolrCache component) throws Exception {
+    Map<String, Object> values = new HashMap<>();
+    values.put(SolrCache.SIZE_PARAM, component.size());
+    values.put(SolrCache.RAM_BYTES_USED_PARAM, component.ramBytesUsed());
+    SolrMetricsContext metricsContext = component.getSolrMetricsContext();
+    if (metricsContext != null) {
+      Map<String, Object> metrics = metricsContext.getMetricsSnapshot();
+      String key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.HIT_RATIO_PARAM;
+      values.put(SolrCache.HIT_RATIO_PARAM, metrics.get(key));
+      key = component.getCategory().toString() + "." + metricsContext.getScope() + "." + SolrCache.LOOKUPS_PARAM;
+      values.put(SolrCache.LOOKUPS_PARAM, metrics.get(key));
+    }
+    return values;
+  }
+
+  @Override
+  protected void doManage() throws Exception {
+    Map<String, Map<String, Object>> currentValues = getCurrentValues();
+    Map<String, Object> totalValues = aggregateTotalValues(currentValues);
+    // pool limits are defined using controlled tags
+    poolLimits.forEach((poolLimitName, value) -> {
+      // only numeric limits are supported
+      if (value == null || !(value instanceof Number)) {
+        return;
+      }
+      double poolLimitValue = ((Number)value).doubleValue();
+      if (poolLimitValue <= 0) {
+        return;
+      }
+      Function<Map<String, Object>, Double> func = controlledToMonitored.get(poolLimitName);
+      if (func == null) {
+        return;
+      }
+      Double totalValue = func.apply(totalValues);
+      if (totalValue.doubleValue() <= 0.0) {
+        return;
+      }
+      double totalDelta = poolLimitValue - totalValue.doubleValue();
+
+      // dead band to avoid thrashing
+      if (Math.abs(totalDelta / poolLimitValue) < deadBand) {
+        return;
+      }
+
+      List<SolrCache> adjustableComponents = new ArrayList<>();
+      components.forEach((name, component) -> {
+        Map<String, Object> resourceLimits = getResourceLimits((SolrCache) component);
+        Object limit = resourceLimits.get(poolLimitName);
+        // XXX we could attempt here to control eg. ramBytesUsed by adjusting maxSize limit
+        // XXX and vice versa if the current limit is undefined or unsupported
+        if (limit == null || !(limit instanceof Number)) {
+          return;
+        }
+        double currentResourceLimit = ((Number)limit).doubleValue();
+        if (currentResourceLimit <= 0) { // undefined or unsupported
+          return;
+        }
+        adjustableComponents.add(component);
+      });
+      optimize(adjustableComponents, currentValues, poolLimitName, poolLimitValue, totalValue.doubleValue());
+    });
+  }
+
+  /**
+   * Manage all eligible components that support this pool limit.
+   */
+  private void optimize(List<SolrCache> components, Map<String, Map<String, Object>> currentValues, String limitName,
+                        double poolLimitValue, double totalValue) {
+    // changeRatio > 1.0 means there are available free resources
+    // changeRatio < 1.0 means there's shortage of resources
+    final AtomicReference<Double> changeRatio = new AtomicReference<>(poolLimitValue / totalValue);
+
+    // ========================== OPTIMIZATION ==============================
+    // if the situation is not critical (ie. total consumption is less than max)
+    // try to proactively optimize by reducing the size of caches with too high hitRatio
+    // (because a lower hit ratio is still acceptable if it means saving resources) and
+    // expand the size of caches with too low hitRatio
+    final AtomicReference<Double> newTotalValue = new AtomicReference<>(totalValue);
+    components.forEach(component -> {
+      long currentLookups = ((Number)currentValues.get(component.getManagedComponentId().toString()).get(SolrCache.LOOKUPS_PARAM)).longValue();
+      long lastLookups = lookups.computeIfAbsent(component.getManagedComponentId().toString(), k -> 0L);
+      if (currentLookups < lastLookups + lookupDelta) {
+        // too little data, skip the optimization
+        return;
+      }
 
 Review comment:
   What if a commit happened between the last adjustment and now and therefore the current value of `lookups` has been reset?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r359712223
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
 ##########
 @@ -1012,6 +1045,10 @@ public void shutdown() {
         }
       }
 
+      if (resourceManager != null) {
 
 Review comment:
   Nit: the null check is redundant, closeQuietly will null check anyway

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
sigram commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r363828310
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/ResourceManagerPool.java
 ##########
 @@ -0,0 +1,216 @@
+package org.apache.solr.managed;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ *
+ */
+public abstract class ResourceManagerPool<T extends ManagedComponent> implements Closeable {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  protected final String name;
+  protected final String type;
+  protected Map<String, Object> poolLimits;
+  protected final Map<String, T> components = new ConcurrentHashMap<>();
+  protected final ResourceManager resourceManager;
+  protected final Class<? extends ManagedComponent> componentClass;
+  private final Map<String, Object> poolParams;
+  protected final ResourcePoolContext poolContext = new ResourcePoolContext();
+  protected final List<ChangeListener> listeners = new ArrayList<>();
+  protected final ReentrantLock updateLock = new ReentrantLock();
+  protected int scheduleDelaySeconds;
+  protected ScheduledFuture<?> scheduledFuture;
+
+  public ResourceManagerPool(String name, String type, ResourceManager resourceManager,
+                                Map<String, Object> poolLimits, Map<String, Object> poolParams) {
+    this.name = name;
+    this.type = type;
+    this.resourceManager = resourceManager;
+    this.componentClass = resourceManager.getResourceManagerPoolFactory().getComponentClassByType(type);
+    this.poolLimits = new HashMap<>(poolLimits);
+    this.poolParams = new HashMap<>(poolParams);
+  }
+
+  /** Unique pool name. */
+  public String getName() {
+    return name;
+  }
+
+  /** Pool type. */
+  public String getType() {
+    return type;
+  }
+
+  public ResourceManager getResourceManager() {
+    return resourceManager;
+  }
+
+  /** Add component to this pool. */
+  public void registerComponent(T managedComponent) {
+    if (!componentClass.isAssignableFrom(managedComponent.getClass())) {
+      log.debug("Pool type '" + type + "' is not supported by the component " + managedComponent.getManagedComponentId());
+      return;
+    }
+    ManagedComponent existing = components.putIfAbsent(managedComponent.getManagedComponentId().toString(), managedComponent);
+    if (existing != null) {
+      throw new IllegalArgumentException("Component '" + managedComponent.getManagedComponentId() + "' already exists in pool '" + name + "' !");
+    }
+  }
+
+  /** Remove named component from this pool. */
+  public boolean unregisterComponent(String componentId) {
+    return components.remove(name) != null;
+  }
+
+  /**
+   * Check whether a named component is registered in this pool.
+   * @param componentId component id
+   * @return true if the component with this name is registered, false otherwise.
+   */
+  public boolean isRegistered(String componentId) {
+    return components.containsKey(componentId);
+  }
+
+  /** Get components managed by this pool. */
+  public Map<String, T> getComponents() {
+    return Collections.unmodifiableMap(components);
+  }
+
+  public void addChangeListener(ChangeListener listener) {
+    if (!listeners.contains(listener)) {
+      listeners.add(listener);
+    }
+  }
+
+  public void removeChangeListener(ChangeListener listener) {
+    listeners.remove(listener);
+  }
+
+
+  /**
+   * Get the current monitored values from all resources. Result is a map with resource names as keys,
+   * and param/value maps as values.
+   */
+  public Map<String, Map<String, Object>> getCurrentValues() throws InterruptedException {
+    updateLock.lockInterruptibly();
 
 Review comment:
   Yes, it's a left-over from a previous refactoring.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org


[GitHub] [lucene-solr] shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API

Posted by GitBox <gi...@apache.org>.
shalinmangar commented on a change in pull request #1100: SOLR-13579: Create resource management API
URL: https://github.com/apache/lucene-solr/pull/1100#discussion_r360271349
 
 

 ##########
 File path: solr/core/src/java/org/apache/solr/managed/ResourceManagerPool.java
 ##########
 @@ -0,0 +1,216 @@
+package org.apache.solr.managed;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.locks.ReentrantLock;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ *
+ */
+public abstract class ResourceManagerPool<T extends ManagedComponent> implements Closeable {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  protected final String name;
+  protected final String type;
+  protected Map<String, Object> poolLimits;
+  protected final Map<String, T> components = new ConcurrentHashMap<>();
+  protected final ResourceManager resourceManager;
+  protected final Class<? extends ManagedComponent> componentClass;
+  private final Map<String, Object> poolParams;
+  protected final ResourcePoolContext poolContext = new ResourcePoolContext();
+  protected final List<ChangeListener> listeners = new ArrayList<>();
 
 Review comment:
   The listeners ArrayList is not thread-safe. Perhaps this should be a CopyOnWriteArraySet to avoid accidental duplicates in the list. If yes, we should modify the ChangeListener javadoc to say that equals and hashCode methods are required.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org