dsmiley commented on a change in pull request #1574:
URL: https://github.com/apache/lucene-solr/pull/1574#discussion_r448702273
##########
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##########
@@ -500,6 +508,31 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
}
+ private void tagRequestWithRequestId(ResponseBuilder rb) {
+ final boolean ridTaggingDisabled =
rb.req.getParams().getBool(CommonParams.DISABLE_REQUEST_ID, false);
+ if (! ridTaggingDisabled) {
+ String rid = getRequestId(rb.req);
+ if
(StringUtils.isBlank(rb.req.getParams().get(CommonParams.REQUEST_ID))) {
+ ModifiableSolrParams params = new
ModifiableSolrParams(rb.req.getParams());
+ params.add(CommonParams.REQUEST_ID, rid);//add rid to the request so
that shards see it
+ rb.req.setParams(params);
+ }
+ if (rb.isDistrib) {
+ rb.rsp.addToLog(CommonParams.REQUEST_ID, rid); //to see it in the logs
of the landing core
+ }
+ }
+ }
+
+ public static String getRequestId(SolrQueryRequest req) {
Review comment:
The fact that this method could generate a RID each time it's called on
the same request seems problematic. Instead, either don't have it do any such
generation (I think my preference), or have it both generate one and save it
into the the params so that the same request ID is returned given the same
request.
Also; let's document our public methods on an important class like
SearchHandler.
##########
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##########
@@ -500,6 +508,31 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
}
}
+ private void tagRequestWithRequestId(ResponseBuilder rb) {
+ final boolean ridTaggingDisabled =
rb.req.getParams().getBool(CommonParams.DISABLE_REQUEST_ID, false);
+ if (! ridTaggingDisabled) {
+ String rid = getRequestId(rb.req);
+ if
(StringUtils.isBlank(rb.req.getParams().get(CommonParams.REQUEST_ID))) {
+ ModifiableSolrParams params = new
ModifiableSolrParams(rb.req.getParams());
Review comment:
Maybe we don't need a flag parameter to decide what to do with rid.
What if Solr sees rid=\* (or true?) then it knows to generate a rid, and if it
sees a blank/empty string (or false?) then it does nothing. If it's something
else than it *is* the rid (either from the client or generated). One parameter
to look at for this feature seems nicer than two. Also, relating to my concern
about wholesale disabling, it could be set in the solrconfig as a param default.
##########
File path:
solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java
##########
@@ -298,7 +305,8 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
final RTimerTree timer = rb.isDebug() ? req.getRequestTimer() : null;
final ShardHandler shardHandler1 = getAndPrepShardHandler(req, rb); //
creates a ShardHandler object only if it's needed
-
+
+ tagRequestWithRequestId(rb);
Review comment:
super minor but please add a blank line after this because it's not
related to timer.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
