You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "nick-boss-tech (via GitHub)" <gi...@apache.org> on 2026/10/01 22:17:45 UTC
[PR] SOLR-18317: Treat standalone nodes=all as this node instead of NPEing on ZooKeeper [solr]
nick-boss-tech opened a new pull request, #5001:
URL: https://github.com/apache/solr/pull/5001
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
https://issues.apache.org/jira/browse/SOLR-18317
In standalone (non-ZooKeeper) mode, admin handlers that accept a `nodes` parameter (e.g. the logging UI) hit a `NullPointerException` on the missing `ZkController` instead of handling the request sensibly.
`RemoteRequestProxy` now centralizes the decision in `shouldProxyTo(...)` / `requireZkController(...)`, and `GenericV1RequestProxy` / `V2SolrRequestBasedProxy` delegate to it:
- missing or empty `nodes` → handled locally (unchanged);
- `nodes=all` in standalone mode → treated as this node, no proxying, no NPE;
- any other explicit node value in standalone mode → still returns `true` from `shouldProxy()` so `validateNodeNames` rejects it with a 400 ("requires SolrCloud except for the value 'all'").
Also fixed while validating: the lambda overload of `requestWithBaseUrl` exists only on `HttpJettySolrClient`, so the `HttpSolrClient` downcast it needs was restored (the branch did not compile without it).
Tests: `GenericV1RequestProxyTest` (7) + `V2SolrRequestBasedProxyTest` (7) + `LoggingHandlerTest` (3, incl. new `nodes=all`-in-standalone and explicit-nodes-400 cases) — green with Error Prone; Spotless clean.
Changelog: `changelog/unreleased/SOLR-18317.yml` (fixed)
### AI assistance
AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this 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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Admin UI no longer sends nodes=all in standalone mode [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #5001:
URL: https://github.com/apache/solr/pull/5001#issuecomment-5995624838
Okay, @nick-boss-tech I took a stab at trying to move us to using the V2 Logging APIs, but we don't yet properly handle everything there... So, I was wondering if you would be interested in a follow up to this PR to tackle that work? I'm going to add more detail to https://issues.apache.org/jira/browse/SOLR-15823
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Admin UI no longer sends nodes=all in standalone mode; narrowed server-side handling [solr]
Posted by "nick-boss-tech (via GitHub)" <gi...@apache.org>.
nick-boss-tech commented on PR #5001:
URL: https://github.com/apache/solr/pull/5001#issuecomment-5958833218
Small correction and two notes on the latest push (6a30021):
The metrics wording in my previous comment and in the code was too broad. The metrics decision in this PR is only about standalone mode. In SolrCloud, `node=all` fans out to all live nodes via the shared validator, and that behavior is unchanged. I reworded the comment in `MetricsHandler.createMetricProxy` and the PR description to say exactly that. Whether SolrCloud `node=all` for metrics should also change is a separate question I have not touched here; happy to open it separately if you think it is wrong.
On JS tests for the new UI branch: the legacy AngularJS Admin UI has no unit-test harness in the repo (no karma/jasmine specs under solr/webapp), so there is no existing suite to extend for the cloud/standalone/unresolved branches. The controller change follows the exact `$watch('isCloudEnabled')` pattern already used by paramsets.js and query.js rather than introducing a new mechanism, but I agree the branch is untested at the JS level, and I did not want to bolt a new harness onto this PR to cover one controller.
On verification: the runs I quoted were local. CI has since started on this head: "gradle check", "Run Solr Tests using Crave.io resources" and "Run Admin UI browser tests" are in progress, and the changelog and labeler checks have passed.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Admin UI no longer sends nodes=all in standalone mode; narrowed server-side handling [solr]
Posted by "nick-boss-tech (via GitHub)" <gi...@apache.org>.
nick-boss-tech commented on PR #5001:
URL: https://github.com/apache/solr/pull/5001#issuecomment-5982080400
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
Done: this PR is now narrowed to the Admin UI change only, and the description is updated to match. The server-side proxy changes are preserved on a separate branch for a later PR. Thank you for the suggestion, and for offering to validate the fix.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Admin UI no longer sends nodes=all in standalone mode [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #5001:
URL: https://github.com/apache/solr/pull/5001#issuecomment-5995451789
@nick-boss-tech what branch are you hoping to land this on? 10 or does it need to go all the way back to a future 9x release? I guess I am wondering just a bit if we should just finish migration to the v2 logging api?
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Admin UI no longer sends nodes=all in standalone mode; narrowed server-side handling [solr]
Posted by "nick-boss-tech (via GitHub)" <gi...@apache.org>.
nick-boss-tech commented on PR #5001:
URL: https://github.com/apache/solr/pull/5001#issuecomment-5958632481
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)*
Thanks, Eric. You were right that the caller was the real problem; my first version reached for a shared server-side change that was broader than the bug. I reworked it along the lines you suggested (new commit 4ca4725):
- The Admin UI is now the primary fix. `LoggingLevelController.setLevel` waits for `isCloudEnabled` to settle (the same `$watch` pattern `paramsets.js` uses to decide cores vs collections), sends `nodes: 'all'` only in SolrCloud mode, and omits the parameter entirely in standalone mode. The `Logging` $resource factory no longer hardcodes `nodes:'all'`.
- I kept a much narrower server-side guard, because a direct `/admin/info/logging?nodes=all` call in standalone mode still NPEd on the missing ZooKeeper handle before this PR. The "all means this node" behavior is now an opt-in on the proxy base class, enabled only for the broadcast-style endpoints (logging and system info on v1, node system info on v2). Explicit node names in standalone mode still get a clear 400.
- On your question about how common `nodes=all` is: inside this proxy framework it is only the logging and system-info endpoints on the plural `nodes` parameter, plus metrics on the singular `node`. For metrics I deliberately did not extend the "all means local" meaning. `node` selects a single destination and `all` was never a valid value for it, so in standalone mode it now gets a clean 400 ("Parameter 'node' requires SolrCloud") instead of the old NPE. That decision is explicit in `MetricsHandler.createMetricProxy` and pinned by new tests in `MetricsHandlerTest`.
Tests: the two proxy test classes, `LoggingHandlerTest` (with the original standalone regression), and the new metrics tests all pass (27 tests), and `:solr:core:check -x test` is green.
Happy to drop the server-side guard entirely if you would rather the API stay strict now that the UI no longer sends the parameter in standalone 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: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Admin UI no longer sends nodes=all in standalone mode; narrowed server-side handling [solr]
Posted by "nick-boss-tech (via GitHub)" <gi...@apache.org>.
nick-boss-tech commented on PR #5001:
URL: https://github.com/apache/solr/pull/5001#issuecomment-5958895967
Addendum to my JS-testing note above: the absence of a harness does not make the branch untestable, so I ran the real `controllers/logging.js` in Node with the Angular plumbing stubbed out and exercised `setLevel` in all three mode states. Cloud sends `nodes: 'all'`; standalone omits the parameter entirely; and when the flag is still unresolved no request goes out at all until it settles, after which the request follows the settled mode. 12 assertions, all passing. The harness is ad hoc and uncommitted since the repo has no home for legacy-UI JS tests, but I am happy to include it if there is a preferred place for it.
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Treat standalone nodes=all as this node instead of NPEing on ZooKeeper [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #5001:
URL: https://github.com/apache/solr/pull/5001#issuecomment-5957047077
What if we just fix the Solr Admin UI? Is `nodes=all` a common parameter that lots of places ahve that could potentially be fixed through this approach? There is a lot of mock tests and more checks, and I just wonder if we made the one place that causes this issue to be a bit smarter on the caller?
Currently in the Solr Admin UI we have lots of places where we consult to decide "am I making this url with a core int he name or a collection in the name"... So understnadin that is there. I'd rather just reuse that logic and then not pass `nodes=all` to a standalone, after all, that just doens't make any sense in user-managed (standalone) mode. (gotta love how we use two names for the same thing everywhere).
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Admin UI no longer sends nodes=all in standalone mode; narrowed server-side handling [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #5001:
URL: https://github.com/apache/solr/pull/5001#issuecomment-5980102166
Can we split out the Admin UI fix into a new PR? I can pretty easily test and validate your fix if it's just the Admin UI change... I'm not sure I'm ready to dig into all that proxy stuff ;-).
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org
Re: [PR] SOLR-18317: Admin UI no longer sends nodes=all in standalone mode [solr]
Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh merged PR #5001:
URL: https://github.com/apache/solr/pull/5001
--
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-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org