Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names #1593

Merged
merged 2 commits into from
Apr 30, 2023
Merged

SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names #1593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3128b8
UI: Cloud>Nodes screen fix display of cores with non-standard shard n…
janhoy Apr 27, 2023
29ebe51
Simplify, no need for function called once
janhoy Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ Bug Fixes

* SOLR-16755: bin/solr's '-noprompt' option no longer works for examples (hossman, janhoy, Houston Putman)

* SOLR-16773: UI: Cloud>Nodes screen - fix display of cores with non-standard shard names (janhoy, hossman)

* SOLR-7609: Internal update requests should fail back to the client in some edge cases for shard splits. Use HTTP status 510 so the client can retry the operation. (Alex Deparvu, David Smiley, Tomás Fernández Löbbe)

* SOLR-16737: Http2SolrClient needs to inherit all properties when initialized with another http2 client (Alex Deparvu, Tomás Fernández Löbbe)
Expand Down
11 changes: 5 additions & 6 deletions solr/webapp/web/js/angular/controllers/cloud.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,6 @@ function isNumeric(n) {
return !isNaN(parseFloat(n)) && isFinite(n);
}

function coreNameToLabel(name) {
return name.replace(/(.*?)_shard((\d+_?)+)_replica_?[ntp]?(\d+)/, '\$1_s\$2r\$4');
}

var nodesSubController = function($scope, Collections, System, Metrics) {
$scope.pageSize = 10;
$scope.showNodes = true;
Expand Down Expand Up @@ -198,10 +194,13 @@ var nodesSubController = function($scope, Collections, System, Metrics) {
for (var replicaName in replicas) {
var core = replicas[replicaName];
core.name = replicaName;
core.label = coreNameToLabel(core['core']);
core.replica = core['core'].replace(/.*_(replica_.*)$/, '\$1');
core.collection = collection.name;
core.shard = shard.name;
core.shard_state = shard.state;
core.label = core['collection'] + "_"
+ (core['shard'] + "_").replace(/shard(\d+)_/, 's\$1')
+ core['replica'].replace(/replica_?[ntp]?(\d+)/, 'r\$1');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think could be made a bit clearer as to what is going here in the GitHub description, e.g. super imposing a box on what has changed or a before/after, which is somewhat standard for UI. Alternatively, you could show how some of the names were treated in the past and how they will be treated going forward. Otherwise, mostly looks good.

However, I know this is in maintenance mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only question is, what is the ntp here for? I'm assuming NRT, Pull, and TLog replicas?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any reasons why we would not do any processing whatsoever of the shard names and simply show them as they were named? I assume there are some other screens or other confusion that could arise, but I am trying to doubly check that I understand the approach here fully.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label is for display purpose only. There is no before/after. This code produces exactly same display label as previously, but now also works for the custom shard names in implicit mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, ntp is replica type.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the test failure have anything to do with the change? It doesn't seem like it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, when implicit mode what would happen before this change?

After, it will show up. It would be helpful to know if (1) there was a particular error, (2) didn't show up at all, (3) was a broken page, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, when implicit mode what would happen before this change?

The label would be ok, but we’d use wrong key for fetching metrics and get empty values in UI.


var node_name = core['node_name'];
var node = getOrCreateObj(node_name, nodes);
Expand Down Expand Up @@ -433,7 +432,7 @@ var nodesSubController = function($scope, Collections, System, Metrics) {
var labelState = (core['state'] !== 'active') ? core['state'] : core['shard_state'];
core['label'] += "_(" + labelState + ")";
}
var coreMetricName = "solr.core." + core['core'].replace(/(.*?)_(shard(\d+_?)+)_(replica.*?)/, '\$1.\$2.\$4');
var coreMetricName = "solr.core." + core['collection'] + "." + core['shard'] + "." + core['replica'];
var coreMetric = m.metrics[coreMetricName];
// we may not actually get metrics back for every expected core (the core may be down)
if (coreMetric) {
Expand Down