You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/12/30 07:34:35 UTC

[GitHub] [solr] ijioio opened a new pull request #479: SOLR-15842: Fix async backup response

ijioio opened a new pull request #479:
URL: https://github.com/apache/solr/pull/479


   https://issues.apache.org/jira/browse/SOLR-15842
   
   # Description
   
   Adding new field to `org.apache.solr.handler.admin.CoreAdminHandler.TaskObject` to hold operation results.
   
   # Solution
   
   I apply a simple changes that adds an additional field `operationRspInfo` to `org.apache.solr.handler.admin.CoreAdminHandler.TaskObject`. It is filled with operation results in case of operation finished successfully (it is exactly the same results that are used in case of sync request). This way we store the results within `TaskObject`.
   
   Later, when request status is sent, besides adding standard value `Response`, an additional value `response` will be added to request response. Value `response` will hold operation results preserved in `TaskObject`.
   
   # Tests
   
   None
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [x] I have created a Jira issue and added the issue ID to my pull request title.
   - [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [x] I have developed this patch against the `main` branch.
   - [ ] I have run `./gradlew check`.
   - [ ] I have added tests for my changes.
   - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1010953596


   > It seems like you and @cpoerschke are in favor of removing Response altogether, just leaving response. I think it would be fine to change Response -> message and leave the information, but if y'all don't think it's useful I won't put up a fight.
   
   I'm OK with both ways. I probably more tended for the remove option because `toLog` list seems to be intended for logging purposes, not for building responses. I've checked the usages of it and it is always used for logging only (except the case of async calls of admin operations mentioned in this issue). There is also a case of using `toLog` to store `hits` value but it is used just as a value holder, not for the response. It is easy to see if you check the calls of `org.apache.solr.response.SolrQueryResponse.getToLog()` and `org.apache.solr.response.SolrQueryResponse.getToLogAsString(String)`). Another reason is that all this data appear as a response only in an internal subrequest data of async call, not in an outer initial request data, so internal `Request` data seems valuable for me only for debugging purposes. 
   
   Maybe we should wait for someone more knowledgeable to advise?


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1006688286


   > I think this could be a good change to make at the same time as [SOLR-15884](https://issues.apache.org/jira/browse/SOLR-15884), since we would be changing the expected response structure for backups.
   
   Agree, I can investigate to find out is it sensitive for changing list vs map mentioned in the SOLR-15884. But as far as I know, the aggregated part of the response is not used within the main Solr code (except maybe SolrJ) but can be actively used outside of Solr by users. 


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1019241112


   > @ijioio What name do you want used when I add a Changelog entry for this?
   
   The most work is done by you ))) Thanks!


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1018906313


   Ok, I brought over that change from #549, so now this PR adds the non-incremental backup response, and correctly merges the responses for async and non-async calls.


-- 
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] HoustonPutman edited a comment on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman edited a comment on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1017821168


   The change from 'Response' to 'msg' turned out to be very easy and didn't affect much else, so I went ahead and did it. This also allowed me to remove the `RESPONSE` variable, since it is no longer in use (meaning that we won't see it in any other responses).
   
   @ijioio I think this should be good to go. Would you mind testing it out locally? Then I can get to work on SOLR-15884 And have this in before 9.0!


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1020190066


   > The most work is done by you ))) Thanks!
   
   But we always try to give out credit to all involved! I will use @ijioio unless you have another name you prefer 🙂 


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1009716765


   @cpoerschke @HoustonPutman I've checked all the sources searching `"Response"` entry and haven't found any pieces of evidence that value under this key is extracted or parsed somewhere. This means that from the perspective of view of Solr itself it seems that it is safe to replace original `Response` entry with `response` entry. 


