[
https://issues.apache.org/jira/browse/HBASE-10985?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13980370#comment-13980370
]
stack commented on HBASE-10985:
-------------------------------
Do we have to do this:
+import
org.apache.hadoop.hbase.regionserver.consensus.SplitTransactionConsensus;
... and this:
+import
org.apache.hadoop.hbase.regionserver.consensus.SplitTransactionConsensus;
+import
org.apache.hadoop.hbase.regionserver.consensus.ZKSplitTransactionConsensus;
i.e. have a higher level package reach over into a package that is at a level
below? We'd like to not build tangles across packages w/ dependencies going
both up and down the package hierarchy. If under consensus we have a
regionserver subpackage, is that a mess? It needs to get its hands into the
regionserver guts?
Tough problem.
Are these to be used by a subclass?
- private Server server;
- private ZooKeeperWatcher watcher;
+ protected Server server;
+ protected ZooKeeperWatcher watcher;
+ protected SplitTransactionConsensus splitTransactionConsensus;
Is that why access is opened up?
Is this right? We have to pass in the watcher. When Mikhail did his close, I
don't think he passed in a zk instance. Having to pass in a zk instance even
though the consensus might not be using zk at all is implementation leaking out
of the Interface/Implementation?
+ ConsensusProvider consensus =
ConsensusProviderFactory.getConsensusProvider(server
+ .getConfiguration());
+ consensus.initialize(server);
+ consensus.start();
+ if
(consensus.getSplitTransactionConsensus().transitionSplittingNode(watcher, p,
hri_a,
+ hri_b, sn, -1, EventType.RS_ZK_REQUEST_REGION_SPLIT,
EventType.RS_ZK_REGION_SPLITTING) == -1) {
byte[] data = ZKAssign.getData(watcher, encodedName);
We have to initialize and then start the consensus? (A start w/o a stop is a
little jarring -- when I see 'start', I always expect to see a 'stop')
When I see this in SplitTransaction which is in the regionserver subpackage:
+import org.apache.hadoop.hbase.consensus.ConsensusProvider;
+import org.apache.hadoop.hbase.consensus.ConsensusProviderFactory;
+import
org.apache.hadoop.hbase.regionserver.consensus.SplitTransactionConsensus;
+import
org.apache.hadoop.hbase.regionserver.consensus.ZKSplitTransactionConsensus;
I wonder if we can do better? Do we have to have this many different packages?
When we do they are leaking into each other. Not good package encapsulation.
When is this called?
+ public void setConsensusProvider(ConsensusProvider consensus) {
Should it be called in the constructor for SplitTransaction?
(Is this 'consensus' we are doing here? Or rather is it, cluster state
transition? we need consensus to do this?)
Be careful about tabs .... in hbase they are two spaces:
- this.parent.getRegionNameAsString(), e);
- }
+ consensus.getSplitTransactionConsensus().createNodeSplitting(server,
+ parent, server.getServerName(), hri_a, hri_b);
Again, the implementation is leaking here?
- znodeVersion = getZKNode(server, services);
+ znodeVersion =
consensus.getSplitTransactionConsensus().getZKNode(server, services, parent,
hri_a, hri_b);
We are asking the consensus provider to do a zk op. Would think it would be an
op that is not named for the implementation type.
I did about 1/3rd of the patch.
Thanks.
> Decouple Split Transaction from Zookeeper
> -----------------------------------------
>
> Key: HBASE-10985
> URL: https://issues.apache.org/jira/browse/HBASE-10985
> Project: HBase
> Issue Type: Sub-task
> Components: Consensus, Zookeeper
> Reporter: Sergey Soldatov
> Attachments: HBASE-10985.patch, HBASE-10985.patch, HBASE-10985.patch
>
>
> As part of HBASE-10296 SplitTransaction should be decoupled from Zookeeper.
> This is an initial patch for review. At the moment the consensus provider
> placed directly to SplitTransaction to minimize affected code. In the ideal
> world it should be done in HServer.
--
This message was sent by Atlassian JIRA
(v6.2#6252)