Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.0.0
    • Component/s: None
    • Labels:
      None

      Description

      ZooKeeper 3.5.0 added a new feature "Dynamic Reconfiguration". Curator needs to support this.

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/curator/pull/76

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/curator/pull/76
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/curator/pull/53

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/curator/pull/53
        Hide
        randgalt Jordan Zimmerman added a comment -

        https://github.com/apache/curator/pull/76 has been updated with a more compatible version of the APIs. The tests still pass. More tests are needed but I think we're mostly there now.

        Show
        randgalt Jordan Zimmerman added a comment - https://github.com/apache/curator/pull/76 has been updated with a more compatible version of the APIs. The tests still pass. More tests are needed but I think we're mostly there now.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-99323847

        There's a new master tracking branch ref'd by #76

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-99323847 There's a new master tracking branch ref'd by #76
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user kondratovich commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-90881347

        ```java
        // org/apache/curator/framework/imps/GetConfigBuilderImpl.java#52
        @Override
        public Ensembleable<byte[]> storingStatIn(Stat stat)

        { this.stat = new Stat(); return this; }

        ````

        Seems you are ignoring specified `Stat` object.

        Show
        githubbot ASF GitHub Bot added a comment - Github user kondratovich commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-90881347 ```java // org/apache/curator/framework/imps/GetConfigBuilderImpl.java#52 @Override public Ensembleable<byte[]> storingStatIn(Stat stat) { this.stat = new Stat(); return this; } ```` Seems you are ignoring specified `Stat` object.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user iocanel commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-90658537

        Yeah,

        I think that I’ll have it in by the end of the week.

        > On Apr 7, 2015, at 8:22 PM, Andrew Kondratovich <notifications@github.com> wrote:
        >
        > Hi, any update on this?
        >
        > —
        > Reply to this email directly or view it on GitHub <https://github.com/apache/curator/pull/53#issuecomment-90653266>.
        >

        Show
        githubbot ASF GitHub Bot added a comment - Github user iocanel commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-90658537 Yeah, I think that I’ll have it in by the end of the week. > On Apr 7, 2015, at 8:22 PM, Andrew Kondratovich <notifications@github.com> wrote: > > Hi, any update on this? > > — > Reply to this email directly or view it on GitHub < https://github.com/apache/curator/pull/53#issuecomment-90653266 >. >
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user kondratovich commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-90653266

        Hi, any update on this?

        Show
        githubbot ASF GitHub Bot added a comment - Github user kondratovich commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-90653266 Hi, any update on this?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user iocanel commented on a diff in the pull request:

        https://github.com/apache/curator/pull/53#discussion_r20758041

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java —
        @@ -0,0 +1,29 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api;
        +
        +public interface Configurable<T> {
        +
        + /**
        + * Sets the configuration version to use.
        + * @param config The version of the configuration.
        + * @throws Exception
        + */
        + Ensembleable<T> fromConfig(long config) throws Exception;
        — End diff –

        Right. Let me fix that!

        Show
        githubbot ASF GitHub Bot added a comment - Github user iocanel commented on a diff in the pull request: https://github.com/apache/curator/pull/53#discussion_r20758041 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java — @@ -0,0 +1,29 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api; + +public interface Configurable<T> { + + /** + * Sets the configuration version to use. + * @param config The version of the configuration. + * @throws Exception + */ + Ensembleable<T> fromConfig(long config) throws Exception; — End diff – Right. Let me fix that!
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/53#discussion_r20757865

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java —
        @@ -0,0 +1,29 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api;
        +
        +public interface Configurable<T> {
        +
        + /**
        + * Sets the configuration version to use.
        + * @param config The version of the configuration.
        + * @throws Exception
        + */
        + Ensembleable<T> fromConfig(long config) throws Exception;
        — End diff –

        It's optional. Here's the Javadoc comment:

        ```

        • @param fromConfig
        • version of the current configuration (optional - causes reconfiguration to throw an exception if configuration is no longer current)
          ```

        Also, in the ZK code,

        ```
        long configId = reconfigRequest.getCurConfigId();

        if (configId Unable to render embedded object: File (= -1 && configId) not found.=lzks.self.getLastSeenQuorumVerifier().getVersion()){
        ```

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/53#discussion_r20757865 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java — @@ -0,0 +1,29 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api; + +public interface Configurable<T> { + + /** + * Sets the configuration version to use. + * @param config The version of the configuration. + * @throws Exception + */ + Ensembleable<T> fromConfig(long config) throws Exception; — End diff – It's optional. Here's the Javadoc comment: ``` @param fromConfig version of the current configuration (optional - causes reconfiguration to throw an exception if configuration is no longer current) ``` Also, in the ZK code, ``` long configId = reconfigRequest.getCurConfigId(); if (configId Unable to render embedded object: File (= -1 && configId) not found. =lzks.self.getLastSeenQuorumVerifier().getVersion()){ ```
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user iocanel commented on a diff in the pull request:

        https://github.com/apache/curator/pull/53#discussion_r20755878

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java —
        @@ -0,0 +1,29 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api;
        +
        +public interface Configurable<T> {
        +
        + /**
        + * Sets the configuration version to use.
        + * @param config The version of the configuration.
        + * @throws Exception
        + */
        + Ensembleable<T> fromConfig(long config) throws Exception;
        — End diff –

        Hmmm, my understanding was that the configuration version was required by ZooKeeper itself. AFAIR, not providing a valid configuration version always resulted in error. Are you sure that its optional?

        Show
        githubbot ASF GitHub Bot added a comment - Github user iocanel commented on a diff in the pull request: https://github.com/apache/curator/pull/53#discussion_r20755878 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java — @@ -0,0 +1,29 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api; + +public interface Configurable<T> { + + /** + * Sets the configuration version to use. + * @param config The version of the configuration. + * @throws Exception + */ + Ensembleable<T> fromConfig(long config) throws Exception; — End diff – Hmmm, my understanding was that the configuration version was required by ZooKeeper itself. AFAIR, not providing a valid configuration version always resulted in error. Are you sure that its optional?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on a diff in the pull request:

        https://github.com/apache/curator/pull/53#discussion_r20752095

        — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java —
        @@ -0,0 +1,29 @@
        +/**
        + * Licensed to the Apache Software Foundation (ASF) under one
        + * or more contributor license agreements. See the NOTICE file
        + * distributed with this work for additional information
        + * regarding copyright ownership. The ASF licenses this file
        + * to you under the Apache License, Version 2.0 (the
        + * "License"); you may not use this file except in compliance
        + * with the License. You may obtain a copy of the License at
        + *
        + * http://www.apache.org/licenses/LICENSE-2.0
        + *
        + * Unless required by applicable law or agreed to in writing,
        + * software distributed under the License is distributed on an
        + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
        + * KIND, either express or implied. See the License for the
        + * specific language governing permissions and limitations
        + * under the License.
        + */
        +package org.apache.curator.framework.api;
        +
        +public interface Configurable<T> {
        +
        + /**
        + * Sets the configuration version to use.
        + * @param config The version of the configuration.
        + * @throws Exception
        + */
        + Ensembleable<T> fromConfig(long config) throws Exception;
        — End diff –

        right now, fromConfig() is required. It should be optional. i.e. I should be able to write:

        ```
        client.reconfig().leaving("").joining("").forEnsemble();
        ```

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/53#discussion_r20752095 — Diff: curator-framework/src/main/java/org/apache/curator/framework/api/Configurable.java — @@ -0,0 +1,29 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.curator.framework.api; + +public interface Configurable<T> { + + /** + * Sets the configuration version to use. + * @param config The version of the configuration. + * @throws Exception + */ + Ensembleable<T> fromConfig(long config) throws Exception; — End diff – right now, fromConfig() is required. It should be optional. i.e. I should be able to write: ``` client.reconfig().leaving("").joining("").forEnsemble(); ```
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user iocanel commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-62555120

        I've pushed additional stuff in my repo, that also contain the
        EnsembleProvider and Tracker.

        I can force push to the apache/CURATOR-160 if you'd like.

        On Tue, Nov 11, 2014 at 3:13 PM, Jordan Zimmerman <notifications@github.com>
        wrote:

        > I can't pull the new changes. I'll just pull from your personal repo I
        > guess.
        >
        > —
        > Reply to this email directly or view it on GitHub
        > <https://github.com/apache/curator/pull/53#issuecomment-62544636>.
        >


        Ioannis Canellos

        Blog: http://iocanel.blogspot.com <http://iocanel.blogspot.com/>
        Twitter: iocanel

        Show
        githubbot ASF GitHub Bot added a comment - Github user iocanel commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-62555120 I've pushed additional stuff in my repo, that also contain the EnsembleProvider and Tracker. I can force push to the apache/ CURATOR-160 if you'd like. On Tue, Nov 11, 2014 at 3:13 PM, Jordan Zimmerman <notifications@github.com> wrote: > I can't pull the new changes. I'll just pull from your personal repo I > guess. > > — > Reply to this email directly or view it on GitHub > < https://github.com/apache/curator/pull/53#issuecomment-62544636 >. > – Ioannis Canellos Blog: http://iocanel.blogspot.com < http://iocanel.blogspot.com/ > Twitter: iocanel
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-62544636

        I can't pull the new changes. I'll just pull from your personal repo I guess.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-62544636 I can't pull the new changes. I'll just pull from your personal repo I guess.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user iocanel commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-62544542

        Yes, I did push to my personal repo so that the pr gets updated.

        On Tue, Nov 11, 2014 at 3:07 PM, Jordan Zimmerman <notifications@github.com>
        wrote:

        > I think you've only pushed to your personal repo. Please push to the
        > apache repo too.
        >
        > —
        > Reply to this email directly or view it on GitHub
        > <https://github.com/apache/curator/pull/53#issuecomment-62544094>.
        >


        Ioannis Canellos

        Blog: http://iocanel.blogspot.com <http://iocanel.blogspot.com/>
        Twitter: iocanel

        Show
        githubbot ASF GitHub Bot added a comment - Github user iocanel commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-62544542 Yes, I did push to my personal repo so that the pr gets updated. On Tue, Nov 11, 2014 at 3:07 PM, Jordan Zimmerman <notifications@github.com> wrote: > I think you've only pushed to your personal repo. Please push to the > apache repo too. > > — > Reply to this email directly or view it on GitHub > < https://github.com/apache/curator/pull/53#issuecomment-62544094 >. > – Ioannis Canellos Blog: http://iocanel.blogspot.com < http://iocanel.blogspot.com/ > Twitter: iocanel
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-62544094

        I think you've only pushed to your personal repo. Please push to the apache repo too.

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-62544094 I think you've only pushed to your personal repo. Please push to the apache repo too.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user iocanel commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-62510702

        I squashed the commits locally and pushed. I haven't pushed yet the watcher fix on GetConfigBuilderImpl and also anything related to the DynamicEnsembleProvider.

        Show
        githubbot ASF GitHub Bot added a comment - Github user iocanel commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-62510702 I squashed the commits locally and pushed. I haven't pushed yet the watcher fix on GetConfigBuilderImpl and also anything related to the DynamicEnsembleProvider.
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user Randgalt commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-62473684

        I'm not seeing the latest code. Did you push it?

        Show
        githubbot ASF GitHub Bot added a comment - Github user Randgalt commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-62473684 I'm not seeing the latest code. Did you push it?
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user iocanel commented on the pull request:

        https://github.com/apache/curator/pull/53#issuecomment-62378548

        After the detailed feedback, I applied all the necessary changes and squashed.

        Show
        githubbot ASF GitHub Bot added a comment - Github user iocanel commented on the pull request: https://github.com/apache/curator/pull/53#issuecomment-62378548 After the detailed feedback, I applied all the necessary changes and squashed.
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user iocanel opened a pull request:

        https://github.com/apache/curator/pull/53

        CURATOR-160 Add builders and dsl for ZooKeeper's config and reconfig m...

        ...ethods.

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/iocanel/curator CURATOR-160

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/curator/pull/53.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #53


        commit 2c576dc344a167ad4a72d71412c98d76ff4e2d3d
        Author: Ioannis Canellos <iocanel@gmail.com>
        Date: 2014-11-06T15:34:47Z

        CURATOR-160 Add builders and dsl for ZooKeeper's config and reconfig methods.


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user iocanel opened a pull request: https://github.com/apache/curator/pull/53 CURATOR-160 Add builders and dsl for ZooKeeper's config and reconfig m... ...ethods. You can merge this pull request into a Git repository by running: $ git pull https://github.com/iocanel/curator CURATOR-160 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/curator/pull/53.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #53 commit 2c576dc344a167ad4a72d71412c98d76ff4e2d3d Author: Ioannis Canellos <iocanel@gmail.com> Date: 2014-11-06T15:34:47Z CURATOR-160 Add builders and dsl for ZooKeeper's config and reconfig methods.
        Hide
        iocanel Ioannis Canellos added a comment -

        Jordan Zimmerman Are you working on it? or it was auto-assigned? I'd like to pick it up if not.

        Show
        iocanel Ioannis Canellos added a comment - Jordan Zimmerman Are you working on it? or it was auto-assigned? I'd like to pick it up if not.

          People

          • Assignee:
            iocanel Ioannis Canellos
            Reporter:
            randgalt Jordan Zimmerman
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development