-- 
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] cpoerschke commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
cpoerschke commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1005554465


   Hello @ijioio, thanks for opening this pull request!
   
   > This is how response for backup status request will looks like in case of adding `response` field:
   
   I found seeing this very helpful, thank you for including it in this PR.
   
   > * I don't think that messing with the original `Response` value is a good idea. It is a general response format used for all the core admin operations. So there may be a chance of someone depending on its content/format.
   
   Yes, if the response format were to change that would need to be clearly communicated to users e.g. via the https://github.com/apache/solr/blob/main/solr/CHANGES.txt and/or https://github.com/apache/solr/blob/main/solr/solr-ref-guide/src/major-changes-in-solr-9.adoc notes, because as you say someone might depend on its content or format.
   
   At least in the case of the `BACKUPCORE` here much of the `Response` seems more like a repeat of the request parameters rather than response information content. And via `something["Response"] instanceof String` or similar clients could differentiate between old and new format. So changing the response format could be practical then? Having `Response` and `response` elements co-exist seems to me confusing in the long-term and for new users.
   
   > * In case we use some field other than `response` we also need to update the function `aggregateResults` to support two types of response types for aggregation. In contrast, if we use the `response` field it will be compatible with current `aggregateResults` code.
   
   That's very insightful, thanks. I wonder what updating the `aggregateResults` might look like, the method has `private` visibility and so changing its signature (if necessary) would be easily possible.
   
   > Another way to solve the problem, is to put results of the operation to the `toLog` list of the response. But this implies parsing the generated string located in the `Response` field and extracting all the required fields from it within the `aggregateResults` method. Looks unnecessary complex.
   
   Yes, that looks too complex and fragile.


-- 
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] HoustonPutman merged pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman merged pull request #479:
URL: https://github.com/apache/solr/pull/479


   


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1020308676


   > But we always try to give out credit to all involved! I will use @ijioio unless you have another name you prefer 🙂
   
   I see, please use `Artem Abeleshev` then 


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1018888098


   I also added in the incremental backup information `indexFileCount`, `uploadedIndexFileCount`, `uploadedIndexFileMB` and `shardBackupIds` in the aggregated response. (if they are in the initial response)


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1007509659


   @ijioio I think this would be great to get into 9.0, because of the backwards incompatibility. 9.0 currently has a feature freeze, but I'm fine arguing for this as a blocker as long as we could get it done in the next few weeks. Let me know how I can help, if I can 🙂 


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1006680521


   > Yes, if the response format were to change that would need to be clearly communicated to users e.g. via the https://github.com/apache/solr/blob/main/solr/CHANGES.txt and/or https://github.com/apache/solr/blob/main/solr/solr-ref-guide/src/major-changes-in-solr-9.adoc notes, because as you say someone might depend on its content or format.
   
   Initially, I was not hoping to go that far as changing the response format. But I personally think this is the best option if possible, especially taking into account another issue concerning responses unification.
   
   > At least in the case of the `BACKUPCORE` here much of the `Response` seems more like a repeat of the request parameters rather than response information content.
   
   Yes, you're absolutely right. All the `Response` value is actually is the content of the `toLog` list located inside the `org.apache.solr.response.SolrQueryResponse`. It is filled only on `preDecorateResponse` and `postDecorateResponse` calls within `org.apache.solr.servlet.HttpSolrCall.handleAdminRequest()`:
   
   ```java
     private void handleAdminRequest() throws IOException {
       SolrQueryResponse solrResp = new SolrQueryResponse();
       SolrCore.preDecorateResponse(solrReq, solrResp);
       handleAdmin(solrResp);
       SolrCore.postDecorateResponse(handler, solrReq, solrResp);
       ....
     }
   ```
   
   Nothing much of the interest is added there: path, query parameters, time, status, and handling echo parameters if provided. In the case of backup, only an async id is added there.
   
   > So changing the response format could be practical then? Having Response and response elements co-exist seems to me confusing in the long-term and for new users.
   
   Agree! It is confusing. I think, if possible, replacing the current `Response` block with a standard `response` is a good option. Maybe we can investigate in that way?
   
   > That's very insightful, thanks. I wonder what updating the aggregateResults might look like, the method has private visibility and so changing its signature (if necessary) would be easily possible.
   
   That's true. It will not be a problem to update it if needed.


