Functionality to run ScanConsistencyIT standalone #4051

kevinrr888 · 2023-12-11T16:47:46Z

Closes #3646.
Refactored ScanConsistencyIT to be able to be run standalone. Can now be run as a test or from main.

Closes apache#3646. Refactored ScanConsistencyIT to be able to be run standalone. Can now be run as a test or from main.

keith-turner · 2023-12-11T17:01:46Z

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

+    checkEquals(stats3.scanned, stats1.scanned);
+    checkEquals(stats3.verified, stats1.verified);
+
+    executor.shutdownNow();


Would be good to shutdown this executor in a finally block still.

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

kevinrr888 · 2023-12-11T18:02:19Z

Added requested changes:

Removed "@ throws Exception" from javadoc. Hopefully fixes build failure from "Checkstyle violations".
Shutdown the executor in a finally block
Table is now deleted when test is done
Check args length using Preconditions.checkArgument()
Moved client into try-with-resources block
Messages are now logged in debug mode if in testing context, info mode otherwise

EdColeman · 2023-12-11T18:23:29Z

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

+   *
+   * @param msg message to be logged
+   */
+  private static void logMessage(String msg) {


Why is this necessary? Logging at a relevant level should apply to either context, and con be controlled through normal logging settings to show or suppress levels as desired.

Assumed (based on previous comments) that the message should be logged in debug mode if we are in the testing context, and info mode otherwise. Should this always be log.info?

My reading is that is one possibility. Depending on the frequency of some of the messages, maybe some are debug and others are at info?

Then using log4j configuration can be tuned for the circumstance.

Changing everything to info should be fine. The test are already configured to log debug output here, so changing to info would not increase the amount output by the test when running it from maven.

keith-turner · 2023-12-11T18:26:11Z

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

+      checkEquals(stats3.verified, stats1.verified);
+    } finally {
+      executor.shutdownNow();
+      client.tableOperations().delete(table);


Placing code in a finally block that may throw an exception is tricky. I would not expected shutting down the executor throw an exception, but deleting the table might. If code in the try block throws an exception and code in the finally block throws an exception, then the exception from the try block is lost. Not sure of a good way to improve this though other than something like the following.

try{ } catch(Exception e) { // log here in case the finally block also throws an exception log.error(e.getMessage(),e); throw e; } finally { executor.shutdownNow(); client.tableOperations().delete(table); }

Ah I see. Could I also just move the deletion of the table into the above try block?

try { ... client.tableOperations().delete(table); } finally { executor.shutdownNow(); }

yeah putting the delete inside the try seems fine. If the test fails it will not delete the table which may be useful for debugging anyway.

dlmarion · 2023-12-11T18:37:40Z

The same can be achieved using the instructions here. Just curious, why would we need this?

keith-turner · 2023-12-11T18:49:26Z

The same can be achieved using the instructions here. Just curious, why would we need this?

We may not, I had forgotten about that, wonder if the following work

Setup accumulo in uno
copy test jar to uno lib dir, this is needed because the test has server side code it runs
restart accumulo
run test using mvn passing in uno accumulo props file

If it does work may still want some of the changes in this PR like deleting the table.

keith-turner · 2023-12-11T18:59:46Z

When running this test manually against an existing instance, would like to be able to adjust this sleep. Would like to be able to run this test against an uno instance for hours.

dlmarion · 2023-12-11T19:00:06Z

See this for running an IT against an existing instance. Caveat: I've never tried it.

kevinrr888 · 2023-12-11T19:48:05Z

More requested changes:

Table deletion is no longer in finally block
Use log.info
Main now accepts sleep time

Since we should be taking in a sleep time parameter, not sure how this would be done using mvn.

Let me know if the approach using main is sufficient.

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

keith-turner · 2023-12-11T22:20:00Z

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

+     * @formatter:off
+     * Note: In order to run main,
+     * 1) Build the project
+     * 2) Copy the accumulo test jar (in /test/target/) to your accumulo installation's


I think we could further minimize changes to the test by requiring copying any needed dependencies like junit. The following may be one way to do this.

mvn dependency:copy-dependencies -DincludeGroupIds="org.junit.jupiter" cp test/target/dependency/junit-jupiter-* $ACCUMULO_HOME/lib/

