Repository navigation
SOLR-15823: Broadcast V2 logging level changes to named nodes and migrate the Admin UI to V2 - #5030
Open
nick-boss-tech wants to merge 5 commits into
Open
nick-boss-tech wants to merge 5 commits into
nick-boss-tech wants to merge 5 commits into
Conversation
PUT /api/node/logging/levels now accepts a 'nodes' query parameter. When present, NodeLogging fans the request out to the named nodes (or every live node for nodes=all) through V2SolrRequestBasedProxy, the same machinery GetNodeSystemInfo uses, and the response carries one result per node plus the names of any requested nodes that did not respond. Unknown node names are rejected before anything is sent, and using 'nodes' outside SolrCloud mode is a 400 rather than an NPE. The V1 handler now calls the endpoint method with a null nodes value so it does not broadcast twice.
…oint LoggingLevelController.setLevel now calls LoggingV2.modifyLocalLogLevel unconditionally, passing nodes=all only in SolrCloud mode, and the V1 Logging factory is deleted from services.js.
NodeLoggingAPITest gains a broadcast case where one node's proxied request fails: the response must carry a result for the node that answered and name the other in failedNodes. The cloud broadcast test now also checks the JVM-wide level afterwards, showing the payload was applied rather than only delivered, and restores the logger to unset when it finishes.
…y have applied the change The response can time out after a node accepted the set-level request, so a node named in failedNodes is not guaranteed to be unchanged. The configuring-logging page now says so next to the failedNodes description.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 AI text below 🤖 (posted on behalf of Nick Shanin)
https://issues.apache.org/jira/browse/SOLR-15823
What happens today
The V2 logging endpoint
PUT /api/node/logging/levelsonly ever applies level changes on the node that receives the request (NodeLogging.java:87-99);NodeLoggingcarries a TODO to add anodesparameter once SOLR-16738 lands (NodeLogging.java:47), and that proxy machinery has since landed (the node system info endpoint uses it (GetNodeSystemInfo.java:70-83)). Because of the gap, the Admin UI logging screen still sets levels through the V1 handler in SolrCloud mode (logging.js:164-175), since only V1 understandsnodes=all.The V1 handler has a second route into the same code: it applies level changes whenever a request carries
setparameters (LoggingHandler.java:75-79), including on GET requests. Eric Pugh noted on the ticket (2026-10-05) that he stumbled over this while testing: he went looking for his POST or PUT in the browser's network log and realized the GET was changing the log level. This PR does not change that behavior; every GET works exactly as it does today.What this change does
The set-level endpoint accepts a
nodesquery parameter (NodeLoggingApis.java:41-51). When it is present,NodeLoggingfans the request out to the named nodes (or every live node fornodes=all) throughV2SolrRequestBasedProxy(NodeLogging.java:123-133), mirroringGetNodeSystemInfo(GetNodeSystemInfo.java:70-83), and the response carries one result per node, keyed by node name, plus afailedNodeslist naming any requested nodes that did not respond (LoggingResponse.java:31-56). A named node that is not part of the cluster is rejected with a 400 before anything is sent (NodeLogging.java:134-145), so an unknown name cannot half-apply the change. A node that leaves the live set while a broadcast is in flight is a narrower case the pre-check cannot cover; it is in Limits. Usingnodeson a standalone node is also a 400, not an NPE (NodeLogging.java:118-121). Whennodesis absent the local-only path is unchanged (NodeLogging.java:100-108), and, again mirroring system info, the receiving node does not also apply the change locally whennodesis present;nodes=allcovers it because the resolved set includes it (NodeLogging.java:111-114). The Admin UI logging screen now calls the V2 endpoint in both modes (nodes: "all"only in cloud mode) (logging.js:163-174), and the V1Loggingfactory is deleted (commit a76e30e48db). The reference guide's logging page documents the parameter (configuring-logging.adoc:105-116).Proof
Verified 2026-10-05 at head 85d8df8 (base e432df1). The top commit adds test coverage only, on top of 100ad2e; no production or UI code changed in it.
NodeLoggingNodesSolrCloudTest(new, 2-node cluster) (NodeLoggingNodesSolrCloudTest.java): 4/4 with this change. Against base production code the same tests fail 3 of 4 for the stated reason: no broadcast happens, the response covers only the receiving node, and an unknown node name is silently treated as a local request. The fourth test, which pins the no-nodeslocal response shape, passes on base too. Assertions run against the response body, since the test cluster's nodes share one JVM and log levels are JVM-wide, so level state is not observable per node. The broadcast case also checks the listing afterwards and finds the new level in effect (JVM-wide is the only way it is observable in this cluster), then restores the logger to unset.NodeLoggingAPITest(NodeLoggingAPITest.java): 8/8, including the standalonenodesguard, the unchanged local response shape, and a broadcast case in which one node's proxied request fails: the response carries the answering node's result and names the failed node infailedNodes.LoggingHandlerTest(LoggingHandlerTest.java): 1/1 (the V1 handler passes a nullnodes(LoggingHandler.java:77-78), so V1 does not broadcast twice).:solr:core:checkand:solr:api:check -x testpass; Error Prone compile is clean. The generated SolrJ client exposessetNodesand forwards the request body through the proxy (NodeLogging.java:123-125, V2SolrRequestBasedProxy.java:58-71), which the cloud test exercises end to end.-Ptests.selenium=true); the one commit since then adds tests only, and the UI code is unchanged:AdminUiLoggingScreenTest(AdminUiLoggingScreenTest.java) 3/3 andAdminUiLoggingStandaloneTest(AdminUiLoggingStandaloneTest.java) 1/1. Both includetestChangeLogLevelViaUi, which clicks a level in the UI and waits for the server to report it, so the UI's request is exercised end to end in cloud and standalone mode.Choices to check
failedNodes; the alternative is a single overall status. Our position: the per-node shape, because the caller needs to know which nodes still run the old levels. Happy to change the shape if another form is preferred.nodes. Only the set-level PUT honorsnodeshere, as the ticket names. The other endpoints (the level and message GETs and the message threshold PUT (NodeLoggingApis.java:34-65)) could take it too. One caution from the ticket discussion: in V1 the GET is itself a level-changing route when it carriessetparameters (LoggingHandler.java:75-79), which Eric Pugh flagged after stumbling over it in testing, and the ticket's verb-split question (changes on PUT or POST, retrieval on GET) is still open. We kept the ticket's scope, left every GET behaving exactly as it does today, and did not let this PR become the place that settles the GET question. Our plan for the read side: the levels GET, which is the read half of the screen this PR migrates and the one place V1 withnodes=allalready aggregates, is open as #5031, a separate follow-up PR stacked on this one; the messages GET and the threshold PUT stay out unless maintainers ask for them, and the follow-up PR names them in its Limits. If you would rather see the levels GET in this PR, say so and we will fold it in.Were these the right calls?
Limits
config-editpermission there (NodeLogging.java:91-92). Node-to-node calls then go through the container's internal client with PKI authentication (RemoteRequestProxy.java:135-137), the same path the system info read uses. On that path the proxied requests present the sending node's own identity rather than the original caller's, and a node that receives one does not re-authorize it (PKIAuthenticationPlugin.java:371-372): the receiving node runs noconfig-editcheck of its own for a change the broadcast delivers to it. A permission rule that is not cluster-wide is therefore effectively widened by the broadcast: a caller allowedconfig-editon the receiving node alone can change levels on every node the request names. The broadcast has not been tested on a cluster with authentication and authorization enabled.failedNodes, the UI ignores the list, and the screen keeps showing the old level. API callers do get thefailedNodeslist.failedNodesafter the fact (NodeLogging.java:145-151); the change may already have been applied on the nodes that did respond. There is no rollback, matching how the V1 broadcast behaved. A node that drops out of the live set mid-request is a harder version of the same case: the proxy re-checks membership as it sends (RemoteRequestProxy.java:121-131) and fails the request at that point, after earlier nodes may already have applied the change, and the caller gets that error rather than afailedNodeslist.Changelog:
changelog/unreleased/SOLR-15823.ymlAI assistance
AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution.