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 2020/03/04 20:19:34 UTC

[GitHub] [accumulo] ctubbsii opened a new pull request #1545: Remove miniDFS from HalfDeadTServerIT

ctubbsii opened a new pull request #1545: Remove miniDFS from HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545
 
 
   Remove use of miniDFS from IT, because sometimes miniDFS is slow to
   start up, and it causes the IT to time out. It is not needed for the
   test, since the test case works fine on LocalFileSystem-based DFS.
   
   Use java.util.scanner to clean up boilerplate BufferedReader stuffs.
   
   Suppress some spammy ZK logs in IT log4j2 config.
   
   Add prefix to DumpOutput class in HalfDeadTServerIT to help troubleshoot
   the IT in future.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo] phrocker commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT

Posted by GitBox <gi...@apache.org>.
phrocker commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545#discussion_r388231472
 
 

 ##########
 File path: test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
 ##########
 @@ -63,7 +66,29 @@ protected int defaultTimeoutSeconds() {
     return 4 * 60;
   }
 
-  class DumpOutput extends Daemon {
+  private static final AtomicBoolean sharedLibBuilt = new AtomicBoolean(false);
+
+  @SuppressFBWarnings(value = "COMMAND_INJECTION",
+      justification = "command executed is not from user input")
+  @BeforeClass
+  public static void buildSharedLib() throws IOException, InterruptedException {
+    String root = System.getProperty("user.dir");
+    String source = root + "/src/test/c/fake_disk_failure.c";
+    String lib = root + "/target/fake_disk_failure.so";
 
 Review comment:
   If the "so" exists, do you even need to rec-compile? I guess you run the risk of it being corrupt or an incorrect so, but a clean should remove this as well.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo] phrocker commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT

Posted by GitBox <gi...@apache.org>.
phrocker commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545#discussion_r388231472
 
 

 ##########
 File path: test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
 ##########
 @@ -63,7 +66,29 @@ protected int defaultTimeoutSeconds() {
     return 4 * 60;
   }
 
-  class DumpOutput extends Daemon {
+  private static final AtomicBoolean sharedLibBuilt = new AtomicBoolean(false);
+
+  @SuppressFBWarnings(value = "COMMAND_INJECTION",
+      justification = "command executed is not from user input")
+  @BeforeClass
+  public static void buildSharedLib() throws IOException, InterruptedException {
+    String root = System.getProperty("user.dir");
+    String source = root + "/src/test/c/fake_disk_failure.c";
+    String lib = root + "/target/fake_disk_failure.so";
 
 Review comment:
   If the so exists, do you even need to rec-compile? I guess you run the risk of it being corrupt or an incorrect so, but a clean should remove this as well.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo] ctubbsii commented on issue #1545: Remove miniDFS from HalfDeadTServerIT

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #1545: Remove miniDFS from HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545#issuecomment-594909537
 
 
   > Given that you spent time trying to figure out this test, may be useful to add a few comments about what the test is attempting to do.
   
   I can do that.
   
   > So is the test passing reliably now?
   
   As far as I can tell, yes. It seems mostly the problem was the occasional slow startup times of miniDFS.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo] ctubbsii commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #1545: Remove miniDFS from HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545#discussion_r388352906
 
 

 ##########
 File path: test/src/main/java/org/apache/accumulo/test/functional/HalfDeadTServerIT.java
 ##########
 @@ -63,7 +66,29 @@ protected int defaultTimeoutSeconds() {
     return 4 * 60;
   }
 
-  class DumpOutput extends Daemon {
+  private static final AtomicBoolean sharedLibBuilt = new AtomicBoolean(false);
+
+  @SuppressFBWarnings(value = "COMMAND_INJECTION",
+      justification = "command executed is not from user input")
+  @BeforeClass
+  public static void buildSharedLib() throws IOException, InterruptedException {
+    String root = System.getProperty("user.dir");
+    String source = root + "/src/test/c/fake_disk_failure.c";
+    String lib = root + "/target/fake_disk_failure.so";
 
 Review comment:
   Perhaps not, but that's a bit out of scope of what I was trying to optimize here. I was actually thinking about creating a proper Makefile and building the shared lib with exec-maven-plugin rather than doing it with a direct call to gcc in the test. But again, I felt it was a bit out of scope.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo] ctubbsii merged pull request #1545: Remove miniDFS from HalfDeadTServerIT

Posted by GitBox <gi...@apache.org>.
ctubbsii merged pull request #1545: Remove miniDFS from HalfDeadTServerIT
URL: https://github.com/apache/accumulo/pull/1545
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services