-- 
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] ijioio edited a comment on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio edited a comment on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1002910206


   It is maybe seems a bit strange to hold two values `Response` and `response`, but I made it for two reasons:
   
   - I don't think that messing with the original `Response` value is a good idea. It is a general response format used for all the core admin operations. So there may be a chance of someone depending on its content/format.
   - In case we use some field other than `response` we also need to update the function `aggregateResults` to support two types of response formats for aggregation. In contrast, if we use the `response` field it will be compatible with current`aggregateResults` code.
   
   This is how response for status request will looks like in case of adding `response` field:
   
   ```json
   {
       "responseHeader": {
           "status": 0,
           "QTime": 19
       },
       "success": {
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 4
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 8
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
               "response": [
                   "startTime",
                   "2021-12-30T06:45:55.978702Z",
                   "indexFileCount",
                   1,
                   "uploadedIndexFileCount",
                   0,
                   "indexSizeMB",
                   0.001,
                   "uploadedIndexFileMB",
                   0.001,
                   "shard",
                   "shard2",
                   "endTime",
                   "2021-12-30T06:45:55.995335Z",
                   "shardBackupId",
                   "md_shard2_1"
               ]
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
               "response": [
                   "startTime",
                   "2021-12-30T06:46:10.266750Z",
                   "indexFileCount",
                   21,
                   "uploadedIndexFileCount",
                   21,
                   "indexSizeMB",
                   0.006,
                   "uploadedIndexFileMB",
                   0.006,
                   "shard",
                   "shard1",
                   "endTime",
                   "2021-12-30T06:46:10.438942Z",
                   "shardBackupId",
                   "md_shard1_1"
               ]
           }
       },
       "10562744829711078047": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
           "response": [
               "startTime",
               "2021-12-30T06:45:55.978702Z",
               "indexFileCount",
               1,
               "uploadedIndexFileCount",
               0,
               "indexSizeMB",
               0.001,
               "uploadedIndexFileMB",
               0.001,
               "shard",
               "shard2",
               "endTime",
               "2021-12-30T06:45:55.995335Z",
               "shardBackupId",
               "md_shard2_1"
           ]
       },
       "10562744829713680734": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
           "response": [
               "startTime",
               "2021-12-30T06:46:10.266750Z",
               "indexFileCount",
               21,
               "uploadedIndexFileCount",
               21,
               "indexSizeMB",
               0.006,
               "uploadedIndexFileMB",
               0.006,
               "shard",
               "shard1",
               "endTime",
               "2021-12-30T06:46:10.438942Z",
               "shardBackupId",
               "md_shard1_1"
           ]
       },
       "response": [
           "collection",
           "techproducts",
           "numShards",
           2,
           "backupId",
           1,
           "indexVersion",
           "9.0.0",
           "startTime",
           "2021-12-30T06:45:35.738064Z",
           "indexSizeMB",
           0.007
       ],
       "status": {
           "state": "completed",
           "msg": "found [1056] in completed tasks"
       }
   }
   ```
   
   Another way to solve the problem, is to put results of the operation to the `toLog` list of the response. But this implies parsing the generated string located in the `Response` field and extracting all the required fields from it within the `aggregateResults` method. Looks unnecessary complex.


