dlmarion commented on code in PR #3274:
URL: https://github.com/apache/accumulo/pull/3274#discussion_r1158646240
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java:
##########
@@ -243,6 +256,10 @@ public interface TabletsMutator extends AutoCloseable {
* Interface for changing a tablets persistent data.
*/
interface TabletMutator {
+ TabletMutator updateLastForAssignmentMode(KeyExtent extent,
TServerInstance location);
+
+ TabletMutator updateLastForCompactionMode(Location location,
TServerInstance serverInstance);
Review Comment:
I wonder if the same concerns that Keith has for the
`updateLastForAssignmentMode` apply here.
##########
core/src/main/java/org/apache/accumulo/core/metadata/schema/Ample.java:
##########
@@ -243,6 +256,10 @@ public interface TabletsMutator extends AutoCloseable {
* Interface for changing a tablets persistent data.
*/
interface TabletMutator {
+ TabletMutator updateLastForAssignmentMode(KeyExtent extent,
TServerInstance location);
Review Comment:
`updateLastForAssignmentMode` is only called by ZooTabletStateStore and
MetaDataStateStore. I wonder if this method could be moved to the
TabletStateStore interface as a static method.
##########
server/base/src/test/java/org/apache/accumulo/server/MockServerContext.java:
##########
@@ -40,7 +40,12 @@ public static ServerContext get() {
ServerContext context = EasyMock.createMock(ServerContext.class);
ConfigurationCopy conf = new
ConfigurationCopy(DefaultConfiguration.getInstance());
conf.set(Property.INSTANCE_VOLUMES, "file:///");
+ // Added the new expected property value
+ conf.set(Property.TSERV_LAST_LOCATION_MODE, "assignment");
expect(context.getConfiguration()).andReturn(conf).anyTimes();
+ // Null pointer pops when attempting to access the property in the context.
+
expect(context.getConfiguration().get(Property.INSTANCE_VOLUMES)).andReturn("file:///")
+ .anyTimes();
Review Comment:
```
expect(context.getConfiguration()).andReturn(conf).anyTimes();
```
will return the ConfigurationCopy object (which is not a Mock object). So,
you don't need to mock calls to the Configuration object, but the Configuration
object does need to be populated with the properties you are looking for. In
this case it is, on line 42. I agree with @keith-turner , I believe that lines
46 - 48 can be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
