dsmiley commented on code in PR #4890:
URL: https://github.com/apache/solr/pull/4890#discussion_r3963573897
##########
solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java:
##########
@@ -229,73 +232,72 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
protected void handlePing(SolrQueryRequest req, SolrQueryResponse rsp)
throws Exception {
- SolrParams params = req.getParams();
SolrCore core = req.getCore();
- // Get the RequestHandler
- String qt = params.get(CommonParams.QT); // optional; you get the default
otherwise
+ SolrParams configParams = resolveConfiguredParams(req);
+ String qt = configParams.get(CommonParams.QT);
SolrRequestHandler handler = core.getRequestHandler(qt);
if (handler == null) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Unknown RequestHandler (qt): "
+ qt);
}
-
if (handler instanceof PingRequestHandler) {
- // In case it's a query for shard, use default handler
- if (params.getBool(ShardParams.IS_SHARD, false)) {
- handler = core.getRequestHandler(null);
- ModifiableSolrParams wparams = new ModifiableSolrParams(params);
- wparams.remove(CommonParams.QT);
- req.setParams(wparams);
- } else {
- throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST,
- "Cannot execute the PingRequestHandler recursively");
- }
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Cannot execute the
PingRequestHandler recursively");
+ }
+
+ ModifiableSolrParams overrides = new ModifiableSolrParams();
+ boolean distrib = req.getParams().getBool(DISTRIB, false);
+ overrides.set(DISTRIB, distrib);
+ if (distrib) {
+ // target the delegate on each shard, not this ping handler
+ overrides.set(ShardParams.SHARDS_QT, qt == null ? "/select" : qt);
}
// Execute the ping query and catch any possible exception
Throwable ex = null;
-
- // In case it's a query for shard, return the result from delegated
handler for distributed
- // query to merge result
- if (params.getBool(ShardParams.IS_SHARD, false)) {
- try {
- core.execute(handler, req, rsp);
- ex = rsp.getException();
- } catch (Exception e) {
- ex = e;
+ SolrQueryRequest pingReq =
req.subRequest(SolrParams.wrapDefaults(overrides, configParams));
+ try {
+ SolrQueryResponse pingrsp = new SolrQueryResponse();
+ core.execute(handler, pingReq, pingrsp);
+ ex = pingrsp.getException();
+ NamedList<Object> headers = rsp.getResponseHeader();
+ if (headers != null) {
+ headers.add("zkConnected",
pingrsp.getResponseHeader().get("zkConnected"));
}
- // Send an error or return
- if (ex != null) {
- throw new SolrException(
- SolrException.ErrorCode.SERVER_ERROR,
- "Ping query caused exception: " + ex.getMessage(),
- ex);
- }
- } else {
- try {
- SolrQueryResponse pingrsp = new SolrQueryResponse();
- core.execute(handler, req, pingrsp);
- ex = pingrsp.getException();
- NamedList<Object> headers = rsp.getResponseHeader();
- if (headers != null) {
- headers.add("zkConnected",
pingrsp.getResponseHeader().get("zkConnected"));
- }
+ } catch (Exception e) {
+ ex = e;
+ } finally {
+ pingReq.close();
+ }
- } catch (Exception e) {
- ex = e;
- }
+ // Send an error or an 'OK' message (response code will be 200)
+ if (ex != null) {
+ throw new SolrException(
+ SolrException.ErrorCode.SERVER_ERROR,
+ "Ping query caused exception: " + ex.getMessage(),
+ ex);
+ }
- // Send an error or an 'OK' message (response code will be 200)
- if (ex != null) {
- throw new SolrException(
- SolrException.ErrorCode.SERVER_ERROR,
- "Ping query caused exception: " + ex.getMessage(),
- ex);
- }
+ rsp.add("status", "OK");
+ }
- rsp.add("status", "OK");
+ /**
+ * Resolves this handler's configured invariants, appends, defaults and
{@code useParams}
+ * paramsets into a single {@link SolrParams}. The delegate handler is named
by {@code qt}; a null
+ * {@code qt} means the core's default handler.
+ */
+ private SolrParams resolveConfiguredParams(SolrQueryRequest req) {
+ SolrQueryRequest configOnly = req.subRequest(new ModifiableSolrParams());
+ try {
Review Comment:
not doing try-with-resources?
##########
solr/core/src/java/org/apache/solr/handler/PingRequestHandler.java:
##########
@@ -229,73 +232,72 @@ public void handleRequestBody(SolrQueryRequest req,
SolrQueryResponse rsp) throw
protected void handlePing(SolrQueryRequest req, SolrQueryResponse rsp)
throws Exception {
- SolrParams params = req.getParams();
SolrCore core = req.getCore();
- // Get the RequestHandler
- String qt = params.get(CommonParams.QT); // optional; you get the default
otherwise
+ SolrParams configParams = resolveConfiguredParams(req);
+ String qt = configParams.get(CommonParams.QT);
SolrRequestHandler handler = core.getRequestHandler(qt);
if (handler == null) {
throw new SolrException(
SolrException.ErrorCode.BAD_REQUEST, "Unknown RequestHandler (qt): "
+ qt);
}
-
if (handler instanceof PingRequestHandler) {
- // In case it's a query for shard, use default handler
- if (params.getBool(ShardParams.IS_SHARD, false)) {
- handler = core.getRequestHandler(null);
- ModifiableSolrParams wparams = new ModifiableSolrParams(params);
- wparams.remove(CommonParams.QT);
- req.setParams(wparams);
- } else {
- throw new SolrException(
- SolrException.ErrorCode.BAD_REQUEST,
- "Cannot execute the PingRequestHandler recursively");
- }
+ throw new SolrException(
+ SolrException.ErrorCode.BAD_REQUEST, "Cannot execute the
PingRequestHandler recursively");
+ }
+
+ ModifiableSolrParams overrides = new ModifiableSolrParams();
+ boolean distrib = req.getParams().getBool(DISTRIB, false);
+ overrides.set(DISTRIB, distrib);
+ if (distrib) {
+ // target the delegate on each shard, not this ping handler
+ overrides.set(ShardParams.SHARDS_QT, qt == null ? "/select" : qt);
}
// Execute the ping query and catch any possible exception
Throwable ex = null;
-
- // In case it's a query for shard, return the result from delegated
handler for distributed
- // query to merge result
- if (params.getBool(ShardParams.IS_SHARD, false)) {
- try {
- core.execute(handler, req, rsp);
- ex = rsp.getException();
- } catch (Exception e) {
- ex = e;
+ SolrQueryRequest pingReq =
req.subRequest(SolrParams.wrapDefaults(overrides, configParams));
+ try {
Review Comment:
lets do try-with-resources while you're editing this line
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