-- 
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] ijioio edited a comment on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio edited a comment on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1002910206


   It is maybe seems a bit strange to hold two values `Response` and `response`, but I made it for two reasons:
   
   - I don't think that messing with the original `Response` value is a good idea. It is a general response format used for all the core admin operations. So there may be a chance of someone depending on its content/format.
   - In case we use some field other than `response` we also need to update the function `aggregateResults` to support two types of response types for aggregation. In contrast, if we use the `response` field it will be compatible with current`aggregateResults` code.
   
   This is how response for backup status request will looks like in case of adding `response` field:
   
   ```json
   {
       "responseHeader": {
           "status": 0,
           "QTime": 19
       },
       "success": {
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 4
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 8
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
               "response": [
                   "startTime",
                   "2021-12-30T06:45:55.978702Z",
                   "indexFileCount",
                   1,
                   "uploadedIndexFileCount",
                   0,
                   "indexSizeMB",
                   0.001,
                   "uploadedIndexFileMB",
                   0.001,
                   "shard",
                   "shard2",
                   "endTime",
                   "2021-12-30T06:45:55.995335Z",
                   "shardBackupId",
                   "md_shard2_1"
               ]
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
               "response": [
                   "startTime",
                   "2021-12-30T06:46:10.266750Z",
                   "indexFileCount",
                   21,
                   "uploadedIndexFileCount",
                   21,
                   "indexSizeMB",
                   0.006,
                   "uploadedIndexFileMB",
                   0.006,
                   "shard",
                   "shard1",
                   "endTime",
                   "2021-12-30T06:46:10.438942Z",
                   "shardBackupId",
                   "md_shard1_1"
               ]
           }
       },
       "10562744829711078047": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
           "response": [
               "startTime",
               "2021-12-30T06:45:55.978702Z",
               "indexFileCount",
               1,
               "uploadedIndexFileCount",
               0,
               "indexSizeMB",
               0.001,
               "uploadedIndexFileMB",
               0.001,
               "shard",
               "shard2",
               "endTime",
               "2021-12-30T06:45:55.995335Z",
               "shardBackupId",
               "md_shard2_1"
           ]
       },
       "10562744829713680734": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
           "response": [
               "startTime",
               "2021-12-30T06:46:10.266750Z",
               "indexFileCount",
               21,
               "uploadedIndexFileCount",
               21,
               "indexSizeMB",
               0.006,
               "uploadedIndexFileMB",
               0.006,
               "shard",
               "shard1",
               "endTime",
               "2021-12-30T06:46:10.438942Z",
               "shardBackupId",
               "md_shard1_1"
           ]
       },
       "response": [
           "collection",
           "techproducts",
           "numShards",
           2,
           "backupId",
           1,
           "indexVersion",
           "9.0.0",
           "startTime",
           "2021-12-30T06:45:35.738064Z",
           "indexSizeMB",
           0.007
       ],
       "status": {
           "state": "completed",
           "msg": "found [1056] in completed tasks"
       }
   }
   ```
   
   Another way to solve the problem, is to put results of the operation to the `toLog` list of the response. But this implies parsing the generated string located in the `Response` field and extracting all the required fields from it within the `aggregateResults` method. Looks unnecessary complex.


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1009625781


   Hi, @HoustonPutman Thanks for the message!
   
   > @ijioio I think this would be great to get into 9.0, because of the backwards incompatibility.
   
   Just to clarify, you're talking about backup format (`Response` update to use `NamedList` instead of `String`) or about aggregated part of the response (switching from `array` to `dictionary`)? Or both?. Sorry if I don't understand you correctly. 
   
   I think I can switch `Response` of type `String` to `response` with `NamedList` of the actual response and investigate more deeply. But as @cpoerschke mentioned above it should be first decided is it OK to make those kinds of general changes. 
   
   All the async managing for the collections are handled by `org.apache.solr.cloud.api.collections.CollectionHandlingUtils.ShardRequestTracker`. First the `sendShardRequest(String, ModifiableSolrParams, ShardHandler, String, ZkStateReader)` is called which will place `asyncId` (if provided) to tracking map `shardAsyncIdByNode`. Then, method `processResponses(NamedList<Object>, ShardHandler, boolean, String, Set<String>)` is executed. In case the initial request was in async mode it will additionaly call `waitForAsyncCallsToComplete` method at the end to gather all the results from all the shards. I suppose originally an idea was to implement the universal async system. So in theory, it is possible to call any distributed operation in async mode. That makes me think that no internal parsing of the shards results is applied except some special cases (like aggregation for backup). Checking of all the possible operations that call `processResponses` to understand how they handle results
  will take time (it doesn't mean it is impossible though :)


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1007509659


   @ijioio I think this would be great to get into 9.0, because of the backwards incompatibility. 9.0 currently has a feature freeze, but I'm fine arguing for this as a blocker as long as we could get it done in the next few weeks. Let me know how I can help, if I can 🙂 


-- 
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] HoustonPutman edited a comment on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman edited a comment on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1018888098


   I also added in the incremental backup information `indexFileCount`, `uploadedIndexFileCount`, `uploadedIndexFileMB` and `shardBackupIds` in the aggregated response. (if they are in the initial response)
   
   I tested and this works with incremental and non-incremental (though the non-incremental backups will not return a response until #549 is merged)


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1019224284


   > The change from 'Response' to 'msg' turned out to be very easy and didn't affect much else, so I went ahead and did it. This also allowed me to remove the RESPONSE variable, since it is no longer in use (meaning that we won't see it in any other responses).
   
   > I also added in the incremental backup information indexFileCount, uploadedIndexFileCount, uploadedIndexFileMB and shardBackupIds in the aggregated response. (if they are in the initial response)
   
   Thanks for the updates! Looks great!
   
   > @ijioio I think this should be good to go. Would you mind testing it out locally? Then I can get to work on SOLR-15884 And have this in before 9.0!
   
   Sure! Here is the response I've got locally, seems works OK:
   
   ```json
   {
       "responseHeader": {
           "status": 0,
           "QTime": 1
       },
       "success": {
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "msg": "TaskId: 10006758022670100 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n2&async=10006758022670100&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_0&action=BACKUPCORE&location=file:///path/to/my/shared/drive/mybackup/techproducts&incremental=true&wt=javabin&version=2} status=0 QTime=0",
               "response": {
                   "startTime": "2022-01-22T12:01:32.192663Z",
                   "indexFileCount": 1,
                   "uploadedIndexFileCount": 1,
                   "indexSizeMB": 0.001,
                   "uploadedIndexFileMB": 0.001,
                   "shard": "shard2",
                   "endTime": "2022-01-22T12:01:34.928067Z",
                   "shardBackupId": "md_shard2_0"
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "msg": "TaskId: 10006758023398500 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n4&async=10006758023398500&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_0&action=BACKUPCORE&location=file:///path/to/my/shared/drive/mybackup/techproducts&incremental=true&wt=javabin&version=2} status=0 QTime=0",
               "response": {
                   "startTime": "2022-01-22T12:01:32.192663Z",
                   "indexFileCount": 21,
                   "uploadedIndexFileCount": 21,
                   "indexSizeMB": 0.006,
                   "uploadedIndexFileMB": 0.006,
                   "shard": "shard1",
                   "endTime": "2022-01-22T12:01:35.505593300Z",
                   "shardBackupId": "md_shard1_0"
               }
           }
       },
       "10006758022670100": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "msg": "TaskId: 10006758022670100 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n2&async=10006758022670100&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_0&action=BACKUPCORE&location=file:///path/to/my/shared/drive/mybackup/techproducts&incremental=true&wt=javabin&version=2} status=0 QTime=0",
           "response": {
               "startTime": "2022-01-22T12:01:32.192663Z",
               "indexFileCount": 1,
               "uploadedIndexFileCount": 1,
               "indexSizeMB": 0.001,
               "uploadedIndexFileMB": 0.001,
               "shard": "shard2",
               "endTime": "2022-01-22T12:01:34.928067Z",
               "shardBackupId": "md_shard2_0"
           }
       },
       "10006758023398500": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "msg": "TaskId: 10006758023398500 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n4&async=10006758023398500&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_0&action=BACKUPCORE&location=file:///path/to/my/shared/drive/mybackup/techproducts&incremental=true&wt=javabin&version=2} status=0 QTime=0",
           "response": {
               "startTime": "2022-01-22T12:01:32.192663Z",
               "indexFileCount": 21,
               "uploadedIndexFileCount": 21,
               "indexSizeMB": 0.006,
               "uploadedIndexFileMB": 0.006,
               "shard": "shard1",
               "endTime": "2022-01-22T12:01:35.505593300Z",
               "shardBackupId": "md_shard1_0"
           }
       },
       "response": {
           "collection": "techproducts",
           "numShards": 2,
           "backupId": 0,
           "indexVersion": "9.0.0",
           "startTime": "2022-01-22T12:01:29.247112200Z",
           "indexFileCount": 22,
           "uploadedIndexFileCount": 22,
           "indexSizeMB": 0.007,
           "uploadedIndexFileMB": 0.007,
           "shardBackupIds": [
               "md_shard2_0",
               "md_shard1_0"
           ]
       },
       "status": {
           "state": "completed",
           "msg": "found [1000] in completed tasks"
       }
   }
   ```