If that works, then would not need the wrappers around the junit assert method call, which would minimizes the test changes.

ctubbsii · 2023-12-19T08:45:14Z

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

-      justification = "predictable random is ok for testing")
+  public static void main(String[] args) {
+    /**
+     * @formatter:off


This is a javadoc. It doesn't make sense to turn off formatting for javadocs... because the end result is still rendered HTML, not formatted plain text. Instead, use html tags, like , <ul>, etc. If you do it right, the javadoc will be formatted reasonably so that it's readable.

Also, if you want < and > to render properly, like you have near the bottom of this javadoc, you're going to need to express it as < or >.

Thanks, I updated the comment

ctubbsii · 2023-12-19T08:47:29Z

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

+    } catch (Exception e) {
+      log.error(e.toString());
+    }


No need to catch and log. Just let it get thrown through the main method. The stack trace there will be more useful than the truncated exception that goes to the logger.

Thanks, changed so main just throws an error now

ctubbsii · 2023-12-19T22:18:05Z

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java

+   * Note: In order to run main,<br>
+   * 1) Build the project<br>
+   * 2) Copy the accumulo test jar (in /test/target/) into your accumulo installation's lib
+   * directory*<br>
+   * 3) Copy the JUnit dependencies into your accumulo installation's lib directory:<br>
+   * $ mvn dependency:copy-dependencies -DincludeGroupIds="org.junit.jupiter"<br>
+   * $ cp test/target/dependency/junit-jupiter-* $ACCUMULO_HOME/lib/<br>
+   * <br>
+   * *Ensure the test jar is in lib before the tablet servers start. Restart tablet servers if
+   * necessary.<br>
+   * <br>
+   * Now, this can be run with<br>
+   * $ accumulo org.apache.accumulo.test.ScanConsistencyIT [props-file] [tmp-dir] [table]
+   * [sleep-time]<br>
+   * <br>
+   *
+   * [props-file]: An accumulo client properties file<br>
+   * [tmp-dir]: tmpDir field for the TestContext object<br>
+   * [table]: The name of the table to be created<br>
+   * [sleep-time]: The time to sleep (ms) after submitting the various concurrent tasks<br>
+   * <br>


I don't know if this works, but I was thinking something more along this:

Suggested change

* Note: In order to run main, 

* 1) Build the project 

* 2) Copy the accumulo test jar (in /test/target/) into your accumulo installation's lib

* directory* 

* 3) Copy the JUnit dependencies into your accumulo installation's lib directory: 

* $ mvn dependency:copy-dependencies -DincludeGroupIds="org.junit.jupiter" 

* $ cp test/target/dependency/junit-jupiter-* $ACCUMULO_HOME/lib/ 

* 

* *Ensure the test jar is in lib before the tablet servers start. Restart tablet servers if

* necessary. 

* 

* Now, this can be run with 

* $ accumulo org.apache.accumulo.test.ScanConsistencyIT [props-file] [tmp-dir] [table]

* [sleep-time] 

* 

*

* [props-file]: An accumulo client properties file 

* [tmp-dir]: tmpDir field for the TestContext object 

* [table]: The name of the table to be created 

* [sleep-time]: The time to sleep (ms) after submitting the various concurrent tasks 

* 

* Note: In order to run main,

* <ol>

* <li>Build the project

* <li>Copy the accumulo test jar (in /test/target/) into your accumulo installation's lib directory

* <li>Copy the JUnit dependencies into your accumulo installation's lib directory:

* <ul>

* <li>mvn dependency:copy-dependencies -DincludeGroupIds="org.junit.jupiter"

* <li>cp test/target/dependency/junit-jupiter-* $ACCUMULO_HOME/lib/

* </ul>

* <li>Ensure the test jar is in lib before the tablet servers start. Restart tablet servers if

* necessary.

* <li>Run with: bin/accumulo org.apache.accumulo.test.ScanConsistencyIT [props-file] [tmp-dir] [table]

* [sleep-time]

* </ol>

*

* [props-file]: An accumulo client properties file 

* [tmp-dir]: tmpDir field for the TestContext object 

* [table]: The name of the table to be created 

* [sleep-time]: The time to sleep (ms) after submitting the various concurrent tasks

Reformatted similarly. From running the QA checks, maven didn't like the unordered list nested within the ordered list which is why I have the third list element the way it is.

dlmarion

I have no issue with the code changes by themselves, but I wonder if this is the best place for them. IMO, this functionality will not be remembered when it comes time to perform release testing as I think the majority of testing occurs from the accumulo-testing repo. If this is merged, then I would suggest that the release testing section of the website be updated to reference this test and any other standalone-capable ITs.

dlmarion · 2026-01-28T18:57:33Z

I did a quick look in the test module, and it looks like the following classes may work in a standalone capacity:

TestIngest
TestMultiTableIngest
VerifyIngest
TestRandomDeletes

These classes aren't tests, but utilities that ITs use.

Functionality to run ScanConsistencyIT standalone

e94a3cc

Closes apache#3646. Refactored ScanConsistencyIT to be able to be run standalone. Can now be run as a test or from main.

keith-turner reviewed Dec 11, 2023

View reviewed changes

DomGarguilo reviewed Dec 11, 2023

View reviewed changes

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java Outdated Show resolved Hide resolved

DomGarguilo reviewed Dec 11, 2023

View reviewed changes

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java Outdated Show resolved Hide resolved

DomGarguilo requested changes Dec 11, 2023

View reviewed changes

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java Outdated Show resolved Hide resolved

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java Outdated Show resolved Hide resolved

Requested changes

68403d5

More requested changes

471c4af

EdColeman reviewed Dec 11, 2023

View reviewed changes

keith-turner reviewed Dec 11, 2023

View reviewed changes

fixed table deletion, use log.info, accept sleep time

bf6b87c

EdColeman reviewed Dec 11, 2023

View reviewed changes

test/src/main/java/org/apache/accumulo/test/ScanConsistencyIT.java Show resolved Hide resolved

keith-turner reviewed Dec 11, 2023

View reviewed changes

kevinrr888 added 4 commits December 12, 2023 09:14

Removed assertion wrapper, added note about JUnit deps

e245a45

scan() no longer throws Exception

14c2b4a

Moved SuppressFBWarnings to correct method

fe2f217

Formatting fix

b398bee

DomGarguilo approved these changes Dec 12, 2023

View reviewed changes

ctubbsii requested changes Dec 19, 2023

View reviewed changes

kevinrr888 added 3 commits December 19, 2023 09:01

Merge branch 'main' into feature-3646

d2df49f

Updated javadoc for main, main now throws exception

4fe6fda

Added code tags to javadocs

6e04d16

DomGarguilo approved these changes Dec 19, 2023

View reviewed changes

ctubbsii reviewed Dec 19, 2023

View reviewed changes

Reformatted javadoc for main

3ea4e83

EdColeman assigned kevinrr888 Mar 18, 2024

ctubbsii added this to the 3.1.0 milestone Jul 12, 2024

dlmarion changed the base branch from main to 3.1 August 26, 2024 12:04

ctubbsii changed the base branch from 3.1 to main March 13, 2025 22:42

ctubbsii modified the milestones: 3.1.0, 4.0.0 Mar 13, 2025

dlmarion approved these changes Jan 28, 2026

View reviewed changes

Functionality to run ScanConsistencyIT standalone #4051

Are you sure you want to change the base?

Functionality to run ScanConsistencyIT standalone #4051

Conversation

kevinrr888 commented Dec 11, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kevinrr888 commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kevinrr888 Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlmarion commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keith-turner commented Dec 11, 2023

Uh oh!

keith-turner commented Dec 11, 2023

Uh oh!

dlmarion commented Dec 11, 2023

Uh oh!

kevinrr888 commented Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

keith-turner Dec 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlmarion left a comment

Choose a reason for hiding this comment

Uh oh!

dlmarion commented Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

kevinrr888 commented Dec 11, 2023 •

edited

Loading

kevinrr888 Dec 11, 2023 •

edited

Loading

dlmarion commented Dec 11, 2023 •

edited

Loading

kevinrr888 commented Dec 11, 2023 •

edited

Loading

keith-turner Dec 11, 2023 •

edited

Loading