Solr
  1. Solr
  2. SOLR-4028

When using ZK chroot, it would be nice if Solr would create the initial path when it doesn't exist.

    Details

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

      Description

      I think this would make it easier to test and develop with SolrCloud, in order to start with a fresh ZK directory now the approach is to delete ZK data, with this improvement one could just add a chroot to the zkHost like:

      java -DzkHost=localhost:2181/testXYZ -jar start.jar

      Right now this is possible but you have to manually create the initial path.

      1. SOLR-4028.patch
        12 kB
        Tomás Fernández Löbbe
      2. SOLR-4028.patch
        5 kB
        Tomás Fernández Löbbe

        Activity

        Hide
        Mark Miller added a comment -

        I've been considering the one for a while - last bit of feedback I got on it is that it would be better if we didn't do this automatically - that it's better to have to be explicit. I think I still lean towards doing it though.

        Show
        Mark Miller added a comment - I've been considering the one for a while - last bit of feedback I got on it is that it would be better if we didn't do this automatically - that it's better to have to be explicit. I think I still lean towards doing it though.
        Hide
        Tomás Fernández Löbbe added a comment -

        What's the argument against doing this automatically? I'm not a ZK expert so I may be missing something, but I do think it would make it easier while developing and testing, as you don't need to start zkClient or anything like that to create these directories, and you don't need delete zk data directory, restart zk nodes, etc.

        Show
        Tomás Fernández Löbbe added a comment - What's the argument against doing this automatically? I'm not a ZK expert so I may be missing something, but I do think it would make it easier while developing and testing, as you don't need to start zkClient or anything like that to create these directories, and you don't need delete zk data directory, restart zk nodes, etc.
        Hide
        Tomás Fernández Löbbe added a comment -

        I think I see the issue here, the problem would be if someone mistype the initial path, instead of throwing exceptions and stopping, we would be creating a new path and probably hiding an error.
        However, we do create paths for overseer and upload configs automatically, I think creating the initial path is more consistent with the current behavior than stopping startup. Other options I thought are:
        • Only create the initial path when bootstrap_conf is true (or bootstrap_confdir). This could still have the same issue described above.
        • Add a new parameter to force creation, something like –DzkHost.create=true. This could add unnecessary parameters and configuration complexity.

        Show
        Tomás Fernández Löbbe added a comment - I think I see the issue here, the problem would be if someone mistype the initial path, instead of throwing exceptions and stopping, we would be creating a new path and probably hiding an error. However, we do create paths for overseer and upload configs automatically, I think creating the initial path is more consistent with the current behavior than stopping startup. Other options I thought are: • Only create the initial path when bootstrap_conf is true (or bootstrap_confdir). This could still have the same issue described above. • Add a new parameter to force creation, something like –DzkHost.create=true. This could add unnecessary parameters and configuration complexity.
        Hide
        Mark Miller added a comment -

        Yeah, I think that was perhaps the concern - basically, it seems ops type people prefer being explicit. Other paths are auto-created, but they are not arbitrary paths supplied by the user as a connect string - I guess it's a little different. If you are trying to connect to an existing node and type something wrong, you just create a new one rather than getting an error.

        I don't know what's best, but like I said, I guess I lean towards auto creating.

        Show
        Mark Miller added a comment - Yeah, I think that was perhaps the concern - basically, it seems ops type people prefer being explicit. Other paths are auto-created, but they are not arbitrary paths supplied by the user as a connect string - I guess it's a little different. If you are trying to connect to an existing node and type something wrong, you just create a new one rather than getting an error. I don't know what's best, but like I said, I guess I lean towards auto creating.
        Hide
        Yonik Seeley added a comment -

        I think I see the issue here, the problem would be if someone mistype the initial path, instead of throwing exceptions and stopping, we would be creating a new path and probably hiding an error.

        That can go the other direction too? A config could be created under /solr and then someone could try to join it by forgetting to specify that root in zkHost.

        Only create the initial path when bootstrap_conf is true (or bootstrap_confdir).

        As long as we need some sort of explicit bootstrap, that seems reasonable.

        Add a new parameter to force creation, something like –DzkHost.create=true.

        Anything that creates a skeleton layout of a new cluster should work the same (auto-create the rot if it doesn't exist). "ZkCLI -cmd bootstrap" for example. Not sure if there are others.

        Show
        Yonik Seeley added a comment - I think I see the issue here, the problem would be if someone mistype the initial path, instead of throwing exceptions and stopping, we would be creating a new path and probably hiding an error. That can go the other direction too? A config could be created under /solr and then someone could try to join it by forgetting to specify that root in zkHost. Only create the initial path when bootstrap_conf is true (or bootstrap_confdir). As long as we need some sort of explicit bootstrap, that seems reasonable. Add a new parameter to force creation, something like –DzkHost.create=true. Anything that creates a skeleton layout of a new cluster should work the same (auto-create the rot if it doesn't exist). "ZkCLI -cmd bootstrap" for example. Not sure if there are others.
        Hide
        Tomás Fernández Löbbe added a comment -

        That can go the other direction too? A config could be created under /solr and then someone could try to join it by forgetting to specify that root in zkHost.

        This can happen today too

        Anything that creates a skeleton layout of a new cluster should work the same (auto-create the rot if it doesn't exist). "ZkCLI -cmd bootstrap" for example. Not sure if there are others.

        Yes, I agree

        Show
        Tomás Fernández Löbbe added a comment - That can go the other direction too? A config could be created under /solr and then someone could try to join it by forgetting to specify that root in zkHost. This can happen today too Anything that creates a skeleton layout of a new cluster should work the same (auto-create the rot if it doesn't exist). "ZkCLI -cmd bootstrap" for example. Not sure if there are others. Yes, I agree
        Hide
        Tomás Fernández Löbbe added a comment -

        With this patch, the initial path is created only when bootstrap_conf or boostrap_confdir are specified.
        ZkCli also creates the initial path when the command is upconfig or bootstrap are used

        Show
        Tomás Fernández Löbbe added a comment - With this patch, the initial path is created only when bootstrap_conf or boostrap_confdir are specified. ZkCli also creates the initial path when the command is upconfig or bootstrap are used
        Hide
        Mark Miller added a comment -

        Cool, thanks.

        Show
        Mark Miller added a comment - Cool, thanks.
        Hide
        Commit Tag Bot added a comment -

        [trunk commit] Mark Robert Miller
        http://svn.apache.org/viewvc?view=revision&revision=1419866

        SOLR-4028: When using ZK chroot, it would be nice if Solr would create the initial path when it doesn't exist.

        Show
        Commit Tag Bot added a comment - [trunk commit] Mark Robert Miller http://svn.apache.org/viewvc?view=revision&revision=1419866 SOLR-4028 : When using ZK chroot, it would be nice if Solr would create the initial path when it doesn't exist.
        Hide
        Mark Miller added a comment -

        Thanks Tomas! Great patch! I've committed it.

        Show
        Mark Miller added a comment - Thanks Tomas! Great patch! I've committed it.
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Mark Robert Miller
        http://svn.apache.org/viewvc?view=revision&revision=1419870

        SOLR-4028: When using ZK chroot, it would be nice if Solr would create the initial path when it doesn't exist.

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Mark Robert Miller http://svn.apache.org/viewvc?view=revision&revision=1419870 SOLR-4028 : When using ZK chroot, it would be nice if Solr would create the initial path when it doesn't exist.

          People

          • Assignee:
            Mark Miller
            Reporter:
            Tomás Fernández Löbbe
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development