You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "kevinrr888 (via GitHub)" <gi...@apache.org> on 2025/05/01 15:18:56 UTC
[PR] IteratorEnvironment bug fixes and improved testing [accumulo]
kevinrr888 opened a new pull request, #5525:
URL: https://github.com/apache/accumulo/pull/5525
- Improved IteratorEnvIT:
- Added means of verifying that the checks done in testEnv() actually ran
- Added testing for more usages of IteratorEnvironment, specifically all usages of the new ClientIteratorEnvironment
- Added more env tests in testEnv()
- Avoid repeated strings throughout IT
- Simplified assertions (can simply use junit assertions as assertions work outside of testing process)
- Fixed some of the bugs found with new ClientIteratorEnvironment when adding more testing
- Javadoc fixes and improvements
closes #5504
--
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
Re: [PR] IteratorEnvironment bug fixes and improved testing [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 merged PR #5525:
URL: https://github.com/apache/accumulo/pull/5525
--
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
Re: [PR] IteratorEnvironment bug fixes and improved testing [accumulo]
Posted by "kevinrr888 (via GitHub)" <gi...@apache.org>.
kevinrr888 commented on code in PR #5525:
URL: https://github.com/apache/accumulo/pull/5525#discussion_r2070431134
##########
test/src/main/java/org/apache/accumulo/test/IteratorEnvIT.java:
##########
@@ -71,121 +92,123 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration hadoo
/**
* Basic scan iterator to test IteratorEnvironment returns what is expected.
*/
- public static class ScanIter extends WrappingIterator {
+ public static class ScanIter extends Filter {
IteratorScope scope = IteratorScope.scan;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
-
// Checking for compaction on a scan should throw an error.
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Basic compaction iterator to test IteratorEnvironment returns what is expected.
*/
- public static class MajcIter extends WrappingIterator {
+ public static class MajcIter extends Filter {
IteratorScope scope = IteratorScope.majc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
- try {
- env.isFullMajorCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
+ // Checking for compaction on majc should not throw an error.
+ testEnv(scope, options, env, false);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
*
*/
- public static class MincIter extends WrappingIterator {
+ public static class MincIter extends Filter {
IteratorScope scope = IteratorScope.minc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ // Checking for compaction on minc should throw an error.
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Test the environment methods return what is expected.
*/
- private static void testEnv(IteratorScope scope, Map<String,String> opts,
- IteratorEnvironment env) {
- TableId expectedTableId = TableId.of(opts.get("expected.table.id"));
+ private static void testEnv(IteratorScope scope, Map<String,String> opts, IteratorEnvironment env,
+ boolean checkCompactionShouldThrow) {
+ String expTableIdString = opts.get(EXPECTED_TABLE_ID_OPT);
+ TableId expectedTableId = expTableIdString == null ? null : TableId.of(expTableIdString);
+
+ if (checkCompactionShouldThrow) {
+ assertThrows(IllegalStateException.class, env::isUserCompaction,
+ "Test failed - Expected to throw IllegalStateException when checking compaction");
+ assertThrows(IllegalStateException.class, env::isFullMajorCompaction,
+ "Test failed - Expected to throw IllegalStateException when checking compaction");
+ } else {
+ assertDoesNotThrow(env::isUserCompaction,
+ "Test failed - Expected not to throw exception when checking compaction");
+ assertDoesNotThrow(env::isFullMajorCompaction,
+ "Test failed - Expected not to throw exception when checking compaction");
+ }
// verify getServiceEnv() and getPluginEnv() are the same objects,
// so further checks only need to use getPluginEnv()
@SuppressWarnings("deprecation")
ServiceEnvironment serviceEnv = env.getServiceEnv();
PluginEnvironment pluginEnv = env.getPluginEnv();
- if (serviceEnv != pluginEnv) {
- throw new RuntimeException("Test failed - assertSame(getServiceEnv(),getPluginEnv())");
- }
+ assertEquals(serviceEnv, pluginEnv, "Test failed - assertSame(getServiceEnv(),getPluginEnv())");
// verify property exists on the table config (deprecated and new),
// with and without custom prefix, but not in the system config
@SuppressWarnings("deprecation")
- String accTableConf = env.getConfig().get("table.custom.iterator.env.test");
- if (!"value1".equals(accTableConf)) {
- throw new RuntimeException("Test failed - Expected table property not found in getConfig().");
- }
+ String accTableConf = env.getConfig().get(CUSTOM_PROP_KEY);
+ assertEquals(CUSTOM_PROP_VAL, accTableConf,
+ "Test failed - Expected table property not found in getConfig().");
var tableConf = pluginEnv.getConfiguration(env.getTableId());
- if (!"value1".equals(tableConf.get("table.custom.iterator.env.test"))) {
- throw new RuntimeException("Test failed - Expected table property not found in table conf.");
- }
- if (!"value1".equals(tableConf.getTableCustom("iterator.env.test"))) {
- throw new RuntimeException("Test failed - Expected table property not found in table conf.");
- }
+ assertEquals(CUSTOM_PROP_VAL, tableConf.get(CUSTOM_PROP_KEY),
+ "Test failed - Expected table property not found in table conf.");
+ assertEquals(CUSTOM_PROP_VAL, tableConf.getTableCustom(CUSTOM_PROP_KEY_SUFFIX),
+ "Test failed - Expected table property not found in table conf.");
var systemConf = pluginEnv.getConfiguration();
- if (systemConf.get("table.custom.iterator.env.test") != null) {
- throw new RuntimeException("Test failed - Unexpected table property found in system conf.");
- }
+ assertNull(systemConf.get(CUSTOM_PROP_KEY),
+ "Test failed - Unexpected table property found in system conf.");
// check other environment settings
- if (!scope.equals(env.getIteratorScope())) {
- throw new RuntimeException("Test failed - Error getting iterator scope");
- }
- if (env.isSamplingEnabled()) {
- throw new RuntimeException("Test failed - isSamplingEnabled returned true, expected false");
- }
- if (!expectedTableId.equals(env.getTableId())) {
- throw new RuntimeException("Test failed - Error getting Table ID");
+ assertEquals(scope, env.getIteratorScope(), "Test failed - Error getting iterator scope");
+ assertFalse(env.isSamplingEnabled(),
+ "Test failed - isSamplingEnabled returned true, expected false");
+ assertEquals(expectedTableId, env.getTableId(), "Test failed - Error getting Table ID");
+ assertNull(env.getSamplerConfiguration());
+ assertThrows(Exception.class, env::cloneWithSamplingEnabled);
Review Comment:
should we configure sampling for the tables in this test somehow and verify the result of env.cloneWithSamplingEnabled? Or is this check sufficient for this test
--
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
Re: [PR] IteratorEnvironment bug fixes and improved testing [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5525:
URL: https://github.com/apache/accumulo/pull/5525#discussion_r2070630086
##########
test/src/main/java/org/apache/accumulo/test/IteratorEnvIT.java:
##########
@@ -71,121 +92,123 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration hadoo
/**
* Basic scan iterator to test IteratorEnvironment returns what is expected.
*/
- public static class ScanIter extends WrappingIterator {
+ public static class ScanIter extends Filter {
IteratorScope scope = IteratorScope.scan;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
-
// Checking for compaction on a scan should throw an error.
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Basic compaction iterator to test IteratorEnvironment returns what is expected.
*/
- public static class MajcIter extends WrappingIterator {
+ public static class MajcIter extends Filter {
IteratorScope scope = IteratorScope.majc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
- try {
- env.isFullMajorCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
+ // Checking for compaction on majc should not throw an error.
+ testEnv(scope, options, env, false);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
*
*/
- public static class MincIter extends WrappingIterator {
+ public static class MincIter extends Filter {
IteratorScope scope = IteratorScope.minc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ // Checking for compaction on minc should throw an error.
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Test the environment methods return what is expected.
*/
- private static void testEnv(IteratorScope scope, Map<String,String> opts,
- IteratorEnvironment env) {
- TableId expectedTableId = TableId.of(opts.get("expected.table.id"));
+ private static void testEnv(IteratorScope scope, Map<String,String> opts, IteratorEnvironment env,
+ boolean checkCompactionShouldThrow) {
+ String expTableIdString = opts.get(EXPECTED_TABLE_ID_OPT);
+ TableId expectedTableId = expTableIdString == null ? null : TableId.of(expTableIdString);
+
+ if (checkCompactionShouldThrow) {
+ assertThrows(IllegalStateException.class, env::isUserCompaction,
+ "Test failed - Expected to throw IllegalStateException when checking compaction");
+ assertThrows(IllegalStateException.class, env::isFullMajorCompaction,
+ "Test failed - Expected to throw IllegalStateException when checking compaction");
+ } else {
+ assertDoesNotThrow(env::isUserCompaction,
+ "Test failed - Expected not to throw exception when checking compaction");
+ assertDoesNotThrow(env::isFullMajorCompaction,
+ "Test failed - Expected not to throw exception when checking compaction");
+ }
// verify getServiceEnv() and getPluginEnv() are the same objects,
// so further checks only need to use getPluginEnv()
@SuppressWarnings("deprecation")
ServiceEnvironment serviceEnv = env.getServiceEnv();
PluginEnvironment pluginEnv = env.getPluginEnv();
- if (serviceEnv != pluginEnv) {
- throw new RuntimeException("Test failed - assertSame(getServiceEnv(),getPluginEnv())");
- }
+ assertEquals(serviceEnv, pluginEnv, "Test failed - assertSame(getServiceEnv(),getPluginEnv())");
// verify property exists on the table config (deprecated and new),
// with and without custom prefix, but not in the system config
@SuppressWarnings("deprecation")
- String accTableConf = env.getConfig().get("table.custom.iterator.env.test");
- if (!"value1".equals(accTableConf)) {
- throw new RuntimeException("Test failed - Expected table property not found in getConfig().");
- }
+ String accTableConf = env.getConfig().get(CUSTOM_PROP_KEY);
+ assertEquals(CUSTOM_PROP_VAL, accTableConf,
+ "Test failed - Expected table property not found in getConfig().");
var tableConf = pluginEnv.getConfiguration(env.getTableId());
- if (!"value1".equals(tableConf.get("table.custom.iterator.env.test"))) {
- throw new RuntimeException("Test failed - Expected table property not found in table conf.");
- }
- if (!"value1".equals(tableConf.getTableCustom("iterator.env.test"))) {
- throw new RuntimeException("Test failed - Expected table property not found in table conf.");
- }
+ assertEquals(CUSTOM_PROP_VAL, tableConf.get(CUSTOM_PROP_KEY),
+ "Test failed - Expected table property not found in table conf.");
+ assertEquals(CUSTOM_PROP_VAL, tableConf.getTableCustom(CUSTOM_PROP_KEY_SUFFIX),
+ "Test failed - Expected table property not found in table conf.");
var systemConf = pluginEnv.getConfiguration();
- if (systemConf.get("table.custom.iterator.env.test") != null) {
- throw new RuntimeException("Test failed - Unexpected table property found in system conf.");
- }
+ assertNull(systemConf.get(CUSTOM_PROP_KEY),
+ "Test failed - Unexpected table property found in system conf.");
// check other environment settings
- if (!scope.equals(env.getIteratorScope())) {
- throw new RuntimeException("Test failed - Error getting iterator scope");
- }
- if (env.isSamplingEnabled()) {
- throw new RuntimeException("Test failed - isSamplingEnabled returned true, expected false");
- }
- if (!expectedTableId.equals(env.getTableId())) {
- throw new RuntimeException("Test failed - Error getting Table ID");
+ assertEquals(scope, env.getIteratorScope(), "Test failed - Error getting iterator scope");
+ assertFalse(env.isSamplingEnabled(),
+ "Test failed - isSamplingEnabled returned true, expected false");
+ assertEquals(expectedTableId, env.getTableId(), "Test failed - Error getting Table ID");
+ assertNull(env.getSamplerConfiguration());
+ assertThrows(Exception.class, env::cloneWithSamplingEnabled);
Review Comment:
good question for @keith-turner
--
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
Re: [PR] IteratorEnvironment bug fixes and improved testing [accumulo]
Posted by "DomGarguilo (via GitHub)" <gi...@apache.org>.
DomGarguilo commented on code in PR #5525:
URL: https://github.com/apache/accumulo/pull/5525#discussion_r2072070556
##########
test/src/main/java/org/apache/accumulo/test/IteratorEnvIT.java:
##########
@@ -227,43 +253,134 @@ public void testCompact(String tableName,
writeData(tableName);
IteratorSetting cfg = new IteratorSetting(1, iteratorClass);
- cfg.addOption("expected.table.id", client.tableOperations().tableIdMap().get(tableName));
+ cfg.addOption(EXPECTED_TABLE_ID_OPT, client.tableOperations().tableIdMap().get(tableName));
CompactionConfig config = new CompactionConfig();
config.setIterators(Collections.singletonList(cfg));
client.tableOperations().compact(tableName, config);
+
+ try (Scanner scan = client.createScanner(tableName)) {
+ validateScanner(scan);
+ }
}
public void testMinCompact(String tableName,
Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass) throws Exception {
writeData(tableName);
IteratorSetting cfg = new IteratorSetting(1, iteratorClass);
- cfg.addOption("expected.table.id", client.tableOperations().tableIdMap().get(tableName));
+ cfg.addOption(EXPECTED_TABLE_ID_OPT, client.tableOperations().tableIdMap().get(tableName));
client.tableOperations().attachIterator(tableName, cfg, EnumSet.of(IteratorScope.minc));
- client.tableOperations().flush(tableName);
+ client.tableOperations().flush(tableName, null, null, true);
+
+ try (Scanner scan = client.createScanner(tableName)) {
+ validateScanner(scan);
+ }
+ }
+
+ private void testRFileScan(Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass)
+ throws Exception {
+ TreeMap<Key,Value> data = createTestData();
+ var fs = FileSystem.getLocal(new Configuration());
+ String rFilePath = createRFile(fs, data);
+
+ IteratorSetting cfg = new IteratorSetting(1, iteratorClass);
+
+ try (Scanner scan = RFile.newScanner().from(rFilePath).withFileSystem(fs)
+ .withTableProperties(getTableConfig().getProperties()).build()) {
+ scan.addScanIterator(cfg);
+ validateScanner(scan);
+ }
+ }
+
+ public void testOfflineScan(String tableName,
+ Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass) throws Exception {
+ writeData(tableName);
+
+ TableId tableId = TableId.of(client.tableOperations().tableIdMap().get(tableName));
+ client.tableOperations().offline(tableName, true);
+
+ IteratorSetting cfg = new IteratorSetting(1, iteratorClass);
+ cfg.addOption(EXPECTED_TABLE_ID_OPT, tableId.canonical());
+
+ try (OfflineScanner scan =
+ new OfflineScanner((ClientContext) client, tableId, Authorizations.EMPTY)) {
+ scan.addScanIterator(cfg);
+ validateScanner(scan);
+ }
+ }
+
+ public void testClientSideScan(String tableName,
+ Class<? extends SortedKeyValueIterator<Key,Value>> iteratorClass) throws Exception {
+ writeData(tableName);
+
+ IteratorSetting cfg = new IteratorSetting(1, iteratorClass);
+ cfg.addOption(EXPECTED_TABLE_ID_OPT, client.tableOperations().tableIdMap().get(tableName));
+
+ try (Scanner scan = client.createScanner(tableName);
+ var clientIterScan = new ClientSideIteratorScanner(scan)) {
+ clientIterScan.addScanIterator(cfg);
+ validateScanner(clientIterScan);
+ }
+ }
+
+ private TreeMap<Key,Value> createTestData() {
+ TreeMap<Key,Value> testData = new TreeMap<>();
+
+ // Write data that we do not expect to be filtered out
+ testData.put(new Key("row1", GOOD_COL_FAM, "cq1"), new Value("val1"));
+ testData.put(new Key("row2", GOOD_COL_FAM, "cq1"), new Value("val2"));
+ testData.put(new Key("row3", GOOD_COL_FAM, "cq1"), new Value("val3"));
+ // Write data that we expect to be filtered out
+ testData.put(new Key("row4", BAD_COL_FAM, "badcq"), new Value("val1"));
+ testData.put(new Key("row5", BAD_COL_FAM, "badcq"), new Value("val2"));
+ testData.put(new Key("row6", BAD_COL_FAM, "badcq"), new Value("val3"));
+
+ return testData;
+ }
+
+ private void validateScanner(Scanner scan) {
+ // Ensure the bad cf was filtered out to ensure init() and testEnv() were called
+ int numElts = 0;
+ for (var e : scan) {
+ numElts++;
+ assertEquals(GOOD_COL_FAM, e.getKey().getColumnFamily().toString());
+ }
+ assertEquals(3, numElts);
+ }
+
+ private String createRFile(FileSystem fs, TreeMap<Key,Value> data) throws Exception {
+ File dir = new File(System.getProperty("user.dir") + "/target/rfilescan-iterenv-test");
+ assertTrue(dir.mkdirs());
+ String filePath = dir.getAbsolutePath() + "/test.rf";
+
+ try (RFileWriter writer = RFile.newWriter().to(filePath).withFileSystem(fs).build()) {
+ writer.append(data.entrySet());
+ }
+
+ fs.deleteOnExit(new Path(dir.getAbsolutePath()));
Review Comment:
This could be a good spot for Junits `@TempDir` potentially. Doesn't make a big difference but it'll do the setup and cleanup for us.
You would just need to add to the top:
```java
@TempDir
private static File tempDir;
```
```suggestion
File testFile = new File(tempDir, "test.rf");
String filePath = testFile.getAbsolutePath();
try (RFileWriter writer = RFile.newWriter().to(filePath).withFileSystem(fs).build()) {
writer.append(data.entrySet());
}
```
--
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
Re: [PR] IteratorEnvironment bug fixes and improved testing [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5525:
URL: https://github.com/apache/accumulo/pull/5525#discussion_r2073734660
##########
test/src/main/java/org/apache/accumulo/test/IteratorEnvIT.java:
##########
@@ -71,121 +92,123 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration hadoo
/**
* Basic scan iterator to test IteratorEnvironment returns what is expected.
*/
- public static class ScanIter extends WrappingIterator {
+ public static class ScanIter extends Filter {
IteratorScope scope = IteratorScope.scan;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
-
// Checking for compaction on a scan should throw an error.
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Basic compaction iterator to test IteratorEnvironment returns what is expected.
*/
- public static class MajcIter extends WrappingIterator {
+ public static class MajcIter extends Filter {
IteratorScope scope = IteratorScope.majc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
- try {
- env.isFullMajorCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
+ // Checking for compaction on majc should not throw an error.
+ testEnv(scope, options, env, false);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
*
*/
- public static class MincIter extends WrappingIterator {
+ public static class MincIter extends Filter {
IteratorScope scope = IteratorScope.minc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ // Checking for compaction on minc should throw an error.
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Test the environment methods return what is expected.
*/
- private static void testEnv(IteratorScope scope, Map<String,String> opts,
- IteratorEnvironment env) {
- TableId expectedTableId = TableId.of(opts.get("expected.table.id"));
+ private static void testEnv(IteratorScope scope, Map<String,String> opts, IteratorEnvironment env,
+ boolean checkCompactionShouldThrow) {
+ String expTableIdString = opts.get(EXPECTED_TABLE_ID_OPT);
+ TableId expectedTableId = expTableIdString == null ? null : TableId.of(expTableIdString);
+
+ if (checkCompactionShouldThrow) {
+ assertThrows(IllegalStateException.class, env::isUserCompaction,
+ "Test failed - Expected to throw IllegalStateException when checking compaction");
+ assertThrows(IllegalStateException.class, env::isFullMajorCompaction,
+ "Test failed - Expected to throw IllegalStateException when checking compaction");
+ } else {
+ assertDoesNotThrow(env::isUserCompaction,
+ "Test failed - Expected not to throw exception when checking compaction");
+ assertDoesNotThrow(env::isFullMajorCompaction,
+ "Test failed - Expected not to throw exception when checking compaction");
+ }
// verify getServiceEnv() and getPluginEnv() are the same objects,
// so further checks only need to use getPluginEnv()
@SuppressWarnings("deprecation")
ServiceEnvironment serviceEnv = env.getServiceEnv();
PluginEnvironment pluginEnv = env.getPluginEnv();
- if (serviceEnv != pluginEnv) {
- throw new RuntimeException("Test failed - assertSame(getServiceEnv(),getPluginEnv())");
- }
+ assertEquals(serviceEnv, pluginEnv, "Test failed - assertSame(getServiceEnv(),getPluginEnv())");
// verify property exists on the table config (deprecated and new),
// with and without custom prefix, but not in the system config
@SuppressWarnings("deprecation")
- String accTableConf = env.getConfig().get("table.custom.iterator.env.test");
- if (!"value1".equals(accTableConf)) {
- throw new RuntimeException("Test failed - Expected table property not found in getConfig().");
- }
+ String accTableConf = env.getConfig().get(CUSTOM_PROP_KEY);
+ assertEquals(CUSTOM_PROP_VAL, accTableConf,
+ "Test failed - Expected table property not found in getConfig().");
var tableConf = pluginEnv.getConfiguration(env.getTableId());
- if (!"value1".equals(tableConf.get("table.custom.iterator.env.test"))) {
- throw new RuntimeException("Test failed - Expected table property not found in table conf.");
- }
- if (!"value1".equals(tableConf.getTableCustom("iterator.env.test"))) {
- throw new RuntimeException("Test failed - Expected table property not found in table conf.");
- }
+ assertEquals(CUSTOM_PROP_VAL, tableConf.get(CUSTOM_PROP_KEY),
+ "Test failed - Expected table property not found in table conf.");
+ assertEquals(CUSTOM_PROP_VAL, tableConf.getTableCustom(CUSTOM_PROP_KEY_SUFFIX),
+ "Test failed - Expected table property not found in table conf.");
var systemConf = pluginEnv.getConfiguration();
- if (systemConf.get("table.custom.iterator.env.test") != null) {
- throw new RuntimeException("Test failed - Unexpected table property found in system conf.");
- }
+ assertNull(systemConf.get(CUSTOM_PROP_KEY),
+ "Test failed - Unexpected table property found in system conf.");
// check other environment settings
- if (!scope.equals(env.getIteratorScope())) {
- throw new RuntimeException("Test failed - Error getting iterator scope");
- }
- if (env.isSamplingEnabled()) {
- throw new RuntimeException("Test failed - isSamplingEnabled returned true, expected false");
- }
- if (!expectedTableId.equals(env.getTableId())) {
- throw new RuntimeException("Test failed - Error getting Table ID");
+ assertEquals(scope, env.getIteratorScope(), "Test failed - Error getting iterator scope");
+ assertFalse(env.isSamplingEnabled(),
+ "Test failed - isSamplingEnabled returned true, expected false");
+ assertEquals(expectedTableId, env.getTableId(), "Test failed - Error getting Table ID");
+ assertNull(env.getSamplerConfiguration());
+ assertThrows(Exception.class, env::cloneWithSamplingEnabled);
Review Comment:
I can look into generally improving test when I look at #5500. I was just thinking of RFileScanner when I opened that issue, but I can look at making it more general.
##########
test/src/main/java/org/apache/accumulo/test/IteratorEnvIT.java:
##########
@@ -71,121 +92,123 @@ public void configureMiniCluster(MiniAccumuloConfigImpl cfg, Configuration hadoo
/**
* Basic scan iterator to test IteratorEnvironment returns what is expected.
*/
- public static class ScanIter extends WrappingIterator {
+ public static class ScanIter extends Filter {
IteratorScope scope = IteratorScope.scan;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
-
// Checking for compaction on a scan should throw an error.
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Basic compaction iterator to test IteratorEnvironment returns what is expected.
*/
- public static class MajcIter extends WrappingIterator {
+ public static class MajcIter extends Filter {
IteratorScope scope = IteratorScope.majc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
- try {
- env.isFullMajorCompaction();
- } catch (IllegalStateException e) {
- throw new RuntimeException("Test failed");
- }
+ // Checking for compaction on majc should not throw an error.
+ testEnv(scope, options, env, false);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
*
*/
- public static class MincIter extends WrappingIterator {
+ public static class MincIter extends Filter {
IteratorScope scope = IteratorScope.minc;
@Override
public void init(SortedKeyValueIterator<Key,Value> source, Map<String,String> options,
IteratorEnvironment env) throws IOException {
super.init(source, options, env);
- testEnv(scope, options, env);
- try {
- env.isUserCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
- try {
- env.isFullMajorCompaction();
- throw new RuntimeException(
- "Test failed - Expected to throw IllegalStateException when checking compaction on a scan.");
- } catch (IllegalStateException e) {}
+ // Checking for compaction on minc should throw an error.
+ testEnv(scope, options, env, true);
+ }
+
+ @Override
+ public boolean accept(Key k, Value v) {
+ // The only reason for filtering out some data is as a way to verify init() and testEnv()
+ // have been called
+ return !k.getColumnFamily().toString().equals(BAD_COL_FAM);
}
}
/**
* Test the environment methods return what is expected.
*/
- private static void testEnv(IteratorScope scope, Map<String,String> opts,
- IteratorEnvironment env) {
- TableId expectedTableId = TableId.of(opts.get("expected.table.id"));
+ private static void testEnv(IteratorScope scope, Map<String,String> opts, IteratorEnvironment env,
+ boolean checkCompactionShouldThrow) {
+ String expTableIdString = opts.get(EXPECTED_TABLE_ID_OPT);
+ TableId expectedTableId = expTableIdString == null ? null : TableId.of(expTableIdString);
+
+ if (checkCompactionShouldThrow) {
Review Comment:
Could use the scope instead of adding the new boolean.
```suggestion
if (scope != majc) {
```
##########
core/src/main/java/org/apache/accumulo/core/iteratorsImpl/ClientIteratorEnvironment.java:
##########
@@ -221,7 +221,7 @@ public PluginEnvironment getPluginEnv() {
@Override
public TableId getTableId() {
- return this.tableId.orElseThrow();
+ return this.tableId.orElse(null);
Review Comment:
Should probably change javadoc for this method in IteratorEnvironment to document this method could return null in cases where there is no table associated with the iterator.
--
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