Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@
*/
package org.apache.accumulo.server.util.adminCommand;

import static java.nio.charset.StandardCharsets.UTF_8;

import java.io.BufferedWriter;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
Expand Down Expand Up @@ -79,6 +86,7 @@
import com.beust.jcommander.JCommander;
import com.beust.jcommander.Parameter;
import com.google.auto.service.AutoService;
import com.google.common.base.Preconditions;

@AutoService(KeywordExecutable.class)
public class Fate extends ServerKeywordExecutable<FateOpts> {
Expand Down Expand Up @@ -152,6 +160,14 @@ static class FateOpts extends ServerOpts {
@Parameter(names = {"-i", "--info"},
description = "Includes detailed transaction information when printing")
boolean printDetails;

@Parameter(names = {"-n", "--numSplits"},
description = "Generate N split points for the fate table and print to stdout. N should always be greater than 1.")
Comment on lines +164 to +165

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@Parameter(names = {"-n", "--numSplits"},
description = "Generate N split points for the fate table and print to stdout. N should always be greater than 1.")
@Parameter(names = {"-n", "--num-splits"},
description = "Generate N split points for the fate table and print to stdout. N must be >= 1.")

int numSplits = 1;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
int numSplits = 1;
int numSplits = -1;

I think we need to default to -1 here so that split generation is opt-in. If we leave it as 1, every fate invocation takes the numSplits > 0 branch and returns before normal commands like --print, --cancel, etc. can run


@Parameter(names = {"-sf", "--splitsFile"},
description = "Write split points to a file. Used with -n or --num-splits.")
String splitsFile = null;
}

private final CountDownLatch lockAcquiredLatch = new CountDownLatch(1);
Expand Down Expand Up @@ -218,6 +234,10 @@ public void execute(JCommander cl, FateOpts options) throws Exception {
Map<FateInstanceType,ReadOnlyFateStore<Fate>> readOnlyFateStores = null;

try {
if (options.numSplits > 0) {
preSplitFateTable(options);
return;
}
if (options.cancel) {
cancelSubmittedFateTxs(context, options.fateIdList);
} else if (options.fail) {
Expand Down Expand Up @@ -267,6 +287,38 @@ public void execute(JCommander cl, FateOpts options) throws Exception {
}
}

private void preSplitFateTable(FateOpts options) throws Exception {
List<String> splits = generateSplits(options.numSplits);

if (options.splitsFile != null) {
try (PrintWriter writer = new PrintWriter(new BufferedWriter(
new OutputStreamWriter(Files.newOutputStream(Path.of(options.splitsFile)), UTF_8)))) {
Comment on lines +294 to +295

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I usually try not to nest constructors in a try-with-resources block. Usually things work out okay but putting each object constructed on its own line makes it more certain that each object will be cleaned up properly in the try-with-resources block as opposed to relying on the closure of the outmost object to cascade down and close the rest.

splits.forEach(writer::println);
}
System.out.println("Wrote " + splits.size() + " split point(s) to " + options.splitsFile);
} else {
splits.forEach(System.out::println);
}
}

static List<String> generateSplits(int numSplits) {
Preconditions.checkArgument(numSplits >= 1,
"Number of splits must be greater than 1. Specifying 0 would generate no splits and leave the table unchanged.",
numSplits);

// Same logic as in FateManager.getDesiredPartitions()
// Work w/ 60 bit unsigned integers to partition the space and then shift over by 4. Used 60
// bits instead of 63 so it nicely aligns w/ hex in the uuid.
long jump = (1L << 60) / (numSplits + 1);
List<String> splits = new ArrayList<>(numSplits);
for (int i = 1; i <= numSplits; i++) {
long start = (i * jump) << 4;
splits.add(new UUID(start, 0).toString());
}

return Collections.unmodifiableList(splits);
}

private FateStores createFateStores(ServerContext context, ZooSession zk, ServiceLock adminLock)
throws InterruptedException, KeeperException {
var lockId = adminLock.getLockID();
Expand Down Expand Up @@ -323,6 +375,13 @@ private void validateFateUserInput(FateOpts cmd) {
throw new IllegalArgumentException(
"At least one txId required when using cancel, fail or delete");
}
if (cmd.numSplits == 0) {
throw new IllegalArgumentException(
"-n / --num-splits must be >= 1. Specifying 0 generates no splits and leaves the table unchanged.");
}
if (cmd.splitsFile != null && cmd.numSplits < 0) {
throw new IllegalArgumentException("-sf requires -n to also be specified.");
}
}

private void cancelSubmittedFateTxs(ServerContext context, List<String> fateIdList)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@
import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.OPID;
import static org.apache.accumulo.core.metadata.schema.TabletMetadata.ColumnType.SELECTED;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
Expand Down Expand Up @@ -151,4 +153,25 @@ public void testDanglingFate() {
assertEquals(Map.of(), found);
}

@Test
public void testGenerateFateSplits() {
// count is correct
assertEquals(4, Fate.generateSplits(4).size());
assertEquals(1, Fate.generateSplits(1).size());

// all valid UUIDs, ascending
List<String> splits = Fate.generateSplits(8);
for (String s : splits) {
assertEquals(s, UUID.fromString(s).toString());
}
for (int i = 0; i < splits.size() - 1; i++) {
assertTrue(splits.get(i).compareTo(splits.get(i + 1)) < 0);
}

// single split is the midpoint (UUID space bisected)
assertEquals(new UUID(Long.MIN_VALUE, 0).toString(), Fate.generateSplits(1).get(0));

// invalid input
assertThrows(IllegalArgumentException.class, () -> Fate.generateSplits(0));
}
}