Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.4
    • Fix Version/s: 0.5, 0.4.1
    • Component/s: hbase, storage handlers
    • Labels:
      None

      Description

      Implement initial revision of endpoint and add new implementation of existing RM interface for HBase endpoint client. Add option to configure RM as endpoint. This will leave current default behavior as is (ZK based RM running per client in storage handler).

        Issue Links

          Activity

          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/
          -----------------------------------------------------------

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary
          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.
          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs


          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION
          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255
          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing
          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review5907
          -----------------------------------------------------------

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java
          <https://reviews.apache.org/r/4313/#comment12870>

          Normally super.stop is called at the end. Should this be moved down or does BaseEndointCoprocessor.stop do nothing?

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java
          <https://reviews.apache.org/r/4313/#comment12871>

          Now that this is IN hbase can we just pass in the Configuration object? Really not a fan of the selective pulling from conf to properties.

          • David

          On 2012-03-13 19:38:09, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-13 19:38:09)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review5907 ----------------------------------------------------------- trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java < https://reviews.apache.org/r/4313/#comment12870 > Normally super.stop is called at the end. Should this be moved down or does BaseEndointCoprocessor.stop do nothing? trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java < https://reviews.apache.org/r/4313/#comment12871 > Now that this is IN hbase can we just pass in the Configuration object? Really not a fan of the selective pulling from conf to properties. David On 2012-03-13 19:38:09, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-13 19:38:09) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review5990
          -----------------------------------------------------------

          The RevisionManagerFactory responsiblity seems to be leaking into the storageHandler code. storageHandler should just call the factory to instantiate RevisionManager. Also should probably consider changing the methods a bit. Split getRevisionManger into getRevisionManagerClient and getRevisionManagerServer. Though it's not really clear to me if the Endpoint needs to use the factory we can have an internal util to create a server instance instead since the user will most like never need an instance of revision manager server.

          The Factory itself shouldn't depend on hadoop classes (ie Configuration) because the original intent was that this library may be used for other storage systems (ie Cassandra). We can revisit that idea but let's try to keep things consistent with the original plan until a decision is made. We can probably provide a util method which takes in the configuration object and return a populated properties instance as part of the HBaseRevisionManagerUtil class.

          Instead of having another factory for the EndPointClient its prolly cleaner to have a class analogous to ZKBaseRevisionManager (ie HBaseClientRevisionManager) which wraps the CoprocessorProxyClient.

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java
          <https://reviews.apache.org/r/4313/#comment12985>

          This should be part of the Factory class since this'll be mainly used there.

          • Francis

          On 2012-03-13 19:38:09, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-13 19:38:09)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review5990 ----------------------------------------------------------- The RevisionManagerFactory responsiblity seems to be leaking into the storageHandler code. storageHandler should just call the factory to instantiate RevisionManager. Also should probably consider changing the methods a bit. Split getRevisionManger into getRevisionManagerClient and getRevisionManagerServer. Though it's not really clear to me if the Endpoint needs to use the factory we can have an internal util to create a server instance instead since the user will most like never need an instance of revision manager server. The Factory itself shouldn't depend on hadoop classes (ie Configuration) because the original intent was that this library may be used for other storage systems (ie Cassandra). We can revisit that idea but let's try to keep things consistent with the original plan until a decision is made. We can probably provide a util method which takes in the configuration object and return a populated properties instance as part of the HBaseRevisionManagerUtil class. Instead of having another factory for the EndPointClient its prolly cleaner to have a class analogous to ZKBaseRevisionManager (ie HBaseClientRevisionManager) which wraps the CoprocessorProxyClient. trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java < https://reviews.apache.org/r/4313/#comment12985 > This should be part of the Factory class since this'll be mainly used there. Francis On 2012-03-13 19:38:09, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-13 19:38:09) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review5992
          -----------------------------------------------------------

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java
          <https://reviews.apache.org/r/4313/#comment12986>

          VERSION should probably in EndPoint class since this is coprocessor specific.

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java
          <https://reviews.apache.org/r/4313/#comment12987>

          Good catch on this one. We will probably need to change the interface to support retries.

          • Francis

          On 2012-03-13 19:38:09, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-13 19:38:09)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review5992 ----------------------------------------------------------- trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java < https://reviews.apache.org/r/4313/#comment12986 > VERSION should probably in EndPoint class since this is coprocessor specific. trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java < https://reviews.apache.org/r/4313/#comment12987 > Good catch on this one. We will probably need to change the interface to support retries. Francis On 2012-03-13 19:38:09, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-13 19:38:09) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-03-15 17:52:56, Francis Liu wrote:

          > The RevisionManagerFactory responsiblity seems to be leaking into the storageHandler code. storageHandler should just call the factory to instantiate RevisionManager. Also should probably consider changing the methods a bit. Split getRevisionManger into getRevisionManagerClient and getRevisionManagerServer. Though it's not really clear to me if the Endpoint needs to use the factory we can have an internal util to create a server instance instead since the user will most like never need an instance of revision manager server.

          >

          > The Factory itself shouldn't depend on hadoop classes (ie Configuration) because the original intent was that this library may be used for other storage systems (ie Cassandra). We can revisit that idea but let's try to keep things consistent with the original plan until a decision is made. We can probably provide a util method which takes in the configuration object and return a populated properties instance as part of the HBaseRevisionManagerUtil class.

          >

          > Instead of having another factory for the EndPointClient its prolly cleaner to have a class analogous to ZKBaseRevisionManager (ie HBaseClientRevisionManager) which wraps the CoprocessorProxyClient.

          >

          >

          >

          A "client" class that implements the interface just to delegate all calls to another instance with the same interface w/o added behavior is tad redundant. Certainly there could be the need to do more than just pass on the call in the future, to deal with specifics of the protocol, perform a mapping etc., at which point such wrapper can be added. Anyways, does not hurt and it's added to the patch..

          About the factory class, it may be desirable to provide control over instantiation when the goal is to build a pluggable interface. An IoC container would mute this debate. But Class.forName is not enough to achieve a lot of abstraction. Settings need to be mapped from HBase conf into properties, fully assuming that the implementation is ZK RM. We can take that up later, I reverted back to the static factory binding.

          • Thomas

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review5990
          -----------------------------------------------------------

          On 2012-03-17 02:33:47, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-17 02:33:47)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-03-15 17:52:56, Francis Liu wrote: > The RevisionManagerFactory responsiblity seems to be leaking into the storageHandler code. storageHandler should just call the factory to instantiate RevisionManager. Also should probably consider changing the methods a bit. Split getRevisionManger into getRevisionManagerClient and getRevisionManagerServer. Though it's not really clear to me if the Endpoint needs to use the factory we can have an internal util to create a server instance instead since the user will most like never need an instance of revision manager server. > > The Factory itself shouldn't depend on hadoop classes (ie Configuration) because the original intent was that this library may be used for other storage systems (ie Cassandra). We can revisit that idea but let's try to keep things consistent with the original plan until a decision is made. We can probably provide a util method which takes in the configuration object and return a populated properties instance as part of the HBaseRevisionManagerUtil class. > > Instead of having another factory for the EndPointClient its prolly cleaner to have a class analogous to ZKBaseRevisionManager (ie HBaseClientRevisionManager) which wraps the CoprocessorProxyClient. > > > A "client" class that implements the interface just to delegate all calls to another instance with the same interface w/o added behavior is tad redundant. Certainly there could be the need to do more than just pass on the call in the future, to deal with specifics of the protocol, perform a mapping etc., at which point such wrapper can be added. Anyways, does not hurt and it's added to the patch.. About the factory class, it may be desirable to provide control over instantiation when the goal is to build a pluggable interface. An IoC container would mute this debate. But Class.forName is not enough to achieve a lot of abstraction. Settings need to be mapped from HBase conf into properties, fully assuming that the implementation is ZK RM. We can take that up later, I reverted back to the static factory binding. Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review5990 ----------------------------------------------------------- On 2012-03-17 02:33:47, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-17 02:33:47) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/
          -----------------------------------------------------------

          (Updated 2012-03-17 02:33:47.910198)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Changes
          -------

          Updated patch for review comments.

          Summary
          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.
          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs (updated)


          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION
          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255
          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing
          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-17 02:33:47.910198) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Changes ------- Updated patch for review comments. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs (updated) trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-03-13 22:45:40, David Capwell wrote:

          > trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java, line 49

          > <https://reviews.apache.org/r/4313/diff/2/?file=91778#file91778line49>

          >

          > Normally super.stop is called at the end. Should this be moved down or does BaseEndointCoprocessor.stop do nothing?

          done

          • Thomas

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review5907
          -----------------------------------------------------------

          On 2012-03-17 02:33:47, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-17 02:33:47)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-03-13 22:45:40, David Capwell wrote: > trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java, line 49 > < https://reviews.apache.org/r/4313/diff/2/?file=91778#file91778line49 > > > Normally super.stop is called at the end. Should this be moved down or does BaseEndointCoprocessor.stop do nothing? done Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review5907 ----------------------------------------------------------- On 2012-03-17 02:33:47, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-17 02:33:47) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-03-15 17:55:41, Francis Liu wrote:

          > trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java, line 28

          > <https://reviews.apache.org/r/4313/diff/2/?file=91777#file91777line28>

          >

          > VERSION should probably in EndPoint class since this is coprocessor specific.

          Unfortunately w/o the VERSION field present in the interface that defines the method(s) invoked on the proxy, we will see:

          Caused by: java.lang.NoSuchFieldException: VERSION
          at java.lang.Class.getField(Class.java:1520)

          • Thomas

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review5992
          -----------------------------------------------------------

          On 2012-03-17 02:33:47, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-17 02:33:47)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-03-15 17:55:41, Francis Liu wrote: > trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java, line 28 > < https://reviews.apache.org/r/4313/diff/2/?file=91777#file91777line28 > > > VERSION should probably in EndPoint class since this is coprocessor specific. Unfortunately w/o the VERSION field present in the interface that defines the method(s) invoked on the proxy, we will see: Caused by: java.lang.NoSuchFieldException: VERSION at java.lang.Class.getField(Class.java:1520) Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review5992 ----------------------------------------------------------- On 2012-03-17 02:33:47, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-17 02:33:47) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1300255 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1300255 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          Thomas Weise added a comment -

          Patch with final set of review comments.

          Show
          Thomas Weise added a comment - Patch with final set of review comments.
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/
          -----------------------------------------------------------

          (Updated 2012-03-20 19:59:40.047929)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary
          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.
          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs (updated)


          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1302742
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1302742
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1302742
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1302742
          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/package-info.java PRE-CREATION
          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1302742
          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1302742
          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing
          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-20 19:59:40.047929) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs (updated) trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/package-info.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1302742 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1302742 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review6228
          -----------------------------------------------------------

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java
          <https://reviews.apache.org/r/4313/#comment13424>

          RevisionManagerEndpoint isntead?

          • Francis

          On 2012-03-20 19:59:40, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-20 19:59:40)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/package-info.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1302742

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1302742

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review6228 ----------------------------------------------------------- trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java < https://reviews.apache.org/r/4313/#comment13424 > RevisionManagerEndpoint isntead? Francis On 2012-03-20 19:59:40, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-20 19:59:40) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/package-info.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1302742 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1302742 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review6243
          -----------------------------------------------------------

          Ship it!

          some small changes I can fix on checkin. do you plan to work on deployment in a separate jira? I'd also like to have all the unit tests use this by default. and have this our main way of deployment.

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java
          <https://reviews.apache.org/r/4313/#comment13488>

          this constant should be defined in this class. since it is specific to this.

          • Francis

          On 2012-03-20 19:59:40, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-20 19:59:40)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/package-info.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1302742

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1302742

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review6243 ----------------------------------------------------------- Ship it! some small changes I can fix on checkin. do you plan to work on deployment in a separate jira? I'd also like to have all the unit tests use this by default. and have this our main way of deployment. trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java < https://reviews.apache.org/r/4313/#comment13488 > this constant should be defined in this class. since it is specific to this. Francis On 2012-03-20 19:59:40, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-20 19:59:40) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/package-info.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1302742 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1302742 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          jiraposter@reviews.apache.org added a comment -

          On 2012-03-22 18:42:09, Francis Liu wrote:

          > some small changes I can fix on checkin. do you plan to work on deployment in a separate jira? I'd also like to have all the unit tests use this by default. and have this our main way of deployment.

          Yes, will make separate JIRA that will also deal with making e2e tests work (need solution to deploy endpoint to hbase), at which point we can switch default.

          On 2012-03-22 18:42:09, Francis Liu wrote:

          > trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java, line 49

          > <https://reviews.apache.org/r/4313/diff/4/?file=93924#file93924line49>

          >

          > this constant should be defined in this class. since it is specific to this.

          It logically belongs there, but we should also think of having a central place that lists all available settings (something like HConstants).

          • Thomas

          -----------------------------------------------------------
          This is an automatically generated e-mail. To reply, visit:
          https://reviews.apache.org/r/4313/#review6243
          -----------------------------------------------------------

          On 2012-03-20 19:59:40, Thomas wrote:

          -----------------------------------------------------------

          This is an automatically generated e-mail. To reply, visit:

          https://reviews.apache.org/r/4313/

          -----------------------------------------------------------

          (Updated 2012-03-20 19:59:40)

          Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell.

          Summary

          -------

          Initial patch for RevisionManager endpoint.

          This addresses bug HCATALOG-310.

          https://issues.apache.org/jira/browse/HCATALOG-310

          Diffs

          -----

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1302742

          trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/package-info.java PRE-CREATION

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1302742

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1302742

          trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION

          Diff: https://reviews.apache.org/r/4313/diff

          Testing

          -------

          unit tests pass

          Thanks,

          Thomas

          Show
          jiraposter@reviews.apache.org added a comment - On 2012-03-22 18:42:09, Francis Liu wrote: > some small changes I can fix on checkin. do you plan to work on deployment in a separate jira? I'd also like to have all the unit tests use this by default. and have this our main way of deployment. Yes, will make separate JIRA that will also deal with making e2e tests work (need solution to deploy endpoint to hbase), at which point we can switch default. On 2012-03-22 18:42:09, Francis Liu wrote: > trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java, line 49 > < https://reviews.apache.org/r/4313/diff/4/?file=93924#file93924line49 > > > this constant should be defined in this class. since it is specific to this. It logically belongs there, but we should also think of having a central place that lists all available settings (something like HConstants). Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/#review6243 ----------------------------------------------------------- On 2012-03-20 19:59:40, Thomas wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/4313/ ----------------------------------------------------------- (Updated 2012-03-20 19:59:40) Review request for Francis Liu, Vandana Ayyalasomayajula and David Capwell. Summary ------- Initial patch for RevisionManager endpoint. This addresses bug HCATALOG-310 . https://issues.apache.org/jira/browse/HCATALOG-310 Diffs ----- trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/HBaseRevisionManagerUtil.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManager.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpoint.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerEndpointClient.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerFactory.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/RevisionManagerProtocol.java PRE-CREATION trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/TableSnapshot.java 1302742 trunk/storage-handlers/hbase/src/java/org/apache/hcatalog/hbase/snapshot/package-info.java PRE-CREATION trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/ManyMiniCluster.java 1302742 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/SkeletonHBaseTest.java 1302742 trunk/storage-handlers/hbase/src/test/org/apache/hcatalog/hbase/snapshot/TestRevisionManagerEndpoint.java PRE-CREATION Diff: https://reviews.apache.org/r/4313/diff Testing ------- unit tests pass Thanks, Thomas
          Hide
          Francis Liu added a comment -

          committed to trunk and branch-0.4. Thanks Thomas

          Show
          Francis Liu added a comment - committed to trunk and branch-0.4. Thanks Thomas

            People

            • Assignee:
              Thomas Weise
              Reporter:
              Thomas Weise
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development