SOLR-17600 Replaces MapSerializable with MapWriter #4011
Conversation
…across Solr codebase
…` across Solr codebase
…serialization consistency
…proved map serialization consistency
…d serialization consistency
…Map` for consistency in plugin and handler serialization
…plugin serialization consistency
…tter readability and consistency
…deredMap` for consistent serialization in config and handler classes
…plugin and component info serialization with `SimpleOrderedMap`
| Map deepCopy = Utils.getDeepCopy(data, 3); | ||
| Map p = (Map) deepCopy.get(NAME); | ||
| if (p == null) deepCopy.put(NAME, p = new LinkedHashMap<>()); | ||
| if (paramSet == null) p.remove(name); |
There was a problem hiding this comment.
I wonder if this if should have { } to match the else?
There was a problem hiding this comment.
I agree that in Java conditionals should always use curly braces for clarity. This is the existing code that was not changed in my PR.
I am hesitant to change it as I am not sure if there is a convention regarding this practice in the project.
| "cacheControl", | ||
| cacheControlHeader); | ||
| public void writeMap(EntryWriter ew) throws IOException { | ||
| ew.put("never304", never304) |
There was a problem hiding this comment.
this is nicer pattern, less a "unique magic pattern".
|
|
||
| public static CreateCoreParams createRequestBodyFromV1Params(SolrParams solrParams) { | ||
| final var v1ParamMap = solrParams.toMap(new HashMap<>()); | ||
| final var v1ParamMap = new SimpleOrderedMap<>(solrParams); |
There was a problem hiding this comment.
There is a subtle semantic difference here. Previously, we had a bonafide Map. If there were duplicate keys, the last value wins (looking at Utils.convertToMap, called by toMap). Probably wasn't intended but there is integrity of the Map contract at least. The constructor you use assumes the input has no duplicate keys. They will be added, and SimpleOrderedMap will not quite act exactly like a Map. I'm not sure what the final effect is without trying/testing. We could change SimpleOrderedMap's constructor here to have a hack for the NamedList (that isn't SimpleOrderedMap) case. Or don't use SimpleOrderedMap here, which I can see was chosen for your convenience, not because it's needed.
There was a problem hiding this comment.
Replaced with Utils.convertToMap.
…CreateCore` for parameter handling simplification
3 participants
https://issues.apache.org/jira/browse/SOLR-17600
Description
This pull request replaces the deprecated
MapSerializableinterface withMapWriteracross the Solr codebase.MapSerializablerelied on thetoMap(Map)pattern, which often led to unnecessary object allocations and inconsistent serialization behavior. By migrating toMapWriterand itswriteMap(EntryWriter)pattern, we improve memory efficiency and ensure more consistent serialization across different formats like JavaBin and JSON.Solution
The migration involved several key steps:
MapSerializableinterface and removed its usage from theMapWriterinterface.MapWriternow serves as the primary interface for object-to-map serialization.MapWriterinstead ofMapSerializable. Key areas include:SolrConfig,PluginInfo,SolrIndexConfig,CacheConfig,ConfigOverlay,RequestParams,IndexSchema.CreateCore,InstallShardData,MigrateDocsAPI,ModifyCollectionAPI,MoveReplicaAPI,SplitShardAPI, and more.ZkNodeProps,IndexFingerprint.toMap(Map<String, Object> map)implementations towriteMap(EntryWriter ew). This allows for direct writing to the underlying data structure (likeJavaBinCodec's output stream) without necessarily creating intermediateMapobjects.Utils.convertToMap,Utils.reflectToMap, and other utility methods to support the new pattern.Utils.makeDeepCopynow usesSimpleOrderedMapwithMapWriterfor better efficiency.toMapcalls and simplified serialization logic in several handlers and components. UpdatedJavaBinCodec,TextWriter, andJSONWriterto optimize forMapWriter.Tests
Existing tests were updated to accommodate the change in serialization:
TestConfig,SolrIndexConfigTest,CacheConfigTest, andNodeConfigClusterPluginsSourceTestto verify that the configuration objects are still correctly serialized.TestSchemaDesignerAPIandSolrExampleCborTestto ensure API and serialization consistency.JavaBinCodecandJSONWritercorrectly handle the newMapWriterimplementations.Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.