[
https://issues.apache.org/jira/browse/SOLR-11526?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16217799#comment-16217799
]
Varun Thacker commented on SOLR-11526:
--------------------------------------
This is bad. Thanks Jason for brining it up.
I think as a first step if someone could shed some light as to whether we
should look at the status code or the "success" key. I always used to believe
that status!=0 is how users determined if an API call failed but you've shown
an example where a failure still returns status=0 .
[[email protected]] You have worked on two related Jiras : SOLR-5392 and
SOLR-4576 . Do you have some thoughts on this?
If I was to propose a change I would change the {{isSuccess}} method to check
the status code and return true / false accordingly. And make sure
TestCollectionAdminRequest / TestCollectionAPI actually test it
This might require a lot of changes though. For example DeleteShardCmd does
this check today and now we should look at status code and not the "success"
key we should first assert that the status code is 0
{code}
NamedList deleteResult = new NamedList();
try {
((DeleteReplicaCmd)ocmh.commandMap.get(DELETEREPLICA)).deleteReplica(clusterState,
replica, deleteResult, () -> {
cleanupLatch.countDown();
if (deleteResult.get("failure") != null) {
synchronized (results) {
results.add("failure", String.format(Locale.ROOT, "Failed to
delete replica for collection=%s shard=%s" +
" on node=%s", replica.getStr(COLLECTION_PROP),
replica.getStr(SHARD_ID_PROP), replica.getStr(NODE_NAME_PROP)));
}
}
SimpleOrderedMap success = (SimpleOrderedMap)
deleteResult.get("success");
if (success != null) {
synchronized (results) {
results.add("success", success);
}
}
});
{code}
You mentioned in the description of the jira that createshard does not set a
"success" key but I see this is CreateShardCmd.java. I don't think most of the
others are still buggy like you mentioned.
{code}
} else {
SimpleOrderedMap success = (SimpleOrderedMap)
results.get("success");
if (success == null) {
success = new SimpleOrderedMap();
results.add("success", success);
}
success.addAll((NamedList) addResult.get("success"));
}
});
{code}
A SolrJ user is calling {{CollectionAdminRequest#listCollections}} . This
throws an exception so that's how today users are building success/failure
logic ? Users using the HTTP API directly would run into all of the issues you
are mentioning though
> CollectionAdminResponse.isSuccess() incorrect for most admin collections APIs
> -----------------------------------------------------------------------------
>
> Key: SOLR-11526
> URL: https://issues.apache.org/jira/browse/SOLR-11526
> Project: Solr
> Issue Type: Improvement
> Security Level: Public(Default Security Level. Issues are Public)
> Components: SolrJ
> Affects Versions: master (8.0)
> Reporter: Jason Gerlowski
> Priority: Minor
>
> {{CollectionAdminResponse}} has a boolean {{isSuccess}} method which reports
> whether the API was called successfully. It returns true if it finds a
> non-null NamedList element called "success". It returns false otherwise.
> Unfortunately, only a handful of the Collection-Admin APIs have this element.
> APIs that don't contain this element in their response will always appear to
> have failed (according to {{isSuccess()}}).
> The current implementation is correct for:
> - CREATECOLLECTION
> - RELOAD
> - SPLITSHARD
> - DELETESHARD
> - DELETECOLLECTION
> - ADDREPLICA
> - MIGRATE
> The current implementation is incorrect for:
> - CREATESHARD
> - CREATEALIAS
> - DELETEALIAS
> - LISTALIASES
> - CLUSTERPROP
> - ADDROLE
> - REMOVEROLE
> - OVERSEERSTATUS
> - CLUSTERSTATUS
> - REQUESTSTATUS
> - DELETESTATUS
> - LIST
> - ADDREPLICAPROP
> - DELETEREPLICAPROP
> - BALANCESHARDUNIQUE
> - REBALANCELEADERS
> (these lists are incomplete)
> A trivial fix for this would be to change the implementation to check the
> "status" NamedList element (which is present in all Collection-Admin APIs).
> My understanding is that the "status' field is set to 0 always on success.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
