stoty commented on code in PR #1875:
URL: https://github.com/apache/phoenix/pull/1875#discussion_r1639263406


##########
phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java:
##########
@@ -890,14 +901,17 @@ private static void deletePriorSchemas(long ts, String 
url) throws Exception {
         }
         try (Connection conn = DriverManager.getConnection(url, props)) {
             DatabaseMetaData dbmd = conn.getMetaData();
-            ResultSet rs = dbmd.getSchemas();
+            ResultSet rs = dbmd.getSchemas(null, "%" + 
phoenixTestTableName.getTableName() + "%");

Review Comment:
   Unrelated to your changes, this code is needlessly complex. (And it's my 
fault).
   
   This should should simply drop all HBase namespaces and tables, and let 
Phoenix re-initialize from scratch for the next test.



##########
pom.xml:
##########
@@ -405,6 +405,30 @@
                  <goal>verify</goal>
               </goals>
             </execution>
+            <execution>
+              <id>SignOffTests</id>
+              <configuration>
+                <reuseForks>false</reuseForks>
+                <argLine>@{jacocoArgLine} -enableassertions -Xmx3000m 
-Djava.security.egd=file:/dev/./urandom 
"-Djava.library.path=${hadoop.library.path}${path.separator}${java.library.path}"
 -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=./target/ -XX:NewRatio=4 
-XX:SurvivorRatio=8 -XX:+UseCompressedOops -XX:+UseConcMarkSweepGC 
-XX:+DisableExplicitGC -XX:+UseCMSInitiatingOccupancyOnly 
-XX:+CMSClassUnloadingEnabled -XX:+CMSScavengeBeforeRemark 
-XX:CMSInitiatingOccupancyFraction=68</argLine>
+                
<groups>org.apache.phoenix.end2end.SupportsDistributedClusterTest</groups>
+              </configuration>
+              <goals>
+                <goal>integration-test</goal>
+                <goal>verify</goal>
+              </goals>
+            </execution>
+            <execution>
+              <id>FailedSignOffTests</id>
+              <configuration>
+                <reuseForks>false</reuseForks>
+                <argLine>@{jacocoArgLine} -enableassertions -Xmx3000m 
-Djava.security.egd=file:/dev/./urandom 
"-Djava.library.path=${hadoop.library.path}${path.separator}${java.library.path}"
 -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=./target/ -XX:NewRatio=4 
-XX:SurvivorRatio=8 -XX:+UseCompressedOops -XX:+UseConcMarkSweepGC 
-XX:+DisableExplicitGC -XX:+UseCMSInitiatingOccupancyOnly 
-XX:+CMSClassUnloadingEnabled -XX:+CMSScavengeBeforeRemark 
-XX:CMSInitiatingOccupancyFraction=68</argLine>

Review Comment:
   I have reorganized the options handling some time ago.
   Please use the new system.



##########
phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java:
##########
@@ -454,8 +455,10 @@ protected static String 
checkClusterInitialized(ReadOnlyProps serverProps) throw
     protected static String setUpTestCluster(@Nonnull Configuration conf, 
ReadOnlyProps overrideProps) throws Exception {
         boolean isDistributedCluster = isDistributedClusterModeEnabled(conf);
         if (!isDistributedCluster) {
+            TEARDOWN_THRESHOLD.set(30);
             return initMiniCluster(conf, overrideProps);
         } else {
+            TEARDOWN_THRESHOLD.set(1);

Review Comment:
   I'm also not conviced that needs to be an AtomicInteger.



##########
phoenix-core/src/test/java/org/apache/phoenix/query/BaseTest.java:
##########
@@ -454,8 +455,10 @@ protected static String 
checkClusterInitialized(ReadOnlyProps serverProps) throw
     protected static String setUpTestCluster(@Nonnull Configuration conf, 
ReadOnlyProps overrideProps) throws Exception {
         boolean isDistributedCluster = isDistributedClusterModeEnabled(conf);
         if (!isDistributedCluster) {
+            TEARDOWN_THRESHOLD.set(30);
             return initMiniCluster(conf, overrideProps);
         } else {
+            TEARDOWN_THRESHOLD.set(1);

Review Comment:
   Do we need to clear up after each test ?
   A Real cluster should have enough resources to hold some leftover tables.



##########
pom.xml:
##########
@@ -405,6 +405,30 @@
                  <goal>verify</goal>
               </goals>
             </execution>
+            <execution>
+              <id>SignOffTests</id>
+              <configuration>
+                <reuseForks>false</reuseForks>
+                <argLine>@{jacocoArgLine} -enableassertions -Xmx3000m 
-Djava.security.egd=file:/dev/./urandom 
"-Djava.library.path=${hadoop.library.path}${path.separator}${java.library.path}"
 -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=./target/ -XX:NewRatio=4 
-XX:SurvivorRatio=8 -XX:+UseCompressedOops -XX:+UseConcMarkSweepGC 
-XX:+DisableExplicitGC -XX:+UseCMSInitiatingOccupancyOnly 
-XX:+CMSClassUnloadingEnabled -XX:+CMSScavengeBeforeRemark 
-XX:CMSInitiatingOccupancyFraction=68</argLine>
+                
<groups>org.apache.phoenix.end2end.SupportsDistributedClusterTest</groups>
+              </configuration>
+              <goals>
+                <goal>integration-test</goal>
+                <goal>verify</goal>
+              </goals>
+            </execution>
+            <execution>
+              <id>FailedSignOffTests</id>
+              <configuration>
+                <reuseForks>false</reuseForks>
+                <argLine>@{jacocoArgLine} -enableassertions -Xmx3000m 
-Djava.security.egd=file:/dev/./urandom 
"-Djava.library.path=${hadoop.library.path}${path.separator}${java.library.path}"
 -XX:+HeapDumpOnOutOfMemoryError -XX:HeapDumpPath=./target/ -XX:NewRatio=4 
-XX:SurvivorRatio=8 -XX:+UseCompressedOops -XX:+UseConcMarkSweepGC 
-XX:+DisableExplicitGC -XX:+UseCMSInitiatingOccupancyOnly 
-XX:+CMSClassUnloadingEnabled -XX:+CMSScavengeBeforeRemark 
-XX:CMSInitiatingOccupancyFraction=68</argLine>

Review Comment:
   What is the value of this ?
   
   De we really need a new category for the tests that cannot be run a 
distributed cluster ?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to