-- 
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] ijioio commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1002910206


   It is maybe seems a bit strange to hold two values `Result` and `result`, but I made it for two reasons:
   
   - I don't think that messing with the original `Response` value is a good idea. It is a general response format used for all the core admin operations. So there may be a chance of someone depending on its content/format.
   - In case we use some field other than `response` we also need to update the function `aggregateResults` to support two types of response formats for aggregation. In contrast, if we use the `response` field it will be compatible with current`aggregateResults` code.
   
   This is how response for status request will looks like in case of adding `response` field:
   
   ```json
   {
       "responseHeader": {
           "status": 0,
           "QTime": 19
       },
       "success": {
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 4
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 8
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
               "response": [
                   "startTime",
                   "2021-12-30T06:45:55.978702Z",
                   "indexFileCount",
                   1,
                   "uploadedIndexFileCount",
                   0,
                   "indexSizeMB",
                   0.001,
                   "uploadedIndexFileMB",
                   0.001,
                   "shard",
                   "shard2",
                   "endTime",
                   "2021-12-30T06:45:55.995335Z",
                   "shardBackupId",
                   "md_shard2_1"
               ]
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
               "response": [
                   "startTime",
                   "2021-12-30T06:46:10.266750Z",
                   "indexFileCount",
                   21,
                   "uploadedIndexFileCount",
                   21,
                   "indexSizeMB",
                   0.006,
                   "uploadedIndexFileMB",
                   0.006,
                   "shard",
                   "shard1",
                   "endTime",
                   "2021-12-30T06:46:10.438942Z",
                   "shardBackupId",
                   "md_shard1_1"
               ]
           }
       },
       "10562744829711078047": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
           "response": [
               "startTime",
               "2021-12-30T06:45:55.978702Z",
               "indexFileCount",
               1,
               "uploadedIndexFileCount",
               0,
               "indexSizeMB",
               0.001,
               "uploadedIndexFileMB",
               0.001,
               "shard",
               "shard2",
               "endTime",
               "2021-12-30T06:45:55.995335Z",
               "shardBackupId",
               "md_shard2_1"
           ]
       },
       "10562744829713680734": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
           "response": [
               "startTime",
               "2021-12-30T06:46:10.266750Z",
               "indexFileCount",
               21,
               "uploadedIndexFileCount",
               21,
               "indexSizeMB",
               0.006,
               "uploadedIndexFileMB",
               0.006,
               "shard",
               "shard1",
               "endTime",
               "2021-12-30T06:46:10.438942Z",
               "shardBackupId",
               "md_shard1_1"
           ]
       },
       "response": [
           "collection",
           "techproducts",
           "numShards",
           2,
           "backupId",
           1,
           "indexVersion",
           "9.0.0",
           "startTime",
           "2021-12-30T06:45:35.738064Z",
           "indexSizeMB",
           0.007
       ],
       "status": {
           "state": "completed",
           "msg": "found [1056] in completed tasks"
       }
   }
   ```
   
   Another way to solve the problem, is to put results of the operation to the `toLog` list of the response. But this implies parsing the generated string located in the `Response` field and extracting all the required fields from it within the `aggregateResults` method. Looks unnecessary complex.


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1005855293


   > Yes, if the response format were to change that would need to be clearly communicated to users e.g. via the https://github.com/apache/solr/blob/main/solr/CHANGES.txt and/or https://github.com/apache/solr/blob/main/solr/solr-ref-guide/src/major-changes-in-solr-9.adoc notes, because as you say someone might depend on its content or format.
   
   I think this could be a good change to make at the same time as [SOLR-15884](https://issues.apache.org/jira/browse/SOLR-15884), since we would be changing the expected response structure for backups.
   
   Personally I think `Response` could be changed to `message` and `response` could be left the same, but that's just my first thought.
   
   > > This is how response for backup status request will looks like in case of adding response field:
   
   > I found seeing this very helpful, thank you for including it in this PR.
   
   I completely agree, this was very detailed and informative. Thanks so much for your research here.


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1017821168


   The change from 'Response' to 'msg' turned out to be very easy and didn't affect much else, so I went ahead and did it. This also allowed me to remove the `RESPONSE` variable, since it is no longer in use (meaning that we won't see it in any other responses).
   
   I think this should be good to go. Would you mind testing it out locally? Then I can get to work on SOLR-15884 And have this in before 9.0!


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1017914278


   @ijioio  What name do you want used when I add a Changelog entry for this?


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1010097795


   > Just to clarify, you're talking about backup format (Response update to use NamedList instead of String) or about aggregated part of the response (switching from array to dictionary)? Or both?. Sorry if I don't understand you correctly.
   
   I'm talking about both (make `response` a map, and either remove `Response` or change it's name to something else). Since both are backwards-incompatible changes, I think it would be prudent to do both in 9.0.
   
   It seems like you and @cpoerschke are in favor of removing `Response` altogether, just leaving `response`. I think it would be fine to change `Response` -> `message` and leave the information, but if y'all don't think it's useful I won't put up a fight.
   
   > I've checked all the sources searching "Response" entry and haven't found any pieces of evidence that value under this key is extracted or parsed somewhere. This means that from the perspective of view of Solr itself it seems that it is safe to replace original Response entry with response entry.
   
   That is very good news, thanks for doing that research!


