Solr
  1. Solr
  2. SOLR-8083

Convert the ZookeeperInfoServlet to a handler at /admin/zookeeper

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.4, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      There is no need to have an extra servlet for this purpose

      By keeping this outside of Solr we cannot even secure it properly using the security framework and now we have taken up a top level name called zookeeper which cannot be used as a collection name

        Issue Links

          Activity

          Hide
          Noble Paul added a comment -

          Upayavira your inputs are welcome

          Show
          Noble Paul added a comment - Upayavira your inputs are welcome
          Hide
          Upayavira added a comment -

          Yes please!!

          The request I have is that this abstract out functions that Zookeeper offers, so:

          • please tell me the list of live nodes
          • please tell me the state of this collection

          We get real craziness when the UI depends upon clusterstate.json, which is replaced by state.json, and then (I assume) the ZookeeperServlet has to rebuild a clusterstate.json file, pretending that it exists in its file system, to keep the UI happy.

          For the tree view, we still need the ability to list the contents of nodes, but I'd rather we only ever use that for the tree view.

          I can look through the UI source, and give you a list of the API endpoints I would love to have. I doubt they will be many or hard to implement. Would that work?

          Show
          Upayavira added a comment - Yes please!! The request I have is that this abstract out functions that Zookeeper offers, so: please tell me the list of live nodes please tell me the state of this collection We get real craziness when the UI depends upon clusterstate.json, which is replaced by state.json, and then (I assume) the ZookeeperServlet has to rebuild a clusterstate.json file, pretending that it exists in its file system, to keep the UI happy. For the tree view, we still need the ability to list the contents of nodes, but I'd rather we only ever use that for the tree view. I can look through the UI source, and give you a list of the API endpoints I would love to have. I doubt they will be many or hard to implement. Would that work?
          Hide
          Noble Paul added a comment -

          Thanks,

          I would like to make the whole thing split into two

          1. get rid of the servlet and make it just a normal request handler. This step would just return the same output as it used to. So, really nothing changes other than the path at which this is accessed
          2. Enhance the standard APIs in such a way that the admin UI can be completely driven using them. The standard APIs will have proper JUnit tests and a well known output. The current output is driven by the UI and not the other way around.

          I would say we open a ticket for #2 and track it separately and the admin ui should eventually stop depending on the /admin/zookeeper end point.

          Show
          Noble Paul added a comment - Thanks, I would like to make the whole thing split into two get rid of the servlet and make it just a normal request handler. This step would just return the same output as it used to. So, really nothing changes other than the path at which this is accessed Enhance the standard APIs in such a way that the admin UI can be completely driven using them. The standard APIs will have proper JUnit tests and a well known output. The current output is driven by the UI and not the other way around. I would say we open a ticket for #2 and track it separately and the admin ui should eventually stop depending on the /admin/zookeeper end point.
          Hide
          Upayavira added a comment -

          I'm in support of that separation.

          How would you expect the admin UI to implement the tree browser without using the /admin/zookeeper API?

          I do see a number of situations where functionality has been placed one side or the other based upon what people know how to do, rather than on where it should be. It'd be great to fix those.

          Regarding /zookeeper, that is sufficiently fundamental a feature that we must make such a change work across both the old and new UIs. Using your new API (#2) could be done in the new UI only.

          Show
          Upayavira added a comment - I'm in support of that separation. How would you expect the admin UI to implement the tree browser without using the /admin/zookeeper API? I do see a number of situations where functionality has been placed one side or the other based upon what people know how to do, rather than on where it should be. It'd be great to fix those. Regarding /zookeeper, that is sufficiently fundamental a feature that we must make such a change work across both the old and new UIs. Using your new API (#2) could be done in the new UI only.
          Hide
          Noble Paul added a comment -

          This change is not for V2 or new UI

          This is largely a cleanup process. Solr should not need servlets .

          How would you expect the admin UI to implement the tree browser without using the /admin/zookeeper API?

          The tree browser will need some Zookeeper API. But whatever it is , it should be an API which is not UI centric . I see stuff like href=zookeeper?xxx which is clearly something the API should never require. Again, I'm not proposing that change in this ticket

          Show
          Noble Paul added a comment - This change is not for V2 or new UI This is largely a cleanup process. Solr should not need servlets . How would you expect the admin UI to implement the tree browser without using the /admin/zookeeper API? The tree browser will need some Zookeeper API. But whatever it is , it should be an API which is not UI centric . I see stuff like href=zookeeper?xxx which is clearly something the API should never require. Again, I'm not proposing that change in this ticket
          Hide
          Upayavira added a comment -

          If in this ticket you are simply proposing to replace /zookeeper with /admin/zookeeper, implemented as a RequestHandler but supporting the same API, then I am fully in support of that, and will do the UI side work.

          I agree wholeheartedly with the API not being UI centric - that's what I was referring to above - work being done on the wrong side of the API, and would love to see that all cleaned up - and again, I'll do the UI side work to make it happen.

          Show
          Upayavira added a comment - If in this ticket you are simply proposing to replace /zookeeper with /admin/zookeeper, implemented as a RequestHandler but supporting the same API, then I am fully in support of that, and will do the UI side work. I agree wholeheartedly with the API not being UI centric - that's what I was referring to above - work being done on the wrong side of the API, and would love to see that all cleaned up - and again, I'll do the UI side work to make it happen.
          Hide
          Noble Paul added a comment -

          work being done on the wrong side of the API

          I shall open a ticket for the same. You please dcument the input params and a sample output you would need and the params and I shall add the API

          Show
          Noble Paul added a comment - work being done on the wrong side of the API I shall open a ticket for the same. You please dcument the input params and a sample output you would need and the params and I shall add the API
          Hide
          Noble Paul added a comment -

          Upayavira I've opened SOLR-8100

          Show
          Noble Paul added a comment - Upayavira I've opened SOLR-8100
          Hide
          Shalin Shekhar Mangar added a comment -
          • please tell me the list of live nodes
          • please tell me the state of this collection

          The clusterstatus API tells you those things already.

          Show
          Shalin Shekhar Mangar added a comment - please tell me the list of live nodes please tell me the state of this collection The clusterstatus API tells you those things already.
          Hide
          ASF subversion and git services added a comment -

          Commit 1705662 from Noble Paul in branch 'dev/trunk'
          [ https://svn.apache.org/r1705662 ]

          SOLR-8083: convert the ZookeeperInfoServlet to a request handler at /admin/zookeeper

          Show
          ASF subversion and git services added a comment - Commit 1705662 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1705662 ] SOLR-8083 : convert the ZookeeperInfoServlet to a request handler at /admin/zookeeper
          Hide
          ASF subversion and git services added a comment -

          Commit 1705683 from Noble Paul in branch 'dev/trunk'
          [ https://svn.apache.org/r1705683 ]

          SOLR-8083: convert the ZookeeperInfoServlet to a request handler at /admin/zookeeper

          Show
          ASF subversion and git services added a comment - Commit 1705683 from Noble Paul in branch 'dev/trunk' [ https://svn.apache.org/r1705683 ] SOLR-8083 : convert the ZookeeperInfoServlet to a request handler at /admin/zookeeper

            People

            • Assignee:
              Noble Paul
              Reporter:
              Noble Paul
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development