Skip to content

fixes multiple tests in ShellServerIT #3785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
keith-turner merged 2 commits into from
Oct 2, 2023
Merged

fixes multiple tests in ShellServerIT #3785

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da748a0
fixes multiple tests in ShellServerIT
keith-turner Sep 27, 2023
df0dfef
Update test/src/main/java/org/apache/accumulo/test/shell/ShellServerI…
keith-turner Sep 29, 2023
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
155 changes: 85 additions & 70 deletions test/src/main/java/org/apache/accumulo/test/shell/ShellServerIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@
import org.apache.accumulo.core.client.IteratorSetting;
import org.apache.accumulo.core.client.IteratorSetting.Column;
import org.apache.accumulo.core.client.Scanner;
import org.apache.accumulo.core.client.TableNotFoundException;
import org.apache.accumulo.core.client.admin.TabletHostingGoal;
import org.apache.accumulo.core.client.sample.RowColumnSampler;
import org.apache.accumulo.core.client.sample.RowSampler;
Expand Down Expand Up @@ -1243,34 +1242,27 @@ public void testSetGoalCommand() throws Exception {
ts.exec("setgoal -t " + table + " -b c -e e -ee -g always");
try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build();
Scanner s = client.createScanner(MetadataTable.NAME, Authorizations.EMPTY)) {
String tableId = client.tableOperations().tableIdMap().get(table);
String tableId = getTableId(table);
s.setRange(new Range(tableId, tableId + "<"));
s.fetchColumn(new Column(HostingColumnFamily.GOAL_COLUMN.getColumnFamily(),
HostingColumnFamily.GOAL_COLUMN.getColumnQualifier()));
for (Entry<Key,Value> e : s) {
switch (e.getKey().getRow().toString()) {
case "1;c":
case "1;e":
assertEquals(TabletHostingGoal.ALWAYS.name(), e.getValue().toString());
break;
case "1;a":
assertEquals(TabletHostingGoal.NEVER.name(), e.getValue().toString());
break;
case "1;g":
case "1<":
// this tablet was loaded ondemand when we executed
// the addsplits command
assertEquals(TabletHostingGoal.ONDEMAND.name(), e.getValue().toString());
break;
default:
fail("Unknown row with hosting goal: " + e.getKey().getRow().toString());
var row = e.getKey().getRow().toString();
if (row.equals(tableId + ";c") || row.equals(tableId + ";e")) {
assertEquals(TabletHostingGoal.ALWAYS.name(), e.getValue().toString());
} else if (row.equals(tableId + ";a")) {
assertEquals(TabletHostingGoal.NEVER.name(), e.getValue().toString());
} else if (row.equals(tableId + ";g") || row.equals(tableId + "<")) {
assertEquals(TabletHostingGoal.ONDEMAND.name(), e.getValue().toString());
} else {
fail("Unknown row with hosting goal: " + e.getKey().getRow().toString());
}
}
}
}

