Skip to content

Commit

Permalink
Revert "SOLR-16512 : Eliminate noggit JSONWriter.Writable from Solr c…
Browse files Browse the repository at this point in the history
…lasses (apache#1159)"

This reverts commit 2dc1108.

SOLR-16530: revrert until perf issues are fixed
  • Loading branch information
noblepaul committed Nov 13, 2022
1 parent 322fa0d commit 7f37115
Show file tree
Hide file tree
Showing 13 changed files with 141 additions and 149 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,23 @@
*/
package org.apache.solr.core.snapshots;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.solr.common.MapWriter;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.params.CoreAdminParams;
import org.apache.solr.common.util.NamedList;
import org.noggit.JSONWriter;

/** This class defines the meta-data about a collection level snapshot */
public class CollectionSnapshotMetaData implements MapWriter {
public static class CoreSnapshotMetaData implements MapWriter {
public class CollectionSnapshotMetaData implements JSONWriter.Writable {
public static class CoreSnapshotMetaData implements JSONWriter.Writable {
private final String coreName;
private final String indexDirPath;
private final long generationNumber;
Expand Down Expand Up @@ -90,13 +90,15 @@ public boolean isLeader() {
}

@Override
public void writeMap(EntryWriter ew) throws IOException {
ew.put(CoreAdminParams.CORE, getCoreName());
ew.put(SolrSnapshotManager.INDEX_DIR_PATH, getIndexDirPath());
ew.put(SolrSnapshotManager.GENERATION_NUM, getGenerationNumber());
ew.put(SolrSnapshotManager.SHARD_ID, getShardId());
ew.put(SolrSnapshotManager.LEADER, isLeader());
ew.put(SolrSnapshotManager.FILE_LIST, getFiles());
public void write(JSONWriter arg0) {
LinkedHashMap<String, Object> info = new LinkedHashMap<>();
info.put(CoreAdminParams.CORE, getCoreName());
info.put(SolrSnapshotManager.INDEX_DIR_PATH, getIndexDirPath());
info.put(SolrSnapshotManager.GENERATION_NUM, getGenerationNumber());
info.put(SolrSnapshotManager.SHARD_ID, getShardId());
info.put(SolrSnapshotManager.LEADER, isLeader());
info.put(SolrSnapshotManager.FILE_LIST, getFiles());
arg0.write(info);
}

public NamedList<Object> toNamedList() {
Expand Down Expand Up @@ -227,11 +229,13 @@ public Collection<String> getShards() {
}

@Override
public void writeMap(EntryWriter ew) throws IOException {
ew.put(CoreAdminParams.NAME, this.name);
ew.put(SolrSnapshotManager.SNAPSHOT_STATUS, this.status.toString());
ew.put(SolrSnapshotManager.CREATION_DATE, this.getCreationDate().getTime());
ew.put(SolrSnapshotManager.SNAPSHOT_REPLICAS, this.replicaSnapshots);
public void write(JSONWriter arg0) {
LinkedHashMap<String, Object> result = new LinkedHashMap<>();
result.put(CoreAdminParams.NAME, this.name);
result.put(SolrSnapshotManager.SNAPSHOT_STATUS, this.status.toString());
result.put(SolrSnapshotManager.CREATION_DATE, this.getCreationDate().getTime());
result.put(SolrSnapshotManager.SNAPSHOT_REPLICAS, this.replicaSnapshots);
arg0.write(result);
}

public NamedList<Object> toNamedList() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
import java.util.Map;
import java.util.Set;
import org.apache.solr.common.AlreadyClosedException;
import org.apache.solr.common.MapWriter;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
Expand All @@ -35,6 +34,7 @@
import org.apache.solr.common.cloud.ZooKeeperException;
import org.apache.solr.common.util.Utils;
import org.apache.zookeeper.KeeperException;
import org.noggit.JSONWriter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -77,7 +77,7 @@ public ZkClientClusterStateProvider(String zkHost) {
*
* @param bytes a byte array of a Json representation of a mapping from collection name to the
* Json representation of a {@link DocCollection} as written by {@link
* ClusterState#writeMap(MapWriter.EntryWriter)}. It can represent one or more collections.
* ClusterState#write(JSONWriter)}. It can represent one or more collections.
* @param liveNodes list of live nodes
* @param coll collection name
* @param zkClient ZK client
Expand Down
3 changes: 0 additions & 3 deletions solr/solrj/src/java/org/apache/solr/common/MapWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,5 @@ default BiConsumer<CharSequence, Object> getBiConsumer() {
}
}

/** A marker interface to denote that this is to be serialized using {@link Object#toString()} */
interface StringValue {}

MapWriter EMPTY = new MapWriterMap(Collections.emptyMap());
}
27 changes: 14 additions & 13 deletions solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import static org.apache.solr.common.util.Utils.STANDARDOBJBUILDER;

import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.Collection;
import java.util.Collections;
Expand All @@ -34,13 +33,13 @@
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import org.apache.solr.common.MapWriter;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.DocCollection.CollectionStateProps;
import org.apache.solr.common.cloud.Replica.ReplicaStateProps;
import org.apache.solr.common.util.Utils;
import org.noggit.JSONParser;
import org.noggit.JSONWriter;
import org.noggit.ObjectBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -51,7 +50,7 @@
*
* @lucene.experimental
*/
public class ClusterState implements MapWriter {
public class ClusterState implements JSONWriter.Writable {

/** Cluster Prop that is http or https. */
public static final String URL_SCHEME = "urlScheme";
Expand Down Expand Up @@ -223,8 +222,8 @@ public String toString() {
* thus don't call it where that's important
*
* @param bytes a byte array of a Json representation of a mapping from collection name to the
* Json representation of a {@link DocCollection} as written by {@link
* #writeMap(EntryWriter)}. It can represent one or more collections.
* Json representation of a {@link DocCollection} as written by {@link #write(JSONWriter)}. It
* can represent one or more collections.
* @param liveNodes list of live nodes
* @return the ClusterState
*/
Expand Down Expand Up @@ -296,8 +295,15 @@ private static DocCollection collectionFromObjects(
}

@Override
public void writeMap(EntryWriter ew) throws IOException {
collectionStates.forEach(ew.getBiConsumer());
public void write(JSONWriter jsonWriter) {
LinkedHashMap<String, DocCollection> map = new LinkedHashMap<>();
for (Entry<String, CollectionRef> e : collectionStates.entrySet()) {
if (e.getValue().getClass() == CollectionRef.class) {
DocCollection coll = e.getValue().get();
map.put(coll.getName(), coll);
}
}
jsonWriter.write(map);
}

@Override
Expand Down Expand Up @@ -370,7 +376,7 @@ public void forEachCollection(Consumer<DocCollection> consumer) {
});
}

public static class CollectionRef implements MapWriter {
public static class CollectionRef {
protected final AtomicInteger gets = new AtomicInteger();
private final DocCollection coll;

Expand Down Expand Up @@ -406,11 +412,6 @@ public boolean isLazilyLoaded() {
return false;
}

@Override
public void writeMap(EntryWriter ew) throws IOException {
get().writeMap(ew);
}

@Override
public String toString() {
if (coll != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,21 @@

import static org.apache.solr.common.util.Utils.toJSONString;

import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.function.BiConsumer;
import java.util.function.BiPredicate;
import org.apache.solr.common.cloud.Replica.ReplicaStateProps;
import org.noggit.JSONWriter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -323,9 +324,11 @@ public String toString() {
}

@Override
public void writeMap(EntryWriter ew) throws IOException {
propMap.forEach(ew.getBiConsumer());
ew.put(CollectionStateProps.SHARDS, slices);
public void write(JSONWriter jsonWriter) {
LinkedHashMap<String, Object> all = new LinkedHashMap<>(slices.size() + 1);
all.putAll(propMap);
all.put(CollectionStateProps.SHARDS, slices);
jsonWriter.write(all);
}

public Replica getReplica(String coreNodeName) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@
import java.util.List;
import java.util.Map;
import org.apache.solr.cluster.api.HashRange;
import org.apache.solr.common.MapWriter;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrInputDocument;
import org.apache.solr.common.cloud.DocCollection.CollectionStateProps;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.StrUtils;
import org.noggit.JSONWriter;

/**
* Class to partition int range into n ranges.
Expand Down Expand Up @@ -89,7 +89,7 @@ public static Map<String, Object> getRouterSpec(ZkNodeProps props) {
// Hash ranges can't currently "wrap" - i.e. max must be greater or equal to min.
// TODO: ranges may not be all contiguous in the future (either that or we will
// need an extra class to model a collection of ranges)
public static class Range implements MapWriter.StringValue, Comparable<Range>, HashRange {
public static class Range implements JSONWriter.Writable, Comparable<Range>, HashRange {
public int min; // inclusive
public int max; // inclusive

Expand Down Expand Up @@ -141,6 +141,11 @@ public boolean equals(Object obj) {
return this.min == other.min && this.max == other.max;
}

@Override
public void write(JSONWriter writer) {
writer.write(toString());
}

@Override
public int compareTo(Range that) {
int mincomp = Integer.compare(this.min, that.min);
Expand Down
22 changes: 17 additions & 5 deletions solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,12 @@
package org.apache.solr.common.cloud;

import static org.apache.solr.common.ConditionalMapWriter.NON_NULL_VAL;
import static org.apache.solr.common.ConditionalMapWriter.dedupeKeyPredicate;

import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Locale;
import java.util.Map;
Expand All @@ -29,6 +31,7 @@
import java.util.function.BiPredicate;
import org.apache.solr.common.MapWriter;
import org.apache.solr.common.util.Utils;
import org.noggit.JSONWriter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -366,7 +369,7 @@ public Object clone() {

@Override
public void writeMap(MapWriter.EntryWriter ew) throws IOException {
_allPropsWriter().writeMap(ew);
ew.put(name, _allPropsWriter());
}

private static final Map<String, State> STATES = new HashMap<>();
Expand All @@ -383,15 +386,16 @@ public static State getState(String shortName) {
}

private MapWriter _allPropsWriter() {
BiPredicate<CharSequence, Object> p =
((BiPredicate<CharSequence, Object>) (k, o) -> !propMap.containsKey(k.toString()))
.and(NON_NULL_VAL);
BiPredicate<CharSequence, Object> p = dedupeKeyPredicate(new HashSet<>()).and(NON_NULL_VAL);
return writer -> {
// XXX this is why this class should be immutable - it's a mess !!!

// propMap takes precedence because it's mutable and we can't control its
// contents, so a third party may override some declared fields
propMap.forEach(writer.getBiConsumer());
for (Map.Entry<String, Object> e : propMap.entrySet()) {
writer.put(e.getKey(), e.getValue(), p);
}

writer
.put(ReplicaStateProps.CORE_NAME, core, p)
.put(ReplicaStateProps.SHARD_ID, shard, p)
Expand All @@ -402,6 +406,14 @@ private MapWriter _allPropsWriter() {
};
}

@Override
public void write(JSONWriter jsonWriter) {
Map<String, Object> map = new LinkedHashMap<>();
// this serializes also our declared properties
_allPropsWriter().toMap(map);
jsonWriter.write(map);
}

@Override
public String toString() {
return name
Expand Down
6 changes: 6 additions & 0 deletions solr/solrj/src/java/org/apache/solr/common/cloud/Slice.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.apache.solr.common.cloud.Replica.Type;
import org.noggit.JSONWriter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -306,6 +307,11 @@ public String toString() {
return name + ':' + toJSONString(propMap);
}

@Override
public void write(JSONWriter jsonWriter) {
jsonWriter.write(propMap);
}

/** JSON properties related to a slice's state. */
public interface SliceStateProps {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,10 @@
import org.apache.solr.common.MapWriter;
import org.apache.solr.common.util.JavaBinCodec;
import org.apache.solr.common.util.Utils;
import org.noggit.JSONWriter;

/** ZkNodeProps contains generic immutable properties. */
public class ZkNodeProps implements MapWriter {
public class ZkNodeProps implements JSONWriter.Writable, MapWriter {

protected final Map<String, Object> propMap;

Expand Down Expand Up @@ -104,6 +105,11 @@ public static ZkNodeProps load(byte[] bytes) {
return new ZkNodeProps(props);
}

@Override
public void write(JSONWriter jsonWriter) {
jsonWriter.write(propMap);
}

/** Get a string property value. */
public String getStr(String key) {
return getStr(key, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1102,9 +1102,6 @@ public boolean writePrimitive(Object val) throws IOException {
} else if (val instanceof LongAccumulator) {
daos.writeLong(((LongAccumulator) val).longValue());
return true;
} else if (val instanceof MapWriter.StringValue) {
writeStr(val.toString());
return true;
}
return false;
}
Expand Down
Loading

0 comments on commit 7f37115

Please sign in to comment.