kotman12 commented on code in PR #2382: URL: https://github.com/apache/solr/pull/2382#discussion_r1602065616
########## solr/modules/monitor/src/java/org/apache/lucene/monitor/MonitorFields.java: ########## @@ -0,0 +1,38 @@ +/* + * + * * 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.lucene.monitor; + +import java.util.Set; + +public class MonitorFields { + + public static final String QUERY_ID = QueryIndex.FIELDS.query_id + "_"; + public static final String CACHE_ID = QueryIndex.FIELDS.cache_id + "_"; + public static final String MONITOR_QUERY = QueryIndex.FIELDS.mq + "_"; + public static final String PAYLOAD = QueryIndex.FIELDS.mq + "_payload_"; Review Comment: So echoing one of your earlier points about limiting the scope of the initial phase, payload is one of those things that would be on my cutting block. Initially I tried to have something that would effectively equate to the [optional lucene-monitor metadata map](https://github.com/apache/lucene/blob/main/lucene/monitor/src/java/org/apache/lucene/monitor/MonitorQuery.java#L43). I believe the point of this is to return back that metadata in case there is something useful that you want to associate with an indexed query. Functionally, it serves the same purpose to a stored/doc-values solr field but in a much less flexible and less normalized way. What's funny is that I haven't actually wired this into the response yet because I don't think any of the default lucene-monitor responses pass it back and I was simply going for feature parity. That aside, is there a particular reason you were exploring making the payload field name overridable? Maybe to let users give it a more self-descri bing name for their particular use case? The difference between a query decomposer and a decoder is that the decomposer takes an in-memory `Query` object and splits it into `List<Query>` where each element is a presumably smaller disjunct that is less expensive to match in the final phase (think for example of an or-expression with 500 terms .. each one of those can be indexed/matched separately for a much faster matching phase). A query decoder simply takes the original full query string and parses it to the original full/undecomposed `Query`. After that you have to split it into the disjuncts again as [there is no _easy_ and _general_ way](https://stackoverflow.com/a/16966866) to serialize those inner `Query`s. That detail aside, I don't see any reason the decoder can't use the decomposer directly and expose that decomposition as part of its API ... this is more of me explaining this distinction between lucene's original `MonitorQuerySerializer` and `QueryDecomposer`. -- 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. To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org For additional commands, e-mail: issues-h...@solr.apache.org