@Test
public void testGetGoalCommand() throws IOException, TableNotFoundException {
public void testGetGoalCommand() throws Exception {

SortedSet<Text> splits =
Sets.newTreeSet(Arrays.asList(new Text("d"), new Text("m"), new Text("s")));
Expand All @@ -1285,32 +1277,34 @@ public void testGetGoalCommand() throws IOException, TableNotFoundException {
String result = ts.exec("getgoal -?");
assertTrue(result.contains("usage: getgoal"));

String tableId = getTableId(tableName);

result = ts.exec("getgoal");
assertTrue(result.contains("TABLE: " + tableName));
assertTrue(result.contains("TABLET ID HOSTING GOAL"));
assertTrue(result.contains("1;d< ONDEMAND"));
assertTrue(result.contains("1;m;d ONDEMAND"));
assertTrue(result.contains("1;s;m ONDEMAND"));
assertTrue(result.contains("1<;s ONDEMAND"));
assertTrue(result.contains(tableId + ";d< ONDEMAND"));
assertTrue(result.contains(tableId + ";m;d ONDEMAND"));
assertTrue(result.contains(tableId + ";s;m ONDEMAND"));
assertTrue(result.contains(tableId + "<;s ONDEMAND"));

ts.exec("setgoal -g ALWAYS -r p");
result = ts.exec("getgoal -r p");
assertFalse(result.contains("1;d< ONDEMAND"));
assertFalse(result.contains("1;m;d ONDEMAND"));
assertTrue(result.contains("1;s;m ALWAYS"));
assertFalse(result.contains("1<;s ONDEMAND"));
assertFalse(result.contains(tableId + ";d< ONDEMAND"));
assertFalse(result.contains(tableId + ";m;d ONDEMAND"));
assertTrue(result.contains(tableId + ";s;m ALWAYS"));
assertFalse(result.contains(tableId + "<;s ONDEMAND"));

result = ts.exec("getgoal");
assertTrue(result.contains("1;d< ONDEMAND"));
assertTrue(result.contains("1;m;d ONDEMAND"));
assertTrue(result.contains("1;s;m ALWAYS"));
assertTrue(result.contains("1<;s ONDEMAND"));
assertTrue(result.contains(tableId + ";d< ONDEMAND"));
assertTrue(result.contains(tableId + ";m;d ONDEMAND"));
assertTrue(result.contains(tableId + ";s;m ALWAYS"));
assertTrue(result.contains(tableId + "<;s ONDEMAND"));

result = ts.exec("getgoal -b f -e p");
assertFalse(result.contains("1;d< ONDEMAND"));
assertTrue(result.contains("1;m;d ONDEMAND"));
assertTrue(result.contains("1;s;m ALWAYS"));
assertFalse(result.contains("1<;s ONDEMAND"));
assertFalse(result.contains(tableId + ";d< ONDEMAND"));
assertTrue(result.contains(tableId + ";m;d ONDEMAND"));
assertTrue(result.contains(tableId + ";s;m ALWAYS"));
assertFalse(result.contains(tableId + "<;s ONDEMAND"));

} finally {
if (splitsFilePath != null) {
Expand All @@ -1321,51 +1315,61 @@ public void testGetGoalCommand() throws IOException, TableNotFoundException {

// Verify that when splits are added after table creation, hosting goals are set properly
@Test
public void testGetGoalCommand_DelayedSplits() throws IOException {
public void testGetGoalCommand_DelayedSplits() throws Exception {

for (int i = 0; i < 40; i++) {
ts.exec("createtable tab" + i, true);
}

final String[] tableName = getUniqueNames(2);

ts.exec("createtable " + tableName[0], true);

String tableId = getTableId(tableName[0]);

String result = ts.exec("getgoal");

assertTrue(result.contains("TABLE: " + tableName[0]));
assertTrue(result.contains("TABLET ID HOSTING GOAL"));
assertTrue(result.contains("1<< ONDEMAND"));
assertTrue(result.matches("(?s).*" + tableId + "<<\\s+ONDEMAND.*"));
Copy link
Copy Markdown
Member

@ctubbsii ctubbsii Sep 28, 2023

Choose a reason for hiding this comment

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

Are the lines in the output coming in an unpredictable order? Is there a reason why we're doing pattern matching anywhere in the multi-line string result for the lines we expect, instead of just iterating over the lines and checking to verify each line contains the next thing we expect?

Something like:

    Iterator<String> iter = splitLines(result);
    line = iter.next(); assertTrue(line.contains(firstLineCondition));
    line = iter.next(); assertTrue(line.contains(nextLineCondition));
    line = iter.next(); assertTrue(line.contains(nextLineCondition));
    // ...
    assertFalse(iter.hasNext());

This seems simpler to read and maintain than a bunch of regexes that don't verify that the output is appearing in the expected order.

Copy link
Copy Markdown
Contributor Author

@keith-turner keith-turner Sep 29, 2023

Choose a reason for hiding this comment

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

The reason I did pattern matching was because the number of spaces in the output would change when the length of the table id changed. So for a one character table id would see more spaces in the output than if there was a two digit table id.

As for checking the order, that could be a follow on issue. It would make the test more strict. The test was not checking the order before these changes.


// add the splits and check goals again
ts.exec("addsplits d m s", true);
result = ts.exec("getgoal");
assertTrue(result.contains("1;d< ONDEMAND"));
assertTrue(result.contains("1;m;d ONDEMAND"));
assertTrue(result.contains("1;s;m ONDEMAND"));
assertTrue(result.contains("1<;s ONDEMAND"));
assertTrue(result.matches("(?s).*" + tableId + "[;]d<\\s+ONDEMAND.*"));
assertTrue(result.matches("(?s).*" + tableId + ";m;d\\s+ONDEMAND.*"));
assertTrue(result.matches("(?s).*" + tableId + ";s;m\\s+ONDEMAND.*"));
assertTrue(result.matches("(?s).*" + tableId + "<;s\\s+ONDEMAND.*"));

// scan metadata table to be sure the hosting goals were properly set
result = ts.exec("scan -t accumulo.metadata -c hosting:goal", true);
assertTrue(result.contains("1;d hosting:goal []\tONDEMAND"));
assertTrue(result.contains("1;m hosting:goal []\tONDEMAND"));
assertTrue(result.contains("1;s hosting:goal []\tONDEMAND"));
assertTrue(result.contains("1< hosting:goal []\tONDEMAND"));
result = ts.exec("scan -t accumulo.metadata -c hosting:goal -b " + tableId, true);
assertTrue(result.contains(tableId + ";d hosting:goal []\tONDEMAND"));
assertTrue(result.contains(tableId + ";m hosting:goal []\tONDEMAND"));
assertTrue(result.contains(tableId + ";s hosting:goal []\tONDEMAND"));
assertTrue(result.contains(tableId + "< hosting:goal []\tONDEMAND"));

ts.exec("createtable " + tableName[1] + " -g always", true);

String tableId2 = getTableId(tableName[1]);

result = ts.exec("getgoal");
assertTrue(result.contains("TABLE: " + tableName[1]));
assertTrue(result.contains("TABLET ID HOSTING GOAL"));
assertTrue(result.contains("2<< ALWAYS"));
assertTrue(result.matches("(?s).*" + tableId2 + "<<\\s+ALWAYS.*"));

ts.exec("addsplits d m s", true);
result = ts.exec("getgoal");
assertTrue(result.contains("2;d< ALWAYS"));
assertTrue(result.contains("2;m;d ALWAYS"));
assertTrue(result.contains("2;s;m ALWAYS"));
assertTrue(result.contains("2<;s ALWAYS"));
assertTrue(result.matches("(?s).*" + tableId2 + ";d<\\s+ALWAYS.*"));
assertTrue(result.matches("(?s).*" + tableId2 + ";m;d\\s+ALWAYS.*"));
assertTrue(result.matches("(?s).*" + tableId2 + ";s;m\\s+ALWAYS.*"));
assertTrue(result.matches("(?s).*" + tableId2 + "<;s\\s+ALWAYS.*"));

// scan metadata table to be sure the hosting goals were properly set
result = ts.exec("scan -t accumulo.metadata -c hosting:goal -b 2", true);
assertTrue(result.contains("2;d hosting:goal []\tALWAYS"));
assertTrue(result.contains("2;m hosting:goal []\tALWAYS"));
assertTrue(result.contains("2;s hosting:goal []\tALWAYS"));
assertTrue(result.contains("2< hosting:goal []\tALWAYS"));

result = ts.exec("scan -t accumulo.metadata -c hosting:goal -b " + tableId2, true);
assertTrue(result.contains(tableId2 + ";d hosting:goal []\tALWAYS"));
assertTrue(result.contains(tableId2 + ";m hosting:goal []\tALWAYS"));
assertTrue(result.contains(tableId2 + ";s hosting:goal []\tALWAYS"));
assertTrue(result.contains(tableId2 + "< hosting:goal []\tALWAYS"));
}

@Test
Expand Down Expand Up @@ -1841,8 +1845,8 @@ public void scansWithNeverHostedTablets() throws Exception {
final String table = getUniqueNames(1)[0];
ts.exec("createtable " + table);
ts.exec("addsplits -t " + table + " a c e g m t");
ts.exec("goal -t " + table + " -b a -e a -g never");
ts.exec("goal -t " + table + " -b c -e e -ee -g always");
ts.exec("setgoal -t " + table + " -b a -e a -g never");
Copy link
Copy Markdown
Member

@ctubbsii ctubbsii Sep 28, 2023

Choose a reason for hiding this comment

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

I think setgoal is not a very good name for a tablet-specific function, as it makes one wonder "goal for what?", because "goal" is too generic of a term.

I think "hosttablet ... always/never/ondemand" or "settabletgoal ... always/never/ondemand". Even these are probably not the best names either, but they give a lot more context to what kind of goal is being set.

Alternatively, one could have a command named "tablet" and a sub-command called something like "online", so you get a very expressive command like: "tablet online... always/never/ondemand" or maybe "tablet hosted... always/never/ondemand".

Or we could overload the current online command to be able to set a range, and an optional flag for ondemand. So, you get the following:

> online -t table -b beginRange -e endRange             # "always"
> offline -t table -b beginRange -e endRange            # "never"
> online -t table -b beginRange -e endRange --ondemand  # "ondemand"

Copy link
Copy Markdown
Member

@ctubbsii ctubbsii Sep 28, 2023

Choose a reason for hiding this comment

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

Elsewhere, we call this "hosting goal", so "sethostinggoal" would be appropriate as well.

Copy link
Copy Markdown
Contributor Author

@keith-turner keith-turner Sep 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@ctubbsii the name of that command is out of scope for this PR, but what you wrote sounds reasonable. Can you open an issue for reconsidering the name?

Copy link
Copy Markdown
Contributor Author

@keith-turner keith-turner Oct 2, 2023

Choose a reason for hiding this comment

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

Opened #3800

ts.exec("setgoal -t " + table + " -b c -e e -ee -g always");

ts.exec("scan -t " + table + " -np -b a -e c", false);
ts.exec("scan -t " + table + " -np -b a -e e", false);
Expand Down Expand Up @@ -2263,7 +2267,7 @@ public void testSummarySelection() throws Exception {
// This test serves to verify the listtablets command as well as the getTabletInformation api,
// which is used by listtablets.
@Test
public void testListTablets() throws IOException, InterruptedException {
public void testListTablets() throws Exception {

final var tables = getUniqueNames(2);
final String table1 = tables[0];
Expand Down Expand Up @@ -2294,22 +2298,33 @@ public void testListTablets() throws IOException, InterruptedException {
}
}

var tableId1 = getTableId(table1);
var tableId2 = getTableId(table2);

String results = ts.exec("listtablets -np -p ShellServerIT_testListTablets.", true);
assertTrue(results.contains("TABLE: ShellServerIT_testListTablets0"));
assertTrue(results.contains("TABLE: ShellServerIT_testListTablets1"));
assertTrue(results.contains("1 -INF g ALWAYS"));
assertTrue(results.contains("1 g n ONDEMAND"));
assertTrue(results.contains("1 n u ALWAYS"));
assertTrue(results.contains("1 u +INF ONDEMAND"));
assertTrue(results.contains("2 -INF f ONDEMAND"));
assertTrue(results.contains("2 f m NEVER"));
assertTrue(results.contains("2 m t ALWAYS"));
assertTrue(results.contains("2 t +INF ONDEMAND"));
assertTrue(
results.contains(tableId1 + " -INF g ALWAYS"));
assertTrue(
results.contains(tableId1 + " g n ONDEMAND"));
assertTrue(
results.contains(tableId1 + " n u ALWAYS"));
assertTrue(
results.contains(tableId1 + " u +INF ONDEMAND"));
assertTrue(
results.contains(tableId2 + " -INF f ONDEMAND"));
assertTrue(results.contains(tableId2 + " f m NEVER"));
assertTrue(
results.contains(tableId2 + " m t ALWAYS"));
assertTrue(
results.contains(tableId2 + " t +INF ONDEMAND"));

// verify the sum of the tablets sizes, number of entries, and dir name match the data in a
// metadata scan
String metadata = ts.exec("scan -np -t accumulo.metadata -b 1 -c loc,file");
String metadata = ts.exec("scan -np -t accumulo.metadata -b " + tableId1 + " -c loc,file");
for (String line : metadata.split("\n")) {
System.out.println(line);
String[] tokens = line.split("\\s+");
if (tokens[1].startsWith("loc")) {
String loc = tokens[3];
Expand All @@ -2318,7 +2333,7 @@ public void testListTablets() throws IOException, InterruptedException {
if (tokens[1].startsWith("file")) {
String[] parts = tokens[1].split("/");
String dir = parts[parts.length - 2];
assertTrue(results.contains(dir));
assertTrue(results.contains(dir), "Did not see " + dir);
String[] sizes = tokens[3].split(",");
String size = String.format("%,d", Integer.parseInt(sizes[0]));
String entries = String.format("%,d", Integer.parseInt(sizes[1]));
Expand Down