You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by no...@apache.org on 2020/07/05 06:26:31 UTC

[lucene-solr] branch branch_8x updated (35d9bd0 -> 4c3ea27)

This is an automated email from the ASF dual-hosted git repository.

noble pushed a change to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


    from 35d9bd0  SOLR-14021: Deprecate HDFS support
     new 1de187e  SOLR-14610: ReflectMapWriter to use VarHandle instead of old legacy reflection (#1635)
     new ef4056e  Merge branch 'branch_8x' of github.com:apache/lucene-solr into branch_8x
     new 4c3ea27  SOLR-14404: update was not working

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 solr/CHANGES.txt                                   |   2 +
 .../apache/solr/api/CustomContainerPlugins.java    |  31 ++++--
 .../apache/solr/common/util/ReflectMapWriter.java  |  29 +----
 .../java/org/apache/solr/common/util/Utils.java    | 123 ++++++++++++++++-----
 .../solr/common/util/TestSolrJsonWriter.java       | 101 ++++++++++++++++-
 5 files changed, 217 insertions(+), 69 deletions(-)


[lucene-solr] 02/03: Merge branch 'branch_8x' of github.com:apache/lucene-solr into branch_8x

Posted by no...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

noble pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit ef4056e2a493513b1a667313748b39fd177a9543
Merge: 1de187e 35d9bd0
Author: noble <no...@apache.org>
AuthorDate: Sun Jul 5 13:52:44 2020 +1000

    Merge branch 'branch_8x' of github.com:apache/lucene-solr into branch_8x

 lucene/ivy-versions.properties                                |  2 +-
 solr/CHANGES.txt                                              |  5 +++++
 .../src/java/org/apache/solr/core/HdfsDirectoryFactory.java   |  4 ++++
 .../solr/core/backup/repository/HdfsBackupRepository.java     | 11 +++++++++++
 .../src/java/org/apache/solr/store/hdfs/HdfsDirectory.java    |  6 ++++++
 .../src/java/org/apache/solr/store/hdfs/HdfsFileWriter.java   |  2 ++
 .../java/org/apache/solr/store/hdfs/HdfsLocalityReporter.java |  4 ++++
 .../src/java/org/apache/solr/store/hdfs/HdfsLockFactory.java  |  4 ++++
 .../src/java/org/apache/solr/update/HdfsTransactionLog.java   |  2 ++
 solr/core/src/java/org/apache/solr/update/HdfsUpdateLog.java  |  6 +++++-
 .../src/test/org/apache/solr/handler/TestContainerPlugin.java |  2 +-
 solr/licenses/org.restlet-2.4.0.jar.sha1                      |  1 -
 solr/licenses/org.restlet-2.4.3.jar.sha1                      |  1 +
 solr/licenses/org.restlet.ext.servlet-2.4.0.jar.sha1          |  1 -
 solr/licenses/org.restlet.ext.servlet-2.4.3.jar.sha1          |  1 +
 solr/solr-ref-guide/src/running-solr-on-hdfs.adoc             |  2 ++
 solr/solr-ref-guide/src/solr-upgrade-notes.adoc               |  2 ++
 17 files changed, 51 insertions(+), 5 deletions(-)



[lucene-solr] 03/03: SOLR-14404: update was not working

Posted by no...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

noble pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 4c3ea27336a41d31d067768afe5f99624c905fb9
Author: noble <no...@apache.org>
AuthorDate: Sun Jul 5 16:25:09 2020 +1000

    SOLR-14404: update was not working
---
 .../apache/solr/api/CustomContainerPlugins.java    | 31 +++++++++++++++-------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/api/CustomContainerPlugins.java b/solr/core/src/java/org/apache/solr/api/CustomContainerPlugins.java
index fe537d9..6536276 100644
--- a/solr/core/src/java/org/apache/solr/api/CustomContainerPlugins.java
+++ b/solr/core/src/java/org/apache/solr/api/CustomContainerPlugins.java
@@ -130,27 +130,29 @@ public class CustomContainerPlugins implements ClusterPropertiesListener, MapWri
           continue;
         }
         if (e.getValue() == Diff.ADDED) {
+          // this plugin is totally new
           for (ApiHolder holder : apiInfo.holders) {
             containerApiBag.register(holder, getTemplateVars(apiInfo.info));
           }
           currentPlugins.put(e.getKey(), apiInfo);
         } else {
+          //this plugin is being updated
           ApiInfo old = currentPlugins.put(e.getKey(), apiInfo);
-          List<ApiHolder> replaced = new ArrayList<>();
           for (ApiHolder holder : apiInfo.holders) {
-            Api oldApi = containerApiBag.lookup(holder.getPath(),
-                holder.getMethod().toString(), null);
-            if (oldApi instanceof ApiHolder) {
-              replaced.add((ApiHolder) oldApi);
-            }
+            //register all new paths
             containerApiBag.register(holder, getTemplateVars(apiInfo.info));
           }
           if (old != null) {
-            for (ApiHolder holder : old.holders) {
-              if (replaced.contains(holder)) continue;// this path is present in the new one as well. so it already got replaced
-              containerApiBag.unregister(holder.getMethod(),getActualPath(old, holder.getPath()));
+            //this is an update of the plugin. But, it is possible that
+            // some paths are remved in the newer version of the plugin
+            for (ApiHolder oldHolder : old.holders) {
+              if(apiInfo.get(oldHolder.api.getEndPoint()) == null) {
+                //there was a path in the old plugin which is not present in the new one
+                containerApiBag.unregister(oldHolder.getMethod(),getActualPath(old, oldHolder.getPath()));
+              }
             }
             if (old instanceof Closeable) {
+              //close the old instance of the plugin
               closeWhileHandlingException((Closeable) old);
             }
           }
@@ -208,6 +210,17 @@ public class CustomContainerPlugins implements ClusterPropertiesListener, MapWri
     private Class klas;
     Object instance;
 
+    ApiHolder get(EndPoint endPoint) {
+      for (ApiHolder holder : holders) {
+        EndPoint e = holder.api.getEndPoint();
+        if(Objects.equals(endPoint.method()[0] , e.method()[0]) &&
+            Objects.equals(endPoint.path()[0], e.path()[0])) {
+          return holder;
+        }
+      }
+      return null;
+    }
+
 
     @SuppressWarnings({"unchecked","rawtypes"})
     public ApiInfo(PluginMeta info, List<String> errs) {


[lucene-solr] 01/03: SOLR-14610: ReflectMapWriter to use VarHandle instead of old legacy reflection (#1635)

Posted by no...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

noble pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 1de187edb41694f9329ea9a2bbef7d94aa57aa25
Author: Noble Paul <no...@users.noreply.github.com>
AuthorDate: Fri Jul 3 15:25:58 2020 +1000

    SOLR-14610: ReflectMapWriter to use VarHandle instead of old legacy reflection (#1635)
---
 solr/CHANGES.txt                                   |   2 +
 .../apache/solr/common/util/ReflectMapWriter.java  |  29 +----
 .../java/org/apache/solr/common/util/Utils.java    | 123 ++++++++++++++++-----
 .../solr/common/util/TestSolrJsonWriter.java       | 101 ++++++++++++++++-
 4 files changed, 195 insertions(+), 60 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 0c3f9a5..7e90806 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -31,6 +31,8 @@ Other Changes
 
 * SOLR-14592: Upgrade Zookeeper to 3.6.1. NOTE: this required upgrading netty to 4.1.50 (Erick Erickson)
 
+* SOLR-14610: ReflectMapWriter to use VarHandle instead of reflection (noble)
+
 * SOLR-10742: SolrCores.getNamesForCore is quite inefficient and blocks other core operations. 
   NOTE: this experimental method has been removed (Erick Erickson)
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ReflectMapWriter.java b/solr/solrj/src/java/org/apache/solr/common/util/ReflectMapWriter.java
index 1142e62..1390e11 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/ReflectMapWriter.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/ReflectMapWriter.java
@@ -18,42 +18,15 @@
 package org.apache.solr.common.util;
 
 import java.io.IOException;
-import java.lang.reflect.Field;
-import java.lang.reflect.Modifier;
 
 import org.apache.solr.common.MapWriter;
-import org.apache.solr.common.annotation.JsonProperty;
 
 // An implementation of MapWriter which is annotated with Jackson annotations
 public interface ReflectMapWriter extends MapWriter {
 
   @Override
   default void writeMap(EntryWriter ew) throws IOException {
-    for (Field field : this.getClass().getDeclaredFields()) {
-      JsonProperty prop = field.getAnnotation(JsonProperty.class);
-      if (prop == null) continue;
-      int modifiers = field.getModifiers();
-      if (Modifier.isPublic(modifiers) && !Modifier.isStatic(modifiers)) {
-        String fname = prop.value().isEmpty() ? field.getName() : prop.value();
-        try {
-          if (field.getType() == int.class) {
-            ew.put(fname, field.getInt(this));
-          } else if (field.getType() == float.class) {
-            ew.put(fname, field.getFloat(this));
-          } else if (field.getType() == double.class) {
-            ew.put(fname, field.getDouble(this));
-          } else if (field.getType() == boolean.class) {
-            ew.put(fname, field.getBoolean(this));
-          } else if (field.getType() == long.class) {
-            ew.put(fname, field.getLong(this));
-          } else {
-            ew.putIfNotNull(fname, field.get(this));
-          }
-        } catch (IllegalAccessException e) {
-          //it should not happen
-        }
-      }
-    }
+    Utils.reflectWrite(ew, this);
   }
 
 }
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index e86d77e..aa2738b 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -16,34 +16,6 @@
  */
 package org.apache.solr.common.util;
 
-import org.apache.http.HttpEntity;
-import org.apache.http.HttpResponse;
-import org.apache.http.client.HttpClient;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.util.EntityUtils;
-import org.apache.solr.client.solrj.cloud.DistribStateManager;
-import org.apache.solr.client.solrj.cloud.autoscaling.VersionedData;
-import org.apache.solr.client.solrj.impl.BinaryRequestWriter;
-import org.apache.solr.common.IteratorWriter;
-import org.apache.solr.common.LinkedHashMapWriter;
-import org.apache.solr.common.MapWriter;
-import org.apache.solr.common.MapWriterMap;
-import org.apache.solr.common.SolrException;
-import org.apache.solr.common.SpecProvider;
-import org.apache.solr.common.cloud.SolrZkClient;
-import org.apache.solr.common.cloud.ZkOperation;
-import org.apache.solr.common.cloud.ZkStateReader;
-import org.apache.solr.common.params.CommonParams;
-import org.apache.zookeeper.KeeperException;
-import org.apache.zookeeper.server.ByteBufferInputStream;
-import org.noggit.CharArr;
-import org.noggit.JSONParser;
-import org.noggit.JSONWriter;
-import org.noggit.ObjectBuilder;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import org.slf4j.MDC;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -54,6 +26,9 @@ import java.io.StringReader;
 import java.io.UnsupportedEncodingException;
 import java.io.Writer;
 import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
+import java.lang.reflect.Field;
+import java.lang.reflect.Modifier;
 import java.net.URL;
 import java.net.URLDecoder;
 import java.nio.BufferOverflowException;
@@ -76,6 +51,7 @@ import java.util.Set;
 import java.util.TreeMap;
 import java.util.TreeSet;
 import java.util.concurrent.Callable;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.BiConsumer;
@@ -83,6 +59,35 @@ import java.util.function.Function;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import org.apache.http.HttpEntity;
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.util.EntityUtils;
+import org.apache.solr.client.solrj.cloud.DistribStateManager;
+import org.apache.solr.client.solrj.cloud.autoscaling.VersionedData;
+import org.apache.solr.client.solrj.impl.BinaryRequestWriter;
+import org.apache.solr.common.IteratorWriter;
+import org.apache.solr.common.LinkedHashMapWriter;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.MapWriterMap;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.SpecProvider;
+import org.apache.solr.common.annotation.JsonProperty;
+import org.apache.solr.common.cloud.SolrZkClient;
+import org.apache.solr.common.cloud.ZkOperation;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.params.CommonParams;
+import org.apache.zookeeper.KeeperException;
+import org.apache.zookeeper.server.ByteBufferInputStream;
+import org.noggit.CharArr;
+import org.noggit.JSONParser;
+import org.noggit.JSONWriter;
+import org.noggit.ObjectBuilder;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.MDC;
+
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Collections.singletonList;
 import static java.util.Collections.unmodifiableList;
@@ -826,5 +831,67 @@ public class Utils {
     return result;
   }
 
+  public static void reflectWrite(MapWriter.EntryWriter ew, Object o) throws IOException{
+    List<FieldWriter> fieldWriters = null;
+    try {
+      fieldWriters = getReflectData(o.getClass());
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+    for (FieldWriter fieldWriter : fieldWriters) {
+      try {
+        fieldWriter.write(ew, o);
+      } catch (IllegalAccessException e) {
+        //should not happen
+      }
+    }
+  }
+
+  @SuppressWarnings("rawtypes")
+  private static List<FieldWriter> getReflectData(Class c) throws IllegalAccessException {
+    boolean sameClassLoader = c.getClassLoader() == Utils.class.getClassLoader();
+    //we should not cache the class references of objects loaded from packages because they will not get garbage collected
+    //TODO fix that later
+    List<FieldWriter> reflectData = sameClassLoader ? storedReflectData.get(c): null;
+    if(reflectData == null) {
+      ArrayList<FieldWriter> l = new ArrayList<>();
+      MethodHandles.Lookup lookup = MethodHandles.publicLookup();
+      for (Field field : lookup.accessClass(c).getFields()) {
+        JsonProperty prop = field.getAnnotation(JsonProperty.class);
+        if (prop == null) continue;
+        int modifiers = field.getModifiers();
+        if (Modifier.isPublic(modifiers) && !Modifier.isStatic(modifiers)) {
+          String fname = prop.value().isEmpty() ? field.getName() : prop.value();
+          final VarHandle vhandle = lookup.unreflectVarHandle(field);
+          if (field.getType() == int.class) {
+            l.add((ew, inst) -> ew.put(fname, (int) vhandle.get(inst)));
+          } else if (field.getType() == long.class) {
+            l.add((ew, inst) -> ew.put(fname, (long) vhandle.get(inst)));
+          } else if (field.getType() == boolean.class) {
+            l.add((ew, inst) -> ew.put(fname, (boolean) vhandle.get(inst)));
+          } else if (field.getType() == double.class) {
+            l.add((ew, inst) -> ew.put(fname, (double) vhandle.get(inst)));
+          } else if (field.getType() == float.class) {
+            l.add((ew, inst) -> ew.put(fname, (float) vhandle.get(inst)));
+          } else {
+            l.add((ew, inst) -> ew.put(fname, vhandle.get(inst)));
+          }
+        }}
+
+      if(sameClassLoader){
+        storedReflectData.put(c, reflectData = Collections.unmodifiableList(new ArrayList<>(l)));
+      }
+    }
+    return reflectData;
+  }
+
+
+
+  @SuppressWarnings("rawtypes")
+  static Map<Class, List<FieldWriter>> storedReflectData =   new ConcurrentHashMap<>();
+
+  interface FieldWriter {
+    void write(MapWriter.EntryWriter ew, Object inst) throws IllegalAccessException, IOException;
+  }
 
 }
diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java b/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java
index fe2849e..b3d2a53 100644
--- a/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java
+++ b/solr/solrj/src/test/org/apache/solr/common/util/TestSolrJsonWriter.java
@@ -17,18 +17,20 @@
 
 package org.apache.solr.common.util;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.StringWriter;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.IteratorWriter;
 import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.annotation.JsonProperty;
 
 public class TestSolrJsonWriter  extends SolrTestCaseJ4 {
   public void test() throws IOException {
-    StringWriter writer = new StringWriter();
 
     Map<String, Object> map = new HashMap<>();
     map.put("k1","v1");
@@ -42,10 +44,12 @@ public class TestSolrJsonWriter  extends SolrTestCaseJ4 {
           .add("v632"));
     });
 
-    try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer)) {
-      jsonWriter.setIndent(true).writeObj(map);
+    StringWriter writer = new StringWriter();
+    try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer).setIndent(false)) {
+      jsonWriter.writeObj(map);
     }
-    Object o = Utils.fromJSONString(writer.toString());
+    String json = writer.toString();
+    Object o = Utils.fromJSONString(json);
     assertEquals("v1", Utils.getObjectByPath(o, true, "k1"));
     assertEquals(1l, Utils.getObjectByPath(o, true, "k2"));
     assertEquals(Boolean.FALSE, Utils.getObjectByPath(o, true, "k3"));
@@ -55,5 +59,94 @@ public class TestSolrJsonWriter  extends SolrTestCaseJ4 {
     assertEquals("v62", Utils.getObjectByPath(o, true, "k5/k62"));
     assertEquals("v631", Utils.getObjectByPath(o, true, "k5/k63[0]"));
     assertEquals("v632", Utils.getObjectByPath(o, true, "k5/k63[1]"));
+    C1 c1 = new C1();
+
+    int iters = 10000;
+    writer = new StringWriter();
+    try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer).setIndent(false)) {
+      jsonWriter.writeObj(c1);
+    }
+   assertEquals(json, writer.toString());
+
+
+   /*Used in perf testing
+   System.out.println("JSON REFLECT write time : "+write2String(c1,iters));
+    System.out.println("JSON Map write time : "+write2String(map, iters));
+
+    System.out.println("javabin REFLECT write time : "+write2Javabin(c1,iters));
+    System.out.println("javabin Map write time : "+write2Javabin(map, iters));*/
+
+  }
+
+  @SuppressForbidden(reason="used for perf testing numbers only")
+  private long write2String(Object o, int iters) throws IOException {
+    long start = System.currentTimeMillis() ;
+    for
+    (int i = 0;i<iters;i++) {
+      StringWriter writer = new StringWriter();
+
+      try (SolrJSONWriter jsonWriter = new SolrJSONWriter(writer)) {
+        jsonWriter.setIndent(true).writeObj(o);
+      }
+    }
+    return System.currentTimeMillis()-start;
+
+  }
+  @SuppressForbidden(reason="used for perf testing numbers only")
+  private long write2Javabin(Object o, int iters) throws IOException {
+    long start = System.currentTimeMillis() ;
+    for (int i = 0;i<iters;i++) {
+      ByteArrayOutputStream baos = new ByteArrayOutputStream();
+      new JavaBinCodec(null).marshal(o, baos);
+    }
+    return System.currentTimeMillis()-start;
+
+//
+//    JSON REFLECT write time : 107
+//    JSON Map write time : 62
+//    javabin REFLECT write time : 48
+//    javabin Map write time : 34
+//
+//    JSON REFLECT write time : 181
+//    JSON Map write time : 38
+//    javabin REFLECT write time : 111
+//    javabin Map write time : 33
+  }
+
+
+  public static class C1 implements ReflectMapWriter {
+    @JsonProperty
+    public String k1 = "v1";
+    @JsonProperty
+    public int k2 = 1;
+    @JsonProperty
+    public boolean k3 = false;
+    @JsonProperty
+    public C2 k4 = new C2();
+
+    @JsonProperty
+    public C3 k5 = new C3() ;
+
+
+  }
+
+  public static class C3 implements ReflectMapWriter {
+    @JsonProperty
+    public String k61 = "v61";
+
+    @JsonProperty
+    public String k62 = "v62";
+
+    @JsonProperty
+    public List<String> k63= List.of("v631","v632");
+  }
+
+  public static class C2 implements ReflectMapWriter {
+    @JsonProperty
+    public String k41 = "v41";
+
+    @JsonProperty
+    public String k42 = "v42";
+
   }
 }