You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "janhoy (via GitHub)" <gi...@apache.org> on 2023/04/27 11:52:41 UTC
[GitHub] [solr] janhoy opened a new pull request, #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
janhoy opened a new pull request, #1593:
URL: https://github.com/apache/solr/pull/1593
https://issues.apache.org/jira/browse/SOLR-16773
--
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
[GitHub] [solr] MarcusSorealheis commented on a diff in pull request #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
Posted by "MarcusSorealheis (via GitHub)" <gi...@apache.org>.
MarcusSorealheis commented on code in PR #1593:
URL: https://github.com/apache/solr/pull/1593#discussion_r1181217959
##########
solr/webapp/web/js/angular/controllers/cloud.js:
##########
@@ -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');
Review Comment:
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.
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1593:
URL: https://github.com/apache/solr/pull/1593#discussion_r1181120304
##########
solr/webapp/web/js/angular/controllers/cloud.js:
##########
@@ -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');
Review Comment:
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.
--
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
[GitHub] [solr] MarcusSorealheis commented on a diff in pull request #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
Posted by "MarcusSorealheis (via GitHub)" <gi...@apache.org>.
MarcusSorealheis commented on code in PR #1593:
URL: https://github.com/apache/solr/pull/1593#discussion_r1180838335
##########
solr/webapp/web/js/angular/controllers/cloud.js:
##########
@@ -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');
Review Comment:
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.
##########
solr/webapp/web/js/angular/controllers/cloud.js:
##########
@@ -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');
Review Comment:
My only question is, what is the ntp here for? I'm assuming NRT, Pull, and TLog replicas?
--
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
[GitHub] [solr] MarcusSorealheis commented on a diff in pull request #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
Posted by "MarcusSorealheis (via GitHub)" <gi...@apache.org>.
MarcusSorealheis commented on code in PR #1593:
URL: https://github.com/apache/solr/pull/1593#discussion_r1181217317
##########
solr/webapp/web/js/angular/controllers/cloud.js:
##########
@@ -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');
Review Comment:
Does the test failure have anything to do with the change? It doesn't seem like 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
[GitHub] [solr] janhoy merged pull request #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy merged PR #1593:
URL: https://github.com/apache/solr/pull/1593
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1593:
URL: https://github.com/apache/solr/pull/1593#discussion_r1181274568
##########
solr/webapp/web/js/angular/controllers/cloud.js:
##########
@@ -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');
Review Comment:
> 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.
--
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
[GitHub] [solr] MarcusSorealheis commented on a diff in pull request #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
Posted by "MarcusSorealheis (via GitHub)" <gi...@apache.org>.
MarcusSorealheis commented on code in PR #1593:
URL: https://github.com/apache/solr/pull/1593#discussion_r1180839759
##########
solr/webapp/web/js/angular/controllers/cloud.js:
##########
@@ -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');
Review Comment:
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.
--
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
[GitHub] [solr] janhoy commented on a diff in pull request #1593: SOLR-16773: UI: Cloud>Nodes screen fix display of cores with non-standard shard names
Posted by "janhoy (via GitHub)" <gi...@apache.org>.
janhoy commented on code in PR #1593:
URL: https://github.com/apache/solr/pull/1593#discussion_r1181120393
##########
solr/webapp/web/js/angular/controllers/cloud.js:
##########
@@ -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');
Review Comment:
Yes, `ntp` is replica type.
--
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