-- 
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] ijioio edited a comment on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio edited a comment on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1010953596


   > It seems like you and @cpoerschke are in favor of removing Response altogether, just leaving response. I think it would be fine to change Response -> message and leave the information, but if y'all don't think it's useful I won't put up a fight.
   
   I'm OK with both ways. I probably more tended for the remove option because `toLog` list seems to be intended for logging purposes, not for building responses. I've checked the usages of it and it is always used for logging only (except the case of async calls of admin operations mentioned in this issue). There is also a case of using `toLog` to store `hits` value but it is used just as a value holder, not for the response. It is easy to see if you check the calls of `org.apache.solr.response.SolrQueryResponse.getToLog()` and `org.apache.solr.response.SolrQueryResponse.getToLogAsString(String)`. Another reason is that all this data appear as a response only in an internal subrequest data of async call, not in an outer initial request data, so internal `Request` data seems valuable for me only for debugging purposes. 


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1020275508


   @ijioio Let me know if you are ok with the name, i'll go ahead and backport to other branches. If not I can fix the name then backport!


-- 
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] HoustonPutman commented on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1018906578


   This is good to go, minus a ChangeLog entry.


-- 
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] ijioio edited a comment on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio edited a comment on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1002910206


   It is maybe seems a bit strange to hold two values `Response` and `response`, but I made it for two reasons:
   
   - I don't think that messing with the original `Response` value is a good idea. It is a general response format used for all the core admin operations. So there may be a chance of someone depending on its content/format.
   - In case we use some field other than `response` we also need to update the function `aggregateResults` to support two types of response formats for aggregation. In contrast, if we use the `response` field it will be compatible with current`aggregateResults` code.
   
   This is how response for backup status request will looks like in case of adding `response` field:
   
   ```json
   {
       "responseHeader": {
           "status": 0,
           "QTime": 19
       },
       "success": {
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 4
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 8
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
               "response": [
                   "startTime",
                   "2021-12-30T06:45:55.978702Z",
                   "indexFileCount",
                   1,
                   "uploadedIndexFileCount",
                   0,
                   "indexSizeMB",
                   0.001,
                   "uploadedIndexFileMB",
                   0.001,
                   "shard",
                   "shard2",
                   "endTime",
                   "2021-12-30T06:45:55.995335Z",
                   "shardBackupId",
                   "md_shard2_1"
               ]
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
               "response": [
                   "startTime",
                   "2021-12-30T06:46:10.266750Z",
                   "indexFileCount",
                   21,
                   "uploadedIndexFileCount",
                   21,
                   "indexSizeMB",
                   0.006,
                   "uploadedIndexFileMB",
                   0.006,
                   "shard",
                   "shard1",
                   "endTime",
                   "2021-12-30T06:46:10.438942Z",
                   "shardBackupId",
                   "md_shard1_1"
               ]
           }
       },
       "10562744829711078047": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
           "response": [
               "startTime",
               "2021-12-30T06:45:55.978702Z",
               "indexFileCount",
               1,
               "uploadedIndexFileCount",
               0,
               "indexSizeMB",
               0.001,
               "uploadedIndexFileMB",
               0.001,
               "shard",
               "shard2",
               "endTime",
               "2021-12-30T06:45:55.995335Z",
               "shardBackupId",
               "md_shard2_1"
           ]
       },
       "10562744829713680734": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///users/rh/solr-8.11.1/backuppp/myBackupName/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
           "response": [
               "startTime",
               "2021-12-30T06:46:10.266750Z",
               "indexFileCount",
               21,
               "uploadedIndexFileCount",
               21,
               "indexSizeMB",
               0.006,
               "uploadedIndexFileMB",
               0.006,
               "shard",
               "shard1",
               "endTime",
               "2021-12-30T06:46:10.438942Z",
               "shardBackupId",
               "md_shard1_1"
           ]
       },
       "response": [
           "collection",
           "techproducts",
           "numShards",
           2,
           "backupId",
           1,
           "indexVersion",
           "9.0.0",
           "startTime",
           "2021-12-30T06:45:35.738064Z",
           "indexSizeMB",
           0.007
       ],
       "status": {
           "state": "completed",
           "msg": "found [1056] in completed tasks"
       }
   }
   ```
   
   Another way to solve the problem, is to put results of the operation to the `toLog` list of the response. But this implies parsing the generated string located in the `Response` field and extracting all the required fields from it within the `aggregateResults` method. Looks unnecessary complex.


