Repository navigation
Conversation
…d Cluster/Node sidebar. Signed-off-by: prithvi <[email protected]>
4915123 to
fc0b096
Compare
Signed-off-by: prithvi <[email protected]>
… tests. Signed-off-by: prithvi <[email protected]>
|
First question: The "node selector" dropdown, I suppose will still be valid only for the current node, i.e. query, update, schema, segments etc etc? Users may believe that it also obeys the chosen node. Not sure how to deal with that. |
There was a problem hiding this comment.
🟡 Changes recommended
Persisted targets can display local data as remote, and remote Core Admin views can execute mutations against the wrong node.
6 open findings
Core Admin mutates the local node while displaying a remote core · New Premature false initialization bypasses asynchronous mode detection · New Node targeting depends on asynchronous live-node discovery · New Reloaded remote nodes trigger child requests against the local node · New Node selector lacks an accessible semantic label · New Missing browser coverage for node selection and session restoration · New
What changed in this PR
Adds cross-node targeting to the legacy Admin UI and supporting V2 node APIs.
Changes:
- Adds a persistent live-node picker and reorganizes the sidebar.
- Extends node-local APIs with
nodesproxy support. - Adds backend proxy integration tests and UI styling.
| File | Description |
|---|---|
solr/webapp/web/partials/logging-levels.html |
Adds cluster-wide logging notice. |
solr/webapp/web/partials/index.html |
Displays the selected node. |
solr/webapp/web/partials/cores.html |
Uses cloud-mode state consistently. |
solr/webapp/web/partials/cloud.html |
Adds node dashboard and Admin UI links. |
solr/webapp/web/js/angular/services.js |
Introduces persistent node-target state. |
solr/webapp/web/js/angular/controllers/threads.js |
Targets thread dumps by node. |
solr/webapp/web/js/angular/controllers/logging.js |
Targets log messages and adjusts level broadcasting. |
solr/webapp/web/js/angular/controllers/java-properties.js |
Targets Java properties by node. |
solr/webapp/web/js/angular/controllers/index.js |
Targets dashboard system information. |
solr/webapp/web/js/angular/controllers/cores.js |
Loads core status from selected nodes. |
solr/webapp/web/js/angular/controllers/collection-overview.js |
Guards missing collection data. |
solr/webapp/web/js/angular/controllers/cloud.js |
Connects node rows to target selection. |
solr/webapp/web/js/angular/app.js |
Integrates node state into navigation. |
solr/webapp/web/index.html |
Adds the node picker and reorganized menu. |
solr/webapp/web/css/angular/menu.css |
Styles the reorganized sidebar and selectors. |
solr/webapp/web/css/angular/logging.css |
Styles the logging notice. |
solr/webapp/web/css/angular/common.css |
Styles the selected-node badge. |
solr/webapp/web/css/angular/cloud.css |
Styles node Admin UI links. |
solr/core/src/test/org/apache/solr/handler/admin/ThreadDumpHandlerTest.java |
Adapts thread-dump tests. |
solr/core/src/test/org/apache/solr/handler/admin/api/NodeLoggingAPITest.java |
Adapts logging API tests. |
solr/core/src/test/org/apache/solr/handler/admin/api/NodeAdminProxySolrCloudTest.java |
Tests proxied node APIs. |
solr/core/src/java/org/apache/solr/handler/admin/ThreadDumpHandler.java |
Adapts the V1 thread handler. |
solr/core/src/java/org/apache/solr/handler/admin/LoggingHandler.java |
Adapts the V1 logging handler. |
solr/core/src/java/org/apache/solr/handler/admin/api/NodeThreadsAPI.java |
Adds thread-dump proxying. |
solr/core/src/java/org/apache/solr/handler/admin/api/NodeLogging.java |
Adds logging-read proxying. |
solr/core/src/java/org/apache/solr/handler/admin/api/GetNodeProperties.java |
Adds properties proxying. |
solr/core/src/java/org/apache/solr/handler/admin/api/CoreStatus.java |
Adds core-status proxying. |
solr/api/src/java/org/apache/solr/client/api/model/NodeThreadsResponse.java |
Supports keyed remote responses. |
solr/api/src/java/org/apache/solr/client/api/model/NodePropertiesResponse.java |
Supports keyed remote responses. |
solr/api/src/java/org/apache/solr/client/api/model/LoggingResponse.java |
Supports keyed remote responses. |
solr/api/src/java/org/apache/solr/client/api/model/CoreStatusResponse.java |
Supports keyed remote responses. |
solr/api/src/java/org/apache/solr/client/api/endpoint/NodeThreadsApi.java |
Adds the nodes parameter. |
solr/api/src/java/org/apache/solr/client/api/endpoint/NodePropertiesApi.java |
Adds the nodes parameter. |
solr/api/src/java/org/apache/solr/client/api/endpoint/NodeLoggingApis.java |
Adds logging nodes parameters. |
solr/api/src/java/org/apache/solr/client/api/endpoint/CoreApis.java |
Adds core-status node targeting. |
changelog/unreleased/PR#5110-admin-ui-node-picker.yml |
Documents the feature. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| CoresV2.getAllCoreStatus(NodeTarget.withNodes({}), function(error, data, response) { | ||
| $timeout(function() { | ||
| if (error) { ApiErrorHandler.handle(response); return; } | ||
| var payload = NodeTarget.unwrap(data, response) || {}; |
yeah.. picker is only for the Node screens. core menu already hides on a remote node, and i added a "Node screens only" hint :) |
| ng-options="node as nodeLabel(node) for node in liveNodes"></select> | ||
| </div> | ||
| <p id="node-selector-hint" ng-class="{remote: viewingRemoteNode}"> | ||
| {{ viewingRemoteNode ? 'Remote node (Node screens only)' : 'This node — Node screens only' }} |
There was a problem hiding this comment.
I'm not convinced with this either...any better messaging which would help hre? @janhoy
Signed-off-by: prithvi <[email protected]>
2365a02 to
996861b
Compare
|
@dsmiley Do you feel strongly about this vs your approach in #5112 ? David's more generic solution would proxy individual requests rather than aggregate, and would require the Admin UI to send N individual requests to the What I like the most about the ServletFilter approach is that the response shape stays the same and is thus more true to REST / OpenAPI. I.e. AdminUI can use a generated V2 API class to talk to logging / threads / xxx endpoint and get the response as an object. Less special code in UI, and less special code in Solr. What I like with the current PR is that it is ready, mergable and re-uses the same mechanism already used in AdminUI. It could ship in 10.2 and a refactor to |
|
I feel strongly in favor of the generic approach for the reasons you stated. I think it's also less code to maintain in the long run once the former mechanism can be removed. I'm ready to pursue my draft PR #5112 further... I was seeking feedback overall. |
LoggingController's log-message-history screen already followed NodeTarget, but LoggingLevelController's listAllLoggersAndLevels call never did, even though the backend endpoint already accepts nodes= (added by this same PR). Picking a remote node had no effect on the Level tree, silently showing local data while every other node-scoped screen correctly switched. Also fixes isNodeScopedPage's page-name regex in app.js, which recognized 'logging' (the history screen) but not 'logging-levels' (the Level screen), so the header's selected-node badge never rendered there even once the data was correctly node-scoped. Adds testLoggingLevelsScreenFollowsSelectedNode to close the test gap.
Backend (reuse + altitude, confirmed by independent review agents): - Add NodeProxyingJerseyResponse, housing the remoteNodeData field and its @JsonAnyGetter/@JsonAnySetter pair that CoreStatusResponse, LoggingResponse, NodePropertiesResponse, and NodeThreadsResponse each carried as identical copies. They now extend it instead of SolrJerseyResponse directly (ListLevelsResponse/LogMessagesResponse get it for free via their existing LoggingResponse parent). - Add V2SolrRequestBasedProxy.proxyToNodes(CoreContainer, SolrRequest, NodeProxyingJerseyResponse), a generic static helper replacing five near-identical private proxy methods in CoreStatus, GetNodeProperties, NodeLogging (x2), and NodeThreadsAPI. Also drops CoreStatus's stray extra nodes-null/empty guard, which was inconsistent with the other four call sites and already redundant (proxyRequest() already no-ops via shouldProxy() when nodes is unset). Frontend: - services.js: NodeTarget.unwrap()'s data.remoteNodeData[...] fallback branch was dead - confirmed empirically (a live 2-node cluster's raw JSON response) that @JsonAnyGetter flattens remoteNodeData's entries to the top level, so a literal 'remoteNodeData' key never appears on the wire; the body[selectedNode] branch above it is what actually matches. - app.js: resetMenu() fetched core status eagerly, then a second time after listClusterNodes resolved the selected node in cloud mode, discarding the first result; the eager call is now skipped once isCloudEnabled is known true. onSelectedNodeChange() also fetched cores manually right before .reload(), which re-fetches them anyway via resetMenu() before anything renders. Verified with the existing test suites (no behavior change): backend NodeAdminProxySolrCloudTest, NodeLoggingAPITest, ThreadDumpHandlerTest; frontend AdminUiNodePickerTest, AdminUiCoreAdminStandaloneTest, plus a broader Selenium sweep (AdminUiDashboardTest, AdminUiCoreAdminScreenTest, AdminUiLoggingScreenTest, AdminUiLoggingStandaloneTest, AdminUiNodeScreensTest, AdminUiCollectionsScreenTest, AdminUiSmokeTest) since resetMenu/onSelectedNodeChange are shared by every page.
|
@iprithv can you review my two commits? |
The file's other two @test methods carry no javadoc; their names already say what they verify. This one's comment added nothing a reader couldn't get from testLoggingLevelsScreenFollowsSelectedNode itself.
…loud test LoggingResponse's new remoteNodeData field duplicated the exact same Map + @JsonAnyGetter/@JsonAnySetter pair already carried by NodeSystemResponse (pre-existing on main, backing GetNodeSystemInfo, which this PR's own description says it mirrors). Extracted a shared NodeProxyingJerseyResponse base; both now extend it instead of SolrJerseyResponse directly, and LoggingResponse's net-new addition is just failedNodes. This NodeProxyingJerseyResponse is intentionally identical to the one introduced in PR apache#5110, which independently deduplicates the same pattern across CoreStatusResponse/NodePropertiesResponse/ NodeThreadsResponse/LoggingResponse. Doing the same extraction here means neither PR depends on the other's merge order: whichever lands first, the other's rebase over the shared class is close to a no-op rather than a real conflict. Also converted NodeLoggingNodesSolrCloudTest from hand-built GenericV2SolrRequest + raw JSON tree assertions to the generated LoggingApi.ModifyLocalLogLevel/ListAllLoggersAndLevels clients with typed LoggingResponse/ListLevelsResponse assertions, matching this PR's own NodeLoggingAPITest and the Testing V2 APIs convention (apis-v2-conventions.adoc) of exercising the generated codegen rather than bypassing it. Verified: NodeLoggingAPITest 8/8, NodeLoggingNodesSolrCloudTest 4/4, LoggingHandlerTest 1/1, NodeSystemInfoProviderTest 2/2, GetNodeSystemInfoSolrCloudTest 1/1, GetNodeSystemInfoTest 1/1.
|
nice... |














https://issues.apache.org/jira/browse/SOLR-12613
HACKATHON :)
added a Node picker so Thread Dump, Java Properties, Logging, Cores, and Dashboard can target any live node via the existing nodes= proxy.
sidebar is split into Cluster and Node. Works in SolrCloud and stays out of the way in standalone.