ctubbsii commented on a change in pull request #1653:
URL: https://github.com/apache/accumulo/pull/1653#discussion_r488680360
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java
##########
@@ -58,57 +54,31 @@ protected MetaDataStateStore(ClientContext context,
CurrentState state, String t
return new MetaDataTableScanner(context, TabletsSection.getRange(), state,
targetTableName);
}
- @Override
public void setLocations(Collection<Assignment> assignments) throws
DistributedStoreException {
- BatchWriter writer = createBatchWriter();
try {
for (Assignment assignment : assignments) {
- Mutation m = new Mutation(assignment.tablet.toMetaRow());
- assignment.server.putLocation(m);
- assignment.server.clearFutureLocation(m);
- SuspendingTServer.clearSuspension(m);
- writer.addMutation(m);
+ TabletMutator tabletMutator = ample.mutateTablet(assignment.tablet);
+ tabletMutator.putLocation(assignment.server, LocationType.CURRENT);
+ tabletMutator.deleteLocation(assignment.server, LocationType.FUTURE);
+ tabletMutator.mutate();
}
} catch (Exception ex) {
throw new DistributedStoreException(ex);
- } finally {
- try {
- writer.close();
- } catch (MutationsRejectedException e) {
- throw new DistributedStoreException(e);
- }
- }
- }
-
- BatchWriter createBatchWriter() {
- try {
- return context.createBatchWriter(targetTableName,
- new BatchWriterConfig().setMaxMemory(MAX_MEMORY)
- .setMaxLatency(LATENCY,
TimeUnit.MILLISECONDS).setMaxWriteThreads(THREADS));
- } catch (Exception e) {
- throw new RuntimeException(e);
}
}
@Override
public void setFutureLocations(Collection<Assignment> assignments)
throws DistributedStoreException {
- BatchWriter writer = createBatchWriter();
try {
for (Assignment assignment : assignments) {
- Mutation m = new Mutation(assignment.tablet.toMetaRow());
- SuspendingTServer.clearSuspension(m);
- assignment.server.putFutureLocation(m);
- writer.addMutation(m);
+ TabletMutator tabletMutator = ample.mutateTablet(assignment.tablet);
+ tabletMutator.deleteSuspension();
+ tabletMutator.putLocation(assignment.server, LocationType.FUTURE);
+ tabletMutator.mutate();
Review comment:
I think this loop could benefit from the same batch writer reuse that
you did in the unassign method below this, where you called `try (var
tabletsMutator = ample.mutateTablets()) {` outside the loop. (Same comment
applies to the `setLocations` method above.)
##########
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:
This replaces `SuspendingTServer.toValue()`, which does not appear to be
used any longer. The old method stored a `HostAndPort.toString()`, but this
stores `Ample.TServer.toString()`. Are these identical?
The serialization for `toValue` should probably remain in the
`SuspendingTServer` class, since that's where the corresponding `fromValue`
method is. So, I recommend changing `SuspendingTServer.toValue` that is called
from here, so if the serialization changes, it can be changed in both `toValue`
and `fromValue` at the same time.
----------------------------------------------------------------
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]
