lucene-solr:master: SOLR-8892: Allow SolrInfoMBeans to return different statistics for /jmx vs web ui calls

[gc...@apache.org]

Repository: lucene-solr
Updated Branches:
  refs/heads/master 3bbf8aaa8 -> fd0d69974


SOLR-8892: Allow SolrInfoMBeans to return different statistics for /jmx vs web ui calls


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/fd0d6997
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/fd0d6997
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/fd0d6997

Branch: refs/heads/master
Commit: fd0d6997477606f9401e602519d903fb2de0e013
Parents: 3bbf8aa
Author: Gregory Chanan <gc...@cloudera.com>
Authored: Thu Mar 31 17:16:17 2016 -0700
Committer: Gregory Chanan <gc...@cloudera.com>
Committed: Tue Apr 5 12:01:40 2016 -0700

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  3 +
 .../org/apache/solr/core/JmxMonitoredMap.java   | 23 +++++++-
 .../apache/solr/core/SolrInfoMBeanWrapper.java  | 62 ++++++++++++++++++++
 .../apache/solr/search/SolrFieldCacheMBean.java | 15 ++++-
 .../apache/solr/core/TestJmxMonitoredMap.java   | 37 ++++++++++++
 .../solr/search/TestSolrFieldCacheMBean.java    | 36 +++++++++---
 6 files changed, 165 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fd0d6997/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 9e6da65..d90a628 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -133,6 +133,9 @@ Other Changes
 
 * SOLR-8869: Optionally disable printing field cache entries in SolrFieldCacheMBean (Gregory Chanan)
 
+* SOLR-8892: Allow SolrInfoMBeans to return different statistics for /jmx vs web ui calls.
+  (Gregory Chana, Mark Miller)
+
 ==================  6.0.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fd0d6997/solr/core/src/java/org/apache/solr/core/JmxMonitoredMap.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/JmxMonitoredMap.java b/solr/core/src/java/org/apache/solr/core/JmxMonitoredMap.java
index 0260f51..57bde93 100644
--- a/solr/core/src/java/org/apache/solr/core/JmxMonitoredMap.java
+++ b/solr/core/src/java/org/apache/solr/core/JmxMonitoredMap.java
@@ -258,7 +258,15 @@ public class JmxMonitoredMap<K, V> extends
 
     public SolrDynamicMBean(String coreHashCode, SolrInfoMBean managedResource, boolean useCachedStatsBetweenGetMBeanInfoCalls) {
       this.useCachedStatsBetweenGetMBeanInfoCalls = useCachedStatsBetweenGetMBeanInfoCalls;
-      this.infoBean = managedResource;
+      if (managedResource instanceof JmxAugmentedSolrInfoMBean) {
+        final JmxAugmentedSolrInfoMBean jmxSpecific = (JmxAugmentedSolrInfoMBean)managedResource;
+        this.infoBean = new SolrInfoMBeanWrapper(jmxSpecific) {
+          @Override
+          public NamedList getStatistics() { return jmxSpecific.getStatisticsForJmx(); }
+        };
+      } else {
+        this.infoBean = managedResource;
+      }
       staticStats = new HashSet<>();
 
       // For which getters are already available in SolrInfoMBean
@@ -412,4 +420,17 @@ public class JmxMonitoredMap<K, V> extends
       throw new UnsupportedOperationException("Operation not Supported");
     }
   }
