Accumulo
  1. Accumulo
  2. ACCUMULO-2212

ZooReaderWriter is a singleton class with a public constructor

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major 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
        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
        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
        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
        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
        ASF IRC Bot added a comment -

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

        Show
        ASF IRC Bot added a comment - Reply from bhavanki on IRC: Test comment from IRC, try again
        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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:
            Bill Havanki
            Reporter:
            Vikram Srivastava
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development