Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
github-actions[bot] closed pull request #2672: Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode URL: https://github.com/apache/solr/pull/2672 -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
github-actions[bot] commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-3198763770 This PR is now closed due to 60 days of inactivity after being marked as stale. Re-opening this PR is still possible, in which case it will be marked as active again. -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
github-actions[bot] commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2989437162 This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution! -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
CommRogue commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2816655194 Any update on this? @patsonluk It would be very nice to see this merged before 10. Also, there still seems to be two PRs opened for this - this one and #2527. -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
github-actions[bot] commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2676457403 This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution! -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
mkhludnev commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2561400684 Doesn't it make sense to add a test confirming this functionality? -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
patsonluk commented on PR #2672:
URL: https://github.com/apache/solr/pull/2672#issuecomment-2561326911
Sorry about the delay! I was on vacation and somehow I missed this thread!
To be honest I'm not totally sure why it returns null for non-select paths:
```
if (!path.endsWith("/select")) return null;
```
My guess is that the design is to avoid all non query operations on
Coordinator node as those might not be supported (so to avoid any surprises).
Perhaps @noblepaul could shed some light here? ✨
As for the 2 others changes, they LGTM! 😊
- adding an if to catch cases i which the collection is null
- adding a node tracking property that would appear in debugQuery mode
--
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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
dsmiley commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2551551068 I'm looking at #2527 and #2672 (this one) which seem the same at a glance; why do we have two PRs? It's really sad this didn't get merged before 9.8. Contributors sometime really need to advocate / push for their changes or they get forgotten if no committer is interested. @patsonluk maybe you could take a look at this PR as you have worked on the Coordinator node and run it production (I assume) Another vote of confidence to this PR (actually #2527 if that matters) is that a colleague took it today in order to try out coordinator node. -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
github-actions[bot] commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2481686802 This PR has had no activity for 60 days and is now labeled as stale. Any new activity will remove the stale label. To attract more reviewers, please tag people who might be familiar with the code area and/or notify the [email protected] mailing list. To exempt this PR from being marked as stale, make it a draft PR or add the label "exempt-stale". If left unattended, this PR will be closed after another 60 days of inactivity. Thank you for your contribution! -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
mkhludnev commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2357617808 I think it's fine to add CHANGES row and push. -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
cpoerschke commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2355677750 Thanks @ellaeln for iterating on this PR, the CI is now green, yay! w.r.t. the remainder of the review(s) here -- unfortunately I'm not familiar enough with the area of the code to help review that, but hopefully others will be. (Solr 9.7 was released earlier this month, so my _guesstimate_ would be that maybe in November or December there might be a Solr 9.8 release.) So if (perhaps) there is no further review input here before then, my suggestion would be to reach out on the dev list in a couple of weeks i.e. early October. Hope that helps. -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
ellaeln commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2353975200 > Hello. Next steps here could be to: > > 1. review and address the 'gradle precommit' failure above > 2. review the suggestions (and apply them if they are agreeable) I investigated the Gradle precommit failure and found that the issue was due to a missing empty line at the end of the file. I’ve added the line and tested it locally, so it should be resolved now. Also I viewed your suggestions and added them to the PR, I thank you for raising them -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
cpoerschke commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2337975951 Hello. Next steps here could be to: 1. review and address the 'gradle precommit' failure above 2. review the suggestions (and apply them if they are agreeable) -- 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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
cpoerschke commented on code in PR #2672:
URL: https://github.com/apache/solr/pull/2672#discussion_r1746719633
##
solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java:
##
@@ -67,12 +73,33 @@ public CoordinatorHttpSolrCall(
this.factory = factory;
}
+ @SuppressWarnings("unchecked")
+ @Override
+ protected void writeResponse(
+ SolrQueryResponse solrRsp, QueryResponseWriter responseWriter, Method
reqMethod)
+ throws IOException {
+
+// make coordinator mode explicit
+if (solrRsp.getValues().get(CommonParams.DEBUG) != null) {
+ final NamedList debug =
+ (NamedList) solrRsp.getValues().get(CommonParams.DEBUG);
Review Comment:
Maybe have local variable to avoid double `get` calls e.g.
```suggestion
final Object debugObj = solrRsp.getValues().get(CommonParams.DEBUG);
if (debugObj != null) {
final NamedList debug =
(NamedList) debugObj;
```
--
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]
Re: [PR] Feature/solr-17334 Minor bugs in Solr dedicated coordinator mode [solr]
ellaeln commented on PR #2672: URL: https://github.com/apache/solr/pull/2672#issuecomment-2312359080 opened a new MR for solr/17334 to expedite merging these changes @cpoerschke please let me know if there's anything a can do to assist moving forward -- 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]