+
+  /**
+   * SolrInfoMBean that provides JMX-specific statistics.  Used, for example,
+   * if generating full statistics is expensive; the expensive statistics can
+   * be generated normally for use with the web ui, while an abbreviated version
+   * are generated for period jmx use.
+   */
+  public interface JmxAugmentedSolrInfoMBean extends SolrInfoMBean {
+    /**
+     * JMX-specific statistics
+     */
+    public NamedList getStatisticsForJmx();
+  }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fd0d6997/solr/core/src/java/org/apache/solr/core/SolrInfoMBeanWrapper.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/core/SolrInfoMBeanWrapper.java b/solr/core/src/java/org/apache/solr/core/SolrInfoMBeanWrapper.java
new file mode 100644
index 0000000..534b884
--- /dev/null
+++ b/solr/core/src/java/org/apache/solr/core/SolrInfoMBeanWrapper.java
@@ -0,0 +1,62 @@
+/*
+ * 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.core;
+
+import java.net.URL;
+
+import org.apache.solr.common.util.NamedList;
+
+/**
+ * Wraps a {@link SolrInfoMBean}.
+ */
+public class SolrInfoMBeanWrapper implements SolrInfoMBean {
+  private final SolrInfoMBean mbean;
+
+  public SolrInfoMBeanWrapper(SolrInfoMBean mbean) {
+    this.mbean = mbean;
+  }
+
+  /** {@inheritDoc} */
+  @Override
+  public String getName() { return mbean.getName(); }
+
+  /** {@inheritDoc} */
+  @Override
+  public String getVersion() { return mbean.getVersion(); }
+
+  /** {@inheritDoc} */
+  @Override
+  public String getDescription() { return mbean.getDescription(); }
+
+  /** {@inheritDoc} */
+  @Override
+  public Category getCategory() { return mbean.getCategory(); }
+
+  /** {@inheritDoc} */
+  @Override
+  public String getSource() { return mbean.getSource(); }
+
+  /** {@inheritDoc} */
+  @Override
+  public URL[] getDocs() { return mbean.getDocs(); }
+
+  /** {@inheritDoc} */
+  @Override
+  public NamedList getStatistics() { return mbean.getStatistics(); }
+
+}

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fd0d6997/solr/core/src/java/org/apache/solr/search/SolrFieldCacheMBean.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/SolrFieldCacheMBean.java b/solr/core/src/java/org/apache/solr/search/SolrFieldCacheMBean.java
index cfc68c4..da99708 100644
--- a/solr/core/src/java/org/apache/solr/search/SolrFieldCacheMBean.java
+++ b/solr/core/src/java/org/apache/solr/search/SolrFieldCacheMBean.java
@@ -24,14 +24,16 @@ import org.apache.solr.common.util.SimpleOrderedMap;
 
 import org.apache.solr.core.SolrCore;
 import org.apache.solr.core.SolrInfoMBean;
+import org.apache.solr.core.JmxMonitoredMap.JmxAugmentedSolrInfoMBean;
 
 /**
  * A SolrInfoMBean that provides introspection of the Solr FieldCache
  *
  */
