ctubbsii commented on a change in pull request #1653:
URL: https://github.com/apache/accumulo/pull/1653#discussion_r488770528
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/metadata/TabletMutatorBase.java
##########
@@ -202,6 +203,23 @@ private String getLocationFamily(LocationType type) {
return this;
}
+ @Override
+ public Ample.TabletMutator putSuspension(Ample.TServer tServer, long
suspensionTime) {
+ Preconditions.checkState(updatesEnabled, "Cannot make updates after
calling mutate.");
+ mutation.put(SuspendLocationColumn.SUSPEND_COLUMN.getColumnFamily(),
+ SuspendLocationColumn.SUSPEND_COLUMN.getColumnQualifier(),
+ new Value(tServer + "|" + suspensionTime));
Review comment:
> I don't fully understand your second paragraph. Are you suggesting I
use the `SuspendingTServer.fromValue()` function in this mutation instead of
doing the `new Value(tServer.getLocation() + "|" + suspensionTime))`?
Prior to your changes, the `SuspendingTServer.toValue()` was responsible for
creating this `new Value`. I'm suggesting that we leave that class responsible
for creating the `new Value`, so it can live alongside that class' existing
`fromValue` method. In order to do that, you would need to turn it into a
static method, so you can use it in your code.
In other words, this code should change to something like:
```suggestion
SuspendingTServer.toValue(tServer, suspensionTime);
```
And then that method should look something like:
```java
public static Value toValue(HostAndPort tServer, long suspensionTime) {
return new Value(tServer + "|" + suspensionTime);
}
```
I probably don't have the types correct in this code, but something like
that is what I'm suggesting.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
