You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "stack (JIRA)" <ji...@apache.org> on 2011/05/19 01:34:51 UTC
[jira] [Created] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
-------------------------------------------------------------------------------------------------------------------------------------
Key: HBASE-3896
URL: https://issues.apache.org/jira/browse/HBASE-3896
Project: HBase
Issue Type: Task
Reporter: stack
If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13464063#comment-13464063 ]
stack commented on HBASE-3896:
------------------------------
bq. I don't think that abstracting out the serverManger and catalogtracker has any real value, at the moment. I don't think there are other implementations of those classes, so pulling out an interface only makes things more complicated, not less.
Other implementations would be mocks that implement the SM and CT Interfaces?
Otherwise, appreciate the interjection. Good input.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13462444#comment-13462444 ]
stack commented on HBASE-3896:
------------------------------
[~cody.marcel@gmail.com] Go for it. Anything you can do to improve the testability would be most welcome. You might consider breaking SM up into multiple Interfaces. There'd be the methods used by AssignmentManager, and then others could go into the Interface the master uses. Not sure if you could make that clean of a distinction... but something to consider. Your SM plan sounds great.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Cody Marcel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Cody Marcel resolved HBASE-3896.
--------------------------------
Resolution: Won't Fix
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Jesse Yates (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13464117#comment-13464117 ]
Jesse Yates commented on HBASE-3896:
------------------------------------
bq. Other implementations would be mocks that implement the SM and CT Interfaces?
Mockito already can just mock those out (eg. ServerManager manager = Mockito.mock(ServerManager.class)), so another interface isn't really all that necessary.
Just my $0.02 :)
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Work stopped] (HBASE-3896) Make AssignmentManager
standalone testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Cody Marcel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Work on HBASE-3896 stopped by Cody Marcel.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13464199#comment-13464199 ]
stack commented on HBASE-3896:
------------------------------
[~cody.marcel@gmail.com] Look for other issues w/ noob. Thanks for your work getting this issue closed.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Cody Marcel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Cody Marcel updated HBASE-3896:
-------------------------------
Assignee: Cody Marcel
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Cody Marcel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13464144#comment-13464144 ]
Cody Marcel commented on HBASE-3896:
------------------------------------
I still think it would be an overall cleaner solution to have interfaces and true mock implementations that can be reused elsewhere, but I don't have strong opinions on it. Any advantage would be minimal at best. I was mainly looking for something easy to get my feet wet. I will close this.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13462114#comment-13462114 ]
stack commented on HBASE-3896:
------------------------------
[~cody.marcel@gmail.com] This is done. Check TestAssignmentManager. See how it mocks up what AM needs. Gets a bit messy around the rpc'ing. If you have ideas on how to improve, please say. Else, I think we can close this out.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Work started] (HBASE-3896) Make AssignmentManager
standalone testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Cody Marcel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Work on HBASE-3896 started by Cody Marcel.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Cody Marcel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13462156#comment-13462156 ]
Cody Marcel commented on HBASE-3896:
------------------------------------
Mockito allows you to work with the class directly, but it seems that there is still advantage to working on an interface. It would certainly make it more injection friendly later. This is what I had in mind for the ServerManger.
Rename ServerManager -> BaseServerManager
extract interface from that and name it ServerManager.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13464141#comment-13464141 ]
stack commented on HBASE-3896:
------------------------------
bq. Mockito already can just mock those out (eg. ServerManager manager = Mockito.mock(ServerManager.class)), so another interface isn't really all that necessary.
That is true (I think that it this way in TestAM).
[~cody.marcel@gmail.com] I think we should close this issue given Jesses' reasoning. This issue was filed in May of 2011 by me when I probably made my first attempt at trying to do a standalone AM and failed thinking I needed SM and CT to be Interfaces. Later in the year, I wrote the first TestAssignmentManager implementation which stands up an AM w/o its Master wrapping using Mockito. Therein I did the trick Jesse suggests of getting around the need of an Interface by doing ServerManager manager = Mockito.mock(ServerManager.class)). I think I should have closed this issue at that time having done a workaround (And Jesse makes good argument that making Interfaces of SM and CT won't help elsewhere). What you think?
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Cody Marcel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13463968#comment-13463968 ]
Cody Marcel commented on HBASE-3896:
------------------------------------
I am making subtasks for this to break the changes up into smaller change lists.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "stack (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13463962#comment-13463962 ]
stack commented on HBASE-3896:
------------------------------
[~cody.marcel@gmail.com] np. Was just a notion.
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Cody Marcel (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13463953#comment-13463953 ]
Cody Marcel commented on HBASE-3896:
------------------------------------
I looked into splitting SM up a bit on the interface. There doesn't seem to be a clear logical line between the methods used by HMaster vs. AM. There is also some overlap of methods used in both.
i.e.
isClusterShutdown
isServerOnline
createDestinationServersList
getDeadServers
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (HBASE-3896) Make AssignmentManager standalone
testable by having its constructor take Interfaces rather than a
CatalogTracker and a ServerManager
Posted by "Jesse Yates (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/HBASE-3896?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13463995#comment-13463995 ]
Jesse Yates commented on HBASE-3896:
------------------------------------
I don't think that abstracting out the serverManger and catalogtracker has any real value, at the moment. I don't think there are other implementations of those classes, so pulling out an interface only makes things more complicated, not less. If there were other implementations, for different use cases, then it would make sense to put it all behind an interface to swap them out easily.
I'm +1 on stack's original comment though that mocking out the classes passes into the AssignmentManager (which is already pretty well setup for testing since it can take in all its dependencies) it enough to make the assignment manager easy to test.
However, I do think there is value in making the serverManger more composition based. Cody mentioned (offline) functions like stop that could be more cleanly abstracted not only here, but around the codebase (I'm thinking you have a StopBuilder that takes in a bunch of things to stop and then builds your 'stopper' so you can close everything out easily). I'm all for adding a couple new classes to help break out the functions of the SM some more, but that shouldn't happen to just hide functionality from certain parts of the code (e.g. hmaster shouldn't know about function X, so we make another interface that doesn't include that.... leads to a really complex inheritance heirarchy that is hard to reason about and makes the concrete classes even harder to read).
TL;DR anything to make the SM smaller (break out functionality) and composition based would be great, but the root of this ticket should be solvable via mocking. Maybe, at the very least if we still end up doing this, rename the ticket?
> Make AssignmentManager standalone testable by having its constructor take Interfaces rather than a CatalogTracker and a ServerManager
> -------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HBASE-3896
> URL: https://issues.apache.org/jira/browse/HBASE-3896
> Project: HBase
> Issue Type: Task
> Reporter: stack
> Assignee: Cody Marcel
>
> If we could stand up an instance of AssignmentManager, a core fat class that has a bunch of critical logic managing state transitions, then it'd be easier writing unit tests around its logic. Currently its hard because it takes a ServerManager and a CatalogTracker, but a little bit of work could turn these into Interfaces. SM looks easy to do. Changing CT into an Interface instead might ripple a little through the code base but it'd probably be well worth it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira