ctubbsii commented on a change in pull request #1653:
URL: https://github.com/apache/accumulo/pull/1653#discussion_r488999796
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java
##########
@@ -128,73 +98,56 @@ public void suspend(Collection<TabletLocationState>
tablets,
private void unassign(Collection<TabletLocationState> tablets,
Map<TServerInstance,List<Path>> logsForDeadServers, long
suspensionTimestamp)
throws DistributedStoreException {
- BatchWriter writer = createBatchWriter();
- try {
+ try (var tabletsMutator = ample.mutateTablets()) {
for (TabletLocationState tls : tablets) {
- Mutation m = new Mutation(tls.extent.toMetaRow());
+ TabletMutator tabletMutator = tabletsMutator.mutateTablet(tls.extent);
if (tls.current != null) {
- tls.current.clearLocation(m);
+ tabletMutator.deleteLocation(tls.current, LocationType.CURRENT);
if (logsForDeadServers != null) {
List<Path> logs = logsForDeadServers.get(tls.current);
if (logs != null) {
for (Path log : logs) {
LogEntry entry =
new LogEntry(tls.extent, 0, tls.current.hostPort(),
log.toString());
- m.put(entry.getColumnFamily(), entry.getColumnQualifier(),
entry.getValue());
+ tabletMutator.putWal(entry);
}
}
}
if (suspensionTimestamp >= 0) {
- SuspendingTServer suspender =
- new SuspendingTServer(tls.current.getLocation(),
suspensionTimestamp);
- suspender.setSuspension(m);
+ tabletMutator.putSuspension(tls.current, suspensionTimestamp);
}
}
if (tls.suspend != null && suspensionTimestamp < 0) {
- SuspendingTServer.clearSuspension(m);
+ tabletMutator.deleteSuspension();
}
if (tls.future != null) {
- tls.future.clearFutureLocation(m);
+ tabletMutator.deleteLocation(tls.future, LocationType.FUTURE);
}
- writer.addMutation(m);
+ tabletMutator.mutate();
}
} catch (Exception ex) {
Review comment:
If all of these code blocks that were updated to use the Ample mutator
utility don't throw any checked exceptions, these catch blocks can be updated
to only catch the more specific, `RuntimeException`, as in:
```suggestion
} catch (RuntimeException ex) {
```
##########
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 {
+ try (var tabletsMutator = ample.mutateTablets()) {
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 =
tabletsMutator.mutateTablet(assignment.tablet);
+ tabletMutator.putLocation(assignment.server, LocationType.CURRENT);
+ tabletMutator.deleteLocation(assignment.server, LocationType.FUTURE);
+ tabletMutator.mutate();
Review comment:
Did you forget to include the `deleteSuspension()` here?
Also, the code might look cleaner if you chained the methods, something like
(not formatted):
```suggestion
tabletsMutator.mutateTablet(assignment.tablet)
.putLocation(assignment.server, LocationType.CURRENT)
.deleteLocation(assignment.server, LocationType.FUTURE)
.deleteSuspension()
.mutate();
```
If the formatter makes it look too ugly, you could prevent the formatter
from messing with it by wrapping it with a comment line before and after, `//
@formatter:off` and `// @formatter:on`.
This kind of simplification can be done in other places in this PR also (I
made a review comment elsewhere where it can be a one-liner.
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/master/state/MetaDataStateStore.java
##########
@@ -128,73 +98,56 @@ public void suspend(Collection<TabletLocationState>
tablets,
private void unassign(Collection<TabletLocationState> tablets,
Map<TServerInstance,List<Path>> logsForDeadServers, long
suspensionTimestamp)
throws DistributedStoreException {
- BatchWriter writer = createBatchWriter();
- try {
+ try (var tabletsMutator = ample.mutateTablets()) {
for (TabletLocationState tls : tablets) {
- Mutation m = new Mutation(tls.extent.toMetaRow());
+ TabletMutator tabletMutator = tabletsMutator.mutateTablet(tls.extent);
if (tls.current != null) {
- tls.current.clearLocation(m);
+ tabletMutator.deleteLocation(tls.current, LocationType.CURRENT);
if (logsForDeadServers != null) {
List<Path> logs = logsForDeadServers.get(tls.current);
if (logs != null) {
for (Path log : logs) {
LogEntry entry =
new LogEntry(tls.extent, 0, tls.current.hostPort(),
log.toString());
- m.put(entry.getColumnFamily(), entry.getColumnQualifier(),
entry.getValue());
+ tabletMutator.putWal(entry);
}
}
}
if (suspensionTimestamp >= 0) {
- SuspendingTServer suspender =
- new SuspendingTServer(tls.current.getLocation(),
suspensionTimestamp);
- suspender.setSuspension(m);
+ tabletMutator.putSuspension(tls.current, suspensionTimestamp);
}
}
if (tls.suspend != null && suspensionTimestamp < 0) {
- SuspendingTServer.clearSuspension(m);
+ tabletMutator.deleteSuspension();
}
if (tls.future != null) {
- tls.future.clearFutureLocation(m);
+ tabletMutator.deleteLocation(tls.future, LocationType.FUTURE);
}
- writer.addMutation(m);
+ tabletMutator.mutate();
}
} catch (Exception ex) {
throw new DistributedStoreException(ex);
- } finally {
- try {
- writer.close();
- } catch (MutationsRejectedException e) {
- throw new DistributedStoreException(e);
- }
}
}
@Override
public void unsuspend(Collection<TabletLocationState> tablets) throws
DistributedStoreException {
- BatchWriter writer = createBatchWriter();
- try {
+ try (var tabletsMutator = ample.mutateTablets()) {
for (TabletLocationState tls : tablets) {
if (tls.suspend != null) {
continue;
}
- Mutation m = new Mutation(tls.extent.toMetaRow());
- SuspendingTServer.clearSuspension(m);
- writer.addMutation(m);
+ TabletMutator tabletMutator = tabletsMutator.mutateTablet(tls.extent);
+ tabletMutator.deleteSuspension();
+ tabletMutator.mutate();
Review comment:
```suggestion
tabletsMutator.mutateTablet(tls.extent).deleteSuspension().mutate();
```
----------------------------------------------------------------
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]
