Solr
  1. Solr
  2. SOLR-6115

Cleanup enum/string action types in Overseer, OverseerCollectionProcessor and CollectionHandler

    Details

    • Type: Task Task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.0, 6.0
    • Component/s: SolrCloud
    • Labels:
      None

      Description

      The enum/string handling for actions in Overseer and OCP is a mess. We should fix it.

      From: https://issues.apache.org/jira/browse/SOLR-5466?focusedCommentId=13918059&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13918059

      I started to untangle the fact that we have all the strings in OverseerCollectionProcessor, but also have a nice CollectionAction enum. And the commands are intermingled with parameters, it all seems rather confusing. Does it make sense to use the enum rather than the strings? Or somehow associate the two? Probably something for another JIRA though...

      1. SOLR-6115.patch
        59 kB
        Shalin Shekhar Mangar
      2. SOLR-6115-branch_4x.patch
        60 kB
        Shalin Shekhar Mangar

        Activity

        Hide
        Shalin Shekhar Mangar added a comment -
        1. Refactor Overseer, OverseerCollectionProcessor and CollectionHandler to use enums instead of strings
        2. Created a new enum called OverseerAction which has actions specific to Overseer. I didn't want to expose them in the CollectionAction enum which is public and part of SolrJ.
        3. Found and changed all references in code and tests.
        4. Renamed "createcollection" to "create", "removecollection" to "delete" and "removeshard" to "deleteshard" to match corresponding CollectionAction enums. This has the side effect that the keys for these stats in the "overseerstatus" API are also renamed but I think that should be okay. I've documented this compat break in the change log. We continue to handle existing items in the overseer queue to preserve back-compat during rolling upgrades.
        Show
        Shalin Shekhar Mangar added a comment - Refactor Overseer, OverseerCollectionProcessor and CollectionHandler to use enums instead of strings Created a new enum called OverseerAction which has actions specific to Overseer. I didn't want to expose them in the CollectionAction enum which is public and part of SolrJ. Found and changed all references in code and tests. Renamed "createcollection" to "create", "removecollection" to "delete" and "removeshard" to "deleteshard" to match corresponding CollectionAction enums. This has the side effect that the keys for these stats in the "overseerstatus" API are also renamed but I think that should be okay. I've documented this compat break in the change log. We continue to handle existing items in the overseer queue to preserve back-compat during rolling upgrades.
        Hide
        Shalin Shekhar Mangar added a comment -

        All tests and precommit pass. I'll commit this soon.

        Show
        Shalin Shekhar Mangar added a comment - All tests and precommit pass. I'll commit this soon.
        Hide
        ASF subversion and git services added a comment -

        Commit 1625891 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1625891 ]

        SOLR-6115: Cleanup enum/string action types in Overseer, OverseerCollectionProcessor and CollectionHandler

        Show
        ASF subversion and git services added a comment - Commit 1625891 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1625891 ] SOLR-6115 : Cleanup enum/string action types in Overseer, OverseerCollectionProcessor and CollectionHandler
        Hide
        ASF subversion and git services added a comment -

        Commit 1625897 from shalin@apache.org in branch 'dev/trunk'
        [ https://svn.apache.org/r1625897 ]

        SOLR-6115: Fix enum usage in DeleteReplicaTest

        Show
        ASF subversion and git services added a comment - Commit 1625897 from shalin@apache.org in branch 'dev/trunk' [ https://svn.apache.org/r1625897 ] SOLR-6115 : Fix enum usage in DeleteReplicaTest
        Hide
        Shalin Shekhar Mangar added a comment -

        This patch is for branch_4x (it is different because SOLR-5473 hasn't been merged to 4x yet)

        Show
        Shalin Shekhar Mangar added a comment - This patch is for branch_4x (it is different because SOLR-5473 hasn't been merged to 4x yet)
        Hide
        ASF subversion and git services added a comment -

        Commit 1625903 from shalin@apache.org in branch 'dev/branches/branch_4x'
        [ https://svn.apache.org/r1625903 ]

        SOLR-6115: Cleanup enum/string action types in Overseer, OverseerCollectionProcessor and CollectionHandler

        Show
        ASF subversion and git services added a comment - Commit 1625903 from shalin@apache.org in branch 'dev/branches/branch_4x' [ https://svn.apache.org/r1625903 ] SOLR-6115 : Cleanup enum/string action types in Overseer, OverseerCollectionProcessor and CollectionHandler
        Hide
        Shalin Shekhar Mangar added a comment -

        Thanks Erick for the original suggestion of fixing this mess

        Show
        Shalin Shekhar Mangar added a comment - Thanks Erick for the original suggestion of fixing this mess
        Hide
        Erick Erickson added a comment -

        And thank you. I just ran into all this again yesterday, I had stuff scattered all over the place for some new functionality and thought "that makes no sense, I'll...". So I fixed up the bits that I had been added, probably should have waited a day ....

        It'll be cool to have all this straightened out! I'm sure there were places all over...

        Show
        Erick Erickson added a comment - And thank you . I just ran into all this again yesterday, I had stuff scattered all over the place for some new functionality and thought "that makes no sense, I'll...". So I fixed up the bits that I had been added, probably should have waited a day .... It'll be cool to have all this straightened out! I'm sure there were places all over...
        Hide
        Shalin Shekhar Mangar added a comment -

        Yeah, I think we can do better than what I did but it's a start. Please feel free to re-factor as you see fit.

        Show
        Shalin Shekhar Mangar added a comment - Yeah, I think we can do better than what I did but it's a start. Please feel free to re-factor as you see fit.
        Hide
        Anshum Gupta added a comment -

        Bulk close after 5.0 release.

        Show
        Anshum Gupta added a comment - Bulk close after 5.0 release.

          People

          • Assignee:
            Shalin Shekhar Mangar
            Reporter:
            Shalin Shekhar Mangar
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development