Uploaded image for project: 'Data Management'
  1. Data Management
  2. DM-1901

Re-implement data loading scripts based on new worker control service

    XMLWordPrintable

    Details

    • Type: Story
    • Status: Done
    • Resolution: Done
    • Fix Version/s: None
    • Component/s: Qserv
    • Labels:
      None

      Description

      Once we have new service that controls worker communication we'll need to reimplement WorkerAdmin class based on that.

        Attachments

          Issue Links

            Activity

            Hide
            salnikov Andy Salnikov added a comment -

            Fabrice, Vaikunth, could you review the changes when you have time for it. This ticket changes both qserv and qserv_testdata (latter is not on github and does not have PR).

            Fabrice, this touches few things that may potentially impact what you are doing with cluster, I removed bunch of ssh-related code from NodeAdmin and I think I updated all dependencies but please check.

            Vaikunth, this will certainly have impact on what you are doing with multi-node setup, please look at the changes in qserv_testdata (but feel free to look at qserv PR as well).

            If you want to split this review between two of you I'd suggest:

            • Vaikunth - qserv_testdata and qserv/core/modules/wmgr/*
            • Fabrice - qserv/admin/*
            Show
            salnikov Andy Salnikov added a comment - Fabrice, Vaikunth, could you review the changes when you have time for it. This ticket changes both qserv and qserv_testdata (latter is not on github and does not have PR). Fabrice, this touches few things that may potentially impact what you are doing with cluster, I removed bunch of ssh-related code from NodeAdmin and I think I updated all dependencies but please check. Vaikunth, this will certainly have impact on what you are doing with multi-node setup, please look at the changes in qserv_testdata (but feel free to look at qserv PR as well). If you want to split this review between two of you I'd suggest: Vaikunth - qserv_testdata and qserv/core/modules/wmgr/* Fabrice - qserv/admin/*
            Hide
            jammes Fabrice Jammes added a comment -

            Hi Andy,

            FYI, I've added some comments to the PR.

            Have a nice day

            Show
            jammes Fabrice Jammes added a comment - Hi Andy, FYI, I've added some comments to the PR. Have a nice day
            Hide
            jammes Fabrice Jammes added a comment -

            ok thanks.

            Show
            jammes Fabrice Jammes added a comment - ok thanks.
            Hide
            salnikov Andy Salnikov added a comment -

            Fabrice, I have pushed the updates that I made in response to your comments. If you are happy with review you can remove yourself from reviewers.

            Show
            salnikov Andy Salnikov added a comment - Fabrice, I have pushed the updates that I made in response to your comments. If you are happy with review you can remove yourself from reviewers.
            Hide
            salnikov Andy Salnikov added a comment -

            Vaikunth, I removed Fabrice, it's yours to review now.

            Show
            salnikov Andy Salnikov added a comment - Vaikunth, I removed Fabrice, it's yours to review now.
            Hide
            vaikunth Vaikunth Thukral added a comment -

            Hi Andy,

            I had some comments on the wmgr/* changes on github, and all the changes in qserv_testdata make sense. In fact I really like that wmgr now seems to be able to do a lot of the stuff I was having trouble adapting to multi-node automatically, which is really essential.

            I have no comments on the qserv_testdata changes (since they are mostly syntactical), but are the integration tests working with these changes? I am not familiar enough with the new interface to say by just looking at the code since I still have questions regarding how the wmgr interface and connections are being handled, and if they will behave differently when doing it in multi-node (which is what happened before).

            For example, before your changes something like sqlInterface.execute() was used to create/drop DBs in mono-node. I had to update that in multi-node to run with the old workerAdmin class connection with something like workerDbConn.execCommanN(). Now it looks like there is just a "connection" and there should be no difference between mono or multi-node? In addition, I think I will need your help in understanding how to create these new wmgrs and if there is any difference in registering them in CSS etc.

            Show
            vaikunth Vaikunth Thukral added a comment - Hi Andy, I had some comments on the wmgr/* changes on github, and all the changes in qserv_testdata make sense. In fact I really like that wmgr now seems to be able to do a lot of the stuff I was having trouble adapting to multi-node automatically, which is really essential. I have no comments on the qserv_testdata changes (since they are mostly syntactical), but are the integration tests working with these changes? I am not familiar enough with the new interface to say by just looking at the code since I still have questions regarding how the wmgr interface and connections are being handled, and if they will behave differently when doing it in multi-node (which is what happened before). For example, before your changes something like sqlInterface.execute() was used to create/drop DBs in mono-node. I had to update that in multi-node to run with the old workerAdmin class connection with something like workerDbConn.execCommanN(). Now it looks like there is just a "connection" and there should be no difference between mono or multi-node? In addition, I think I will need your help in understanding how to create these new wmgrs and if there is any difference in registering them in CSS etc.
            Hide
            salnikov Andy Salnikov added a comment -

            Thanks Vaikunth. The client code indeed is supposed to be simpler now, you have one clean (I hope) interface to wmgr which takes care of the most operations. It works transparently with worker and mater databases. I updated integration test code so you can use as an example for communication with workers as well (and we should talk when you are at SLAC).
            Integration tests work OK with these changes, they could be slightly slower (few percent level) as data loading is now done via HTTP rather than direct mysql connection.

            Show
            salnikov Andy Salnikov added a comment - Thanks Vaikunth. The client code indeed is supposed to be simpler now, you have one clean (I hope) interface to wmgr which takes care of the most operations. It works transparently with worker and mater databases. I updated integration test code so you can use as an example for communication with workers as well (and we should talk when you are at SLAC). Integration tests work OK with these changes, they could be slightly slower (few percent level) as data loading is now done via HTTP rather than direct mysql connection.
            Hide
            salnikov Andy Salnikov added a comment -

            Merged and pushed both qserv and testdata. Done

            Show
            salnikov Andy Salnikov added a comment - Merged and pushed both qserv and testdata. Done

              People

              Assignee:
              salnikov Andy Salnikov
              Reporter:
              salnikov Andy Salnikov
              Reviewers:
              Vaikunth Thukral
              Watchers:
              Andy Salnikov, Fabrice Jammes, Vaikunth Thukral
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  CI Builds

                  No builds found.