You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2022/08/10 12:12:39 UTC
[GitHub] [accumulo] dlmarion commented on a diff in pull request #2863: Various modifications to the scan server ITs
dlmarion commented on code in PR #2863:
URL: https://github.com/apache/accumulo/pull/2863#discussion_r942358565
##########
test/src/main/java/org/apache/accumulo/test/ScanServerConcurrentTabletScanIT.java:
##########
@@ -114,8 +114,10 @@ public void testScanSameTabletDifferentDataTabletMetadataCacheEnabled() throws E
client.tableOperations().create(tableName);
// Load 1000 k/v
- ReadWriteIT.ingest(client, 10, 100, 50, 0, "COLA", tableName);
+ final int rowCount = 10, colCount = 100;
+ ReadWriteIT.ingest(client, rowCount, colCount, 50, 0, "COLA", tableName);
Review Comment:
could call ScanServerIT.createTableAndIngest to cover lines 114, 118, and 119 if createTableAndIngest was modified to allow rows and cols to be passed.
##########
test/src/main/java/org/apache/accumulo/test/ScanServerConcurrentTabletScanIT.java:
##########
@@ -134,8 +136,10 @@ public void testScanSameTabletDifferentDataTabletMetadataCacheEnabled() throws E
}
// Load another 100 k/v
- ReadWriteIT.ingest(client, 10, 10, 50, 0, "COLB", tableName);
+ final int rowCount1 = 10, colCount1 = 10;
+ ReadWriteIT.ingest(client, rowCount1, colCount1, 50, 0, "COLB", tableName);
Review Comment:
could call ScanServerIT.createTableAndIngest here too, and just ignore the fact that the table already exists
##########
test/src/main/java/org/apache/accumulo/test/ScanServerIT_NoServers.java:
##########
@@ -106,43 +102,17 @@ public void testBatchScan() throws Exception {
try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) {
String tableName = getUniqueNames(1)[0];
- client.tableOperations().create(tableName);
-
- ReadWriteIT.ingest(client, 10, 10, 50, 0, tableName);
-
- client.tableOperations().flush(tableName, null, null, true);
+ createTableAndIngest(client, tableName);
try (BatchScanner scanner = client.createBatchScanner(tableName, Authorizations.EMPTY)) {
scanner.setRanges(Collections.singletonList(new Range()));
scanner.setConsistencyLevel(ConsistencyLevel.EVENTUAL);
- assertEquals(100, Iterables.size(scanner));
+ assertEquals(EXPECTED_INGEST_ENTRIES_COUNT, Iterables.size(scanner));
ReadWriteIT.ingest(client, 10, 10, 50, 10, tableName);
- // since there are no scan servers and we are reading from tservers we should see update
- assertEquals(200, Iterables.size(scanner));
+ // since there are no scan servers, and we are reading from tservers, we should see update
+ assertEquals(EXPECTED_INGEST_ENTRIES_COUNT * 2, Iterables.size(scanner));
} // when the scanner is closed, all open sessions should be closed
}
}
- @Test
- public void testScanOfflineTable() throws Exception {
Review Comment:
This test class is testing the case where the client has specified ConsistencyLevel.EVENTUAL, but there are no ScanServers running. In this case the scans go to the TabletServer. So, the tests in this class, while appearing to be duplicates, are not. If there is a test somewhere else that validates that you can't scan an offline table in the TabletServer, then I would be ok with removing this.
##########
test/src/main/java/org/apache/accumulo/test/ScanServerConcurrentTabletScanIT.java:
##########
@@ -182,52 +187,56 @@ public void testScanSameTabletDifferentDataTabletMetadataCacheDisabled() throws
client.tableOperations().create(tableName);
// Load 1000 k/v
- ReadWriteIT.ingest(client, 10, 100, 50, 0, "COLA", tableName);
+ final int rowCount = 10, colCount = 100;
+ ReadWriteIT.ingest(client, rowCount, colCount, 50, 0, "COLA", tableName);
Review Comment:
could call ScanServerIT.createTableAndIngest here
##########
test/src/main/java/org/apache/accumulo/test/ScanServerIT.java:
##########
@@ -240,4 +197,18 @@ public void testBatchScannerTimeout() throws Exception {
}
}
}
+
+ protected static void createTableAndIngest(AccumuloClient client, String tableName)
+ throws Exception {
+ createTableAndIngest(client, tableName, new NewTableConfiguration());
+ }
+
+ protected static void createTableAndIngest(AccumuloClient client, String tableName,
Review Comment:
If you modified this to accept the row and column count as method parameters, then you could call this from other classes, for example ScanServerConcurrentTabletScanIT.
##########
test/src/main/java/org/apache/accumulo/test/ScanServerIT.java:
##########
@@ -132,46 +127,20 @@ public void testBatchScan() throws Exception {
try (AccumuloClient client = Accumulo.newClient().from(getClientProps()).build()) {
String tableName = getUniqueNames(1)[0];
- client.tableOperations().create(tableName);
-
- ReadWriteIT.ingest(client, 10, 10, 50, 0, tableName);
-
- client.tableOperations().flush(tableName, null, null, true);
+ createTableAndIngest(client, tableName);
try (BatchScanner scanner = client.createBatchScanner(tableName, Authorizations.EMPTY)) {
scanner.setRanges(Collections.singletonList(new Range()));
scanner.setConsistencyLevel(ConsistencyLevel.EVENTUAL);
- assertEquals(100, Iterables.size(scanner));
+ assertEquals(EXPECTED_INGEST_ENTRIES_COUNT, Iterables.size(scanner));
ReadWriteIT.ingest(client, 10, 10, 50, 10, tableName);
- assertEquals(100, Iterables.size(scanner));
+ assertEquals(EXPECTED_INGEST_ENTRIES_COUNT, Iterables.size(scanner));
scanner.setConsistencyLevel(ConsistencyLevel.IMMEDIATE);
- assertEquals(200, Iterables.size(scanner));
+ assertEquals(EXPECTED_INGEST_ENTRIES_COUNT * 2, Iterables.size(scanner));
} // when the scanner is closed, all open sessions should be closed
}
}
- @Test
- public void testScanOfflineTable() throws Exception {
Review Comment:
I would like to keep this test. It would be easily possible to scan an offline table with ScanServers by moving the call to ClientContext.requireNotOffline to a place further down the scan call stack. If there is another test somewhere that validates that you can't scan an offline table, then I would be in favor of disabling this test vs removing it.
##########
test/src/main/java/org/apache/accumulo/test/ScanServerConcurrentTabletScanIT.java:
##########
@@ -182,52 +187,56 @@ public void testScanSameTabletDifferentDataTabletMetadataCacheDisabled() throws
client.tableOperations().create(tableName);
// Load 1000 k/v
- ReadWriteIT.ingest(client, 10, 100, 50, 0, "COLA", tableName);
+ final int rowCount = 10, colCount = 100;
+ ReadWriteIT.ingest(client, rowCount, colCount, 50, 0, "COLA", tableName);
client.tableOperations().flush(tableName, null, null, true);
+ final int firstBatchOfEntriesCount = rowCount * colCount;
+
+ try (Scanner scanner1 = client.createScanner(tableName, Authorizations.EMPTY)) {
+ scanner1.setRange(new Range());
+ scanner1.setBatchSize(100);
+ scanner1.setReadaheadThreshold(0);
+ scanner1.setConsistencyLevel(ConsistencyLevel.EVENTUAL);
+
+ // iter1 should read 1000 k/v
+ Iterator<Entry<Key,Value>> iter1 = scanner1.iterator();
+
+ // Partially read the data and then start a 2nd scan
+ int count1 = 0;
+ while (iter1.hasNext() && count1 < 10) {
+ iter1.next();
+ count1++;
+ }
- Scanner scanner1 = client.createScanner(tableName, Authorizations.EMPTY);
- scanner1.setRange(new Range());
- scanner1.setBatchSize(100);
- scanner1.setReadaheadThreshold(0);
- scanner1.setConsistencyLevel(ConsistencyLevel.EVENTUAL);
-
- // iter1 should read 1000 k/v
- Iterator<Entry<Key,Value>> iter1 = scanner1.iterator();
-
- // Partially read the data and then start a 2nd scan
- int count1 = 0;
- while (iter1.hasNext() && count1 < 10) {
- iter1.next();
- count1++;
- }
-
- // Load another 100 k/v
- ReadWriteIT.ingest(client, 10, 10, 50, 0, "COLB", tableName);
- client.tableOperations().flush(tableName, null, null, true);
-
- // iter2 should read 1100 k/v because the tablet metadata is not cached.
- Iterator<Entry<Key,Value>> iter2 = scanner1.iterator();
- int count2 = 0;
- boolean useIter1 = true;
-
- do {
- if (useIter1) {
- if (iter1.hasNext()) {
- iter1.next();
- count1++;
+ // Load another 100 k/v
+ final int rowCount1 = 10, colCount1 = 10;
+ ReadWriteIT.ingest(client, rowCount1, colCount1, 50, 0, "COLB", tableName);
+ client.tableOperations().flush(tableName, null, null, true);
Review Comment:
could call ScanServerIT.createTableAndIngest here
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org