Uploaded image for project: 'Accumulo'
  1. Accumulo
  2. ACCUMULO-2212

ZooReaderWriter is a singleton class with a public constructor

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.6.1, 1.7.0
    • Component/s: None
    • Labels:
      None

      Issue Links

        Activity

        Hide
        bhavanki Bill Havanki added a comment -

        While working on ACCUMULO-2635 I ran across this issue. I plan to rework o.a.a.server.zookeeper.ZooReaderWriter as a (renamed) factory for ordinary Fate ZooReaderWriter objects.

        Show
        bhavanki Bill Havanki added a comment - While working on ACCUMULO-2635 I ran across this issue. I plan to rework o.a.a.server.zookeeper.ZooReaderWriter as a (renamed) factory for ordinary Fate ZooReaderWriter objects.
        Hide
        bhavanki Bill Havanki added a comment -

        I discarded the first review; Eric points out that it was really large for 1.6.1, and I agree. The second review is much smaller but still introduces what's needed for this particular issue.

        Show
        bhavanki Bill Havanki added a comment - I discarded the first review; Eric points out that it was really large for 1.6.1, and I agree. The second review is much smaller but still introduces what's needed for this particular issue.
        Hide
        asfbot ASF IRC Bot added a comment -

        Reply from bhavanki on IRC:
        Test comment from IRC, try again

        Show
        asfbot ASF IRC Bot added a comment - Reply from bhavanki on IRC: Test comment from IRC, try again
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 484491d21abf84240912209ae16e33639cbfa595 in accumulo's branch refs/heads/1.6.1-SNAPSHOT from Bill Havanki
        [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=484491d ]

        ACCUMULO-2212 Add ZooReaderWriterFactory

        The o.a.a.server.zookeeper.ZooReaderWriter class is mostly copied into a new
        ZooReaderWriterFactory class. Users of the factory can work directly with FATE
        ZooReaderWriter instances, instead of those of the server type, which is no longer
        needed. The logic for building instances from site configuration data is in the
        factory.

        As part of this change, the invocation handler used to retry ZK calls on connection
        loss is refactored into a new RetryingInvocationHandler.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 484491d21abf84240912209ae16e33639cbfa595 in accumulo's branch refs/heads/1.6.1-SNAPSHOT from Bill Havanki [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=484491d ] ACCUMULO-2212 Add ZooReaderWriterFactory The o.a.a.server.zookeeper.ZooReaderWriter class is mostly copied into a new ZooReaderWriterFactory class. Users of the factory can work directly with FATE ZooReaderWriter instances, instead of those of the server type, which is no longer needed. The logic for building instances from site configuration data is in the factory. As part of this change, the invocation handler used to retry ZK calls on connection loss is refactored into a new RetryingInvocationHandler.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit 484491d21abf84240912209ae16e33639cbfa595 in accumulo's branch refs/heads/master from Bill Havanki
        [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=484491d ]

        ACCUMULO-2212 Add ZooReaderWriterFactory

        The o.a.a.server.zookeeper.ZooReaderWriter class is mostly copied into a new
        ZooReaderWriterFactory class. Users of the factory can work directly with FATE
        ZooReaderWriter instances, instead of those of the server type, which is no longer
        needed. The logic for building instances from site configuration data is in the
        factory.

        As part of this change, the invocation handler used to retry ZK calls on connection
        loss is refactored into a new RetryingInvocationHandler.

        Show
        jira-bot ASF subversion and git services added a comment - Commit 484491d21abf84240912209ae16e33639cbfa595 in accumulo's branch refs/heads/master from Bill Havanki [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=484491d ] ACCUMULO-2212 Add ZooReaderWriterFactory The o.a.a.server.zookeeper.ZooReaderWriter class is mostly copied into a new ZooReaderWriterFactory class. Users of the factory can work directly with FATE ZooReaderWriter instances, instead of those of the server type, which is no longer needed. The logic for building instances from site configuration data is in the factory. As part of this change, the invocation handler used to retry ZK calls on connection loss is refactored into a new RetryingInvocationHandler.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit cd045729e44173ab145b25e40942e6c0b5e694b0 in accumulo's branch refs/heads/master from Bill Havanki
        [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=cd04572 ]

        ACCUMULO-2212 Fix UTF8 constant in ZooReaderWriterFactory

        The merge of ACCUMULO-2212 work from the 1.6.1-SNAPSHOT branch introduced the use of
        Constants.UTF8, which does not exist in the master branch. This commit replaces that
        constant with a locally defined one.

        Show
        jira-bot ASF subversion and git services added a comment - Commit cd045729e44173ab145b25e40942e6c0b5e694b0 in accumulo's branch refs/heads/master from Bill Havanki [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=cd04572 ] ACCUMULO-2212 Fix UTF8 constant in ZooReaderWriterFactory The merge of ACCUMULO-2212 work from the 1.6.1-SNAPSHOT branch introduced the use of Constants.UTF8, which does not exist in the master branch. This commit replaces that constant with a locally defined one.
        Hide
        jira-bot ASF subversion and git services added a comment -

        Commit cd045729e44173ab145b25e40942e6c0b5e694b0 in accumulo's branch refs/heads/master from Bill Havanki
        [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=cd04572 ]

        ACCUMULO-2212 Fix UTF8 constant in ZooReaderWriterFactory

        The merge of ACCUMULO-2212 work from the 1.6.1-SNAPSHOT branch introduced the use of
        Constants.UTF8, which does not exist in the master branch. This commit replaces that
        constant with a locally defined one.

        Show
        jira-bot ASF subversion and git services added a comment - Commit cd045729e44173ab145b25e40942e6c0b5e694b0 in accumulo's branch refs/heads/master from Bill Havanki [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=cd04572 ] ACCUMULO-2212 Fix UTF8 constant in ZooReaderWriterFactory The merge of ACCUMULO-2212 work from the 1.6.1-SNAPSHOT branch introduced the use of Constants.UTF8, which does not exist in the master branch. This commit replaces that constant with a locally defined one.
        Hide
        ctubbsii Christopher Tubbs added a comment -

        Bill Havanki: You should use JDK7's StandardCharsets.UTF_8 for UTF-8 constants, not a locally defined one.

        Show
        ctubbsii Christopher Tubbs added a comment - Bill Havanki : You should use JDK7's StandardCharsets.UTF_8 for UTF-8 constants, not a locally defined one.
        Hide
        ctubbsii Christopher Tubbs added a comment -

        I was looking back at this, and this new o.a.a.server.zookeeper.ZooReaderWriterFactory does not appear to be used at all. Is that intended? Is this a bug? Should this class be deleted?

        Also, the problem originally reported (singleton with public constructor) seems to have shifted to this ZooReaderWriterFactory class instead of the original ZooReaderWriter.

        Since this has already shipped in 1.6.1, I suppose a new issue should be opened to deal with it, but before I do that, I'm kind of curious what the intentions were for this unused class.

        Show
        ctubbsii Christopher Tubbs added a comment - I was looking back at this, and this new o.a.a.server.zookeeper.ZooReaderWriterFactory does not appear to be used at all. Is that intended? Is this a bug? Should this class be deleted? Also, the problem originally reported (singleton with public constructor) seems to have shifted to this ZooReaderWriterFactory class instead of the original ZooReaderWriter . Since this has already shipped in 1.6.1, I suppose a new issue should be opened to deal with it, but before I do that, I'm kind of curious what the intentions were for this unused class.
        Hide
        busbey Sean Busbey added a comment -

        Also, the problem originally reported (singleton with public constructor) seems to have shifted to this ZooReaderWriterFactory class instead of the original ZooReaderWriter.

        AFAICT ZooReaderWriterFactory isn't a singleton. It just shares state across all instances so that it can provide a singleton instance of the standard ZooReaderWriter and the retrying version.

        Show
        busbey Sean Busbey added a comment - Also, the problem originally reported (singleton with public constructor) seems to have shifted to this ZooReaderWriterFactory class instead of the original ZooReaderWriter. AFAICT ZooReaderWriterFactory isn't a singleton. It just shares state across all instances so that it can provide a singleton instance of the standard ZooReaderWriter and the retrying version.
        Hide
        ctubbsii Christopher Tubbs added a comment -

        Ah, good catch. Okay, so the issue is that it's not used, not that the original issue was shifted. Good catch.

        Show
        ctubbsii Christopher Tubbs added a comment - Ah, good catch. Okay, so the issue is that it's not used, not that the original issue was shifted. Good catch.

          People

          • Assignee:
            bhavanki Bill Havanki
            Reporter:
            vickyuec Vikram Srivastava
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development