Refactors tablet mgmt iterator to share decision code w/ TGW #3904
Conversation
The TabletGroupWatcher uses the TabletManagementIterator to filter tablets that need attention. The TabletMgmtIter had its own custom code to make decision about tablets. This commit modifies the TabletMgmtIterator to use the same code as the TGW when making decisions about tablets. This makes it easier to reason about and change the behavior of the TGW. In making the TGW and TabletMgmtIter share code a change was also made to how they get information to make decisions. A new class called TabletManagementParameters was introduced that contains an immutable snapshot of the information that both classes need to make decisions. With this change the iterator and TGW are using the same information for a pass over the metadata table, in the past the information was obtained at different times and could have been different due to race conditions. These race conditions were probably not harmful, but removing them makes the code easier to reason about. Also move some code outside of TGW away from using the TableMgmtIterator. Need to move all code outside of TGW away from using this iterator in order to make the code easier to maintain. Left TODOs in the code about this and will open follow on issues.
dlmarion
left a comment
dlmarion
left a comment
There was a problem hiding this comment.
Please also check that ManagerAssignmentIT is still passing.
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletGoalState.java
Outdated
Show resolved
Hide resolved
.../base/src/main/java/org/apache/accumulo/server/manager/state/TabletManagementParameters.java
Show resolved
Hide resolved
| return Collections.unmodifiableMap(copy); | ||
| } | ||
|
|
||
| private static class JsonData { |
There was a problem hiding this comment.
Would something less generic than JsonData help when it is declared / used? Maybe TMParmsJson or even ParamsJason?
There was a problem hiding this comment.
IMO that's not necessary because its a private inner class and is only used inside the class and never seen outside of the class.
@dlmarion that was a good call on running that test. Ran and it blew up. Turned out the best way to fix was to move a lot of code from using the tablet mgmt scanner to using ample. Made that change in e1da6f0. After that change the only thing left using the tabletmgmt iterator is TGW which is good. Got LocatorIT working while cleaning up the code and removed Disabled from it. |
| System.out.println( | ||
| mti + " is " + state + " #walogs:" + mti.getTabletMetadata().getLogs().size()); | ||
| && tabletMetadata.getHostingGoal() == TabletHostingGoal.ONDEMAND) { | ||
| System.out.println(tabletMetadata.getExtent() + " is " + state + " #walogs:" |
There was a problem hiding this comment.
Seems like state here is always going to be hosted. We could just change the print statement. Also, maybe a better name for this class is ListHostedOnDemandTablets as it does not list the unhosted ones. I think it's fine to submit a follow-on issue for this if you don't want to address this here.
test/src/main/java/org/apache/accumulo/test/ManagerRepairsDualAssignmentIT.java
Outdated
Show resolved
Hide resolved
server/base/src/main/java/org/apache/accumulo/server/manager/state/TabletGoalState.java
Outdated
Show resolved
Hide resolved
Updates the FATE operations that split tablets to handle walogs. These changes should wait for a tablet with walogs to recover before proceeding. The changes to actually make recovery happen were already done in apache#3904 with changes to TabletGoalState.compute(). This change is a WIP because it depends on apache#3847 and needs those changes to be complete. fixes apache#3844
4 participants
The TabletGroupWatcher uses the TabletManagementIterator to filter tablets that need attention. The TabletMgmtIter had its own custom code to make decision about tablets. This commit modifies the TabletMgmtIterator to use the same code as the TGW when making decisions about tablets. This makes it easier to reason about and change the behavior of the TGW.
In making the TGW and TabletMgmtIter share code a change was also made to how they get information to make decisions. A new class called TabletManagementParameters was introduced that contains an immutable snapshot of the information that both classes need to make decisions. With this change the iterator and TGW are using the same information for a pass over the metadata table, in the past the information was obtained at different times and could have been different due to race conditions. These race conditions were probably not harmful, but removing them makes the code easier to reason about.
Also move some code outside of TGW away from using the TableMgmtIterator. Need to move all code outside of TGW away from using this iterator in order to make the code easier to maintain. Left TODOs in the code about this and will open follow on issues.