-- 
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] ijioio edited a comment on pull request #479: SOLR-15842: Fix async backup response

Posted by GitBox <gi...@apache.org>.
ijioio edited a comment on pull request #479:
URL: https://github.com/apache/solr/pull/479#issuecomment-1002910206


   It is maybe seems a bit strange to hold two values `Response` and `response`, but I made it for two reasons:
   
   - I don't think that messing with the original `Response` value is a good idea. It is a general response format used for all the core admin operations. So there may be a chance of someone depending on its content/format.
   - In case we use some field other than `response` we also need to update the function `aggregateResults` to support two types of response types for aggregation. In contrast, if we use the `response` field it will be compatible with current`aggregateResults` code.
   
   This is how response for backup status request will looks like in case of adding `response` field:
   
   ```json
   {
       "responseHeader": {
           "status": 0,
           "QTime": 19
       },
       "success": {
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 4
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 8
               }
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///path/to/my/shared/drive/mybackup/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
               "response": [
                   "startTime",
                   "2021-12-30T06:45:55.978702Z",
                   "indexFileCount",
                   1,
                   "uploadedIndexFileCount",
                   0,
                   "indexSizeMB",
                   0.001,
                   "uploadedIndexFileMB",
                   0.001,
                   "shard",
                   "shard2",
                   "endTime",
                   "2021-12-30T06:45:55.995335Z",
                   "shardBackupId",
                   "md_shard2_1"
               ]
           },
           "localhost:8983_solr": {
               "responseHeader": {
                   "status": 0,
                   "QTime": 0
               },
               "STATUS": "completed",
               "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///path/to/my/shared/drive/mybackup/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
               "response": [
                   "startTime",
                   "2021-12-30T06:46:10.266750Z",
                   "indexFileCount",
                   21,
                   "uploadedIndexFileCount",
                   21,
                   "indexSizeMB",
                   0.006,
                   "uploadedIndexFileMB",
                   0.006,
                   "shard",
                   "shard1",
                   "endTime",
                   "2021-12-30T06:46:10.438942Z",
                   "shardBackupId",
                   "md_shard1_1"
               ]
           }
       },
       "10562744829711078047": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829711078047 webapp=null path=/admin/cores params={core=techproducts_shard2_replica_n4&async=10562744829711078047&qt=/admin/cores&name=shard2&shardBackupId=md_shard2_1&action=BACKUPCORE&location=file:///path/to/my/shared/drive/mybackup/techproducts/&incremental=true&prevShardBackupId=md_shard2_0&wt=javabin&version=2} status=0 QTime=4",
           "response": [
               "startTime",
               "2021-12-30T06:45:55.978702Z",
               "indexFileCount",
               1,
               "uploadedIndexFileCount",
               0,
               "indexSizeMB",
               0.001,
               "uploadedIndexFileMB",
               0.001,
               "shard",
               "shard2",
               "endTime",
               "2021-12-30T06:45:55.995335Z",
               "shardBackupId",
               "md_shard2_1"
           ]
       },
       "10562744829713680734": {
           "responseHeader": {
               "status": 0,
               "QTime": 0
           },
           "STATUS": "completed",
           "Response": "TaskId: 10562744829713680734 webapp=null path=/admin/cores params={core=techproducts_shard1_replica_n1&async=10562744829713680734&qt=/admin/cores&name=shard1&shardBackupId=md_shard1_1&action=BACKUPCORE&location=file:///path/to/my/shared/drive/mybackup/techproducts/&incremental=true&prevShardBackupId=md_shard1_0&wt=javabin&version=2} status=0 QTime=8",
           "response": [
               "startTime",
               "2021-12-30T06:46:10.266750Z",
               "indexFileCount",
               21,
               "uploadedIndexFileCount",
               21,
               "indexSizeMB",
               0.006,
               "uploadedIndexFileMB",
               0.006,
               "shard",
               "shard1",
               "endTime",
               "2021-12-30T06:46:10.438942Z",
               "shardBackupId",
               "md_shard1_1"
           ]
       },
       "response": [
           "collection",
           "techproducts",
           "numShards",
           2,
           "backupId",
           1,
           "indexVersion",
           "9.0.0",
           "startTime",
           "2021-12-30T06:45:35.738064Z",
           "indexSizeMB",
           0.007
       ],
       "status": {
           "state": "completed",
           "msg": "found [1056] in completed tasks"
       }
   }
   ```
   
   Another way to solve the problem, is to put results of the operation to the `toLog` list of the response. But this implies parsing the generated string located in the `Response` field and extracting all the required fields from it within the `aggregateResults` method. Looks unnecessary complex.


-- 
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