-public class SolrFieldCacheMBean implements SolrInfoMBean {
+public class SolrFieldCacheMBean implements JmxAugmentedSolrInfoMBean {
 
   private boolean disableEntryList = Boolean.getBoolean("disableSolrFieldCacheMBeanEntryList");
+  private boolean disableJmxEntryList = Boolean.getBoolean("disableSolrFieldCacheMBeanEntryListJmx");
 
   @Override
   public String getName() { return this.getClass().getName(); }
@@ -51,10 +53,19 @@ public class SolrFieldCacheMBean implements SolrInfoMBean {
   }
   @Override
   public NamedList getStatistics() {
+    return getStats(!disableEntryList);
+  }
+
+  @Override
+  public NamedList getStatisticsForJmx() {
+    return getStats(!disableEntryList && !disableJmxEntryList);
+  }
+
+  private NamedList getStats(boolean listEntries) {
     NamedList stats = new SimpleOrderedMap();
     String[] entries = UninvertingReader.getUninvertedStats();
     stats.add("entries_count", entries.length);
-    if (!disableEntryList) {
+    if (listEntries) {
       for (int i = 0; i < entries.length; i++) {
         stats.add("entry#" + i, entries[i]);
       }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fd0d6997/solr/core/src/test/org/apache/solr/core/TestJmxMonitoredMap.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/core/TestJmxMonitoredMap.java b/solr/core/src/test/org/apache/solr/core/TestJmxMonitoredMap.java
index e98b790..9f1e2ab 100644
--- a/solr/core/src/test/org/apache/solr/core/TestJmxMonitoredMap.java
+++ b/solr/core/src/test/org/apache/solr/core/TestJmxMonitoredMap.java
@@ -180,4 +180,41 @@ public class TestJmxMonitoredMap extends LuceneTestCase {
 
   }
 
+  @Test
+  public void testJmxAugmentedSolrInfoMBean() throws Exception {
+    final MockInfoMBean mock = new MockInfoMBean();
+    final String jmxKey = "jmx";
+    final String jmxValue = "jmxValue";
+
+    MockJmxAugmentedSolrInfoMBean mbean = new MockJmxAugmentedSolrInfoMBean(mock) {
+      @Override
+      public NamedList getStatisticsForJmx() {
+        NamedList stats = getStatistics();
+        stats.add(jmxKey, jmxValue);
+        return stats;
+      }
+    };
+    monitoredMap.put("mock", mbean);
+
+    // assert getStatistics called when used as a map.  Note can't use equals here to compare
+    // because getStatistics returns a new Object each time.
+    assertNull(monitoredMap.get("mock").getStatistics().get(jmxKey));
+
+    //  assert getStatisticsForJmx called when used as jmx server
+    Set<ObjectInstance> objects = mbeanServer.queryMBeans(null, Query.match(
+        Query.attr("name"), Query.value("mock")));
+    ObjectName name = objects.iterator().next().getObjectName();
+    assertMBeanTypeAndValue(name, jmxKey, jmxValue.getClass(), jmxValue);
+  }
+
+  private static abstract class MockJmxAugmentedSolrInfoMBean
+      extends SolrInfoMBeanWrapper implements JmxMonitoredMap.JmxAugmentedSolrInfoMBean {
+
+    public MockJmxAugmentedSolrInfoMBean(SolrInfoMBean mbean) {
+      super(mbean);
+    }
+
+    @Override
+    public abstract NamedList getStatisticsForJmx();
+  }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/fd0d6997/solr/core/src/test/org/apache/solr/search/TestSolrFieldCacheMBean.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestSolrFieldCacheMBean.java b/solr/core/src/test/org/apache/solr/search/TestSolrFieldCacheMBean.java
index b90430e..5343f73 100644
--- a/solr/core/src/test/org/apache/solr/search/TestSolrFieldCacheMBean.java
+++ b/solr/core/src/test/org/apache/solr/search/TestSolrFieldCacheMBean.java
@@ -42,20 +42,40 @@ public class TestSolrFieldCacheMBean extends SolrTestCaseJ4 {
     assertU(commit());
     assertQ(req("q", "*:*", "sort", "id asc"), "//*[@numFound='1']");
 
-    SolrFieldCacheMBean mbean = new SolrFieldCacheMBean();
-    NamedList stats = mbean.getStatistics();
-    assert(new Integer(stats.get("entries_count").toString()) > 0);
-    assertNotNull(stats.get("entry#0"));
+    // Test with entry list enabled
+    assertEntryListIncluded(false);
 
     // Test again with entry list disabled
     System.setProperty("disableSolrFieldCacheMBeanEntryList", "true");
     try {
-      mbean = new SolrFieldCacheMBean();
-      stats = mbean.getStatistics();
-      assert(new Integer(stats.get("entries_count").toString()) > 0);
-      assertNull(stats.get("entry#0"));
+      assertEntryListNotIncluded(false);
     } finally {
       System.clearProperty("disableSolrFieldCacheMBeanEntryList");
     }
+
+    // Test with entry list enabled for jmx
+    assertEntryListIncluded(true);
+
+    // Test with entry list disabled for jmx
+    System.setProperty("disableSolrFieldCacheMBeanEntryListJmx", "true");
+    try {
+      assertEntryListNotIncluded(true);
+    } finally {
+      System.clearProperty("disableSolrFieldCacheMBeanEntryListJmx");
+    }
+  }
+
+  private void assertEntryListIncluded(boolean checkJmx) {
+    SolrFieldCacheMBean mbean = new SolrFieldCacheMBean();
+    NamedList stats = checkJmx ? mbean.getStatisticsForJmx() : mbean.getStatistics();
+    assert(new Integer(stats.get("entries_count").toString()) > 0);
+    assertNotNull(stats.get("entry#0"));
+  }
+
+  private void assertEntryListNotIncluded(boolean checkJmx) {
+    SolrFieldCacheMBean mbean = new SolrFieldCacheMBean();
+    NamedList stats = checkJmx ? mbean.getStatisticsForJmx() : mbean.getStatistics();
+    assert(new Integer(stats.get("entries_count").toString()) > 0);
+    assertNull(stats.get("entry#0"));
   }
 }