[accumulo] branch 1.10 updated: Fix #1792 flaky/broken GcMetricsIT

2020-12-16 Thread ctubbsii
This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a commit to branch 1.10
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/1.10 by this push:
 new 0773baa  Fix #1792 flaky/broken GcMetricsIT
0773baa is described below

commit 0773baac791ab2055d949bd62948d2ba39cca1a3
Author: Christopher Tubbs 
AuthorDate: Wed Dec 16 22:29:17 2020 -0500

Fix #1792 flaky/broken GcMetricsIT

Rewrite portions of GcMetricsIT to improve its reliability and to add
comments and rename methods to make it more clear how it is intended to
work, so it's easier to debug in future.

Also use daemon threads for the tailer, and ensure it is started
consistently (using a dedicated method) and closed properly when done.

Remove unnecessary catching of exceptions, and allow them to be thrown
from the tests instead, because they will be handled by the test
framework.
---
 .../accumulo/test/functional/GcMetricsIT.java  | 313 -
 .../accumulo/test/functional/MasterMetricsIT.java  |  14 +-
 .../accumulo/test/metrics/MetricsFileTailer.java   |  25 +-
 .../resources/hadoop-metrics2-accumulo.properties  |  36 +--
 .../test/metrics/MetricsFileTailerTest.java|  75 +++--
 5 files changed, 185 insertions(+), 278 deletions(-)

diff --git 
a/test/src/main/java/org/apache/accumulo/test/functional/GcMetricsIT.java 
b/test/src/main/java/org/apache/accumulo/test/functional/GcMetricsIT.java
index 51a4136..120ac35 100644
--- a/test/src/main/java/org/apache/accumulo/test/functional/GcMetricsIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/GcMetricsIT.java
@@ -1,22 +1,27 @@
 /*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ *   http://www.apache.org/licenses/LICENSE-2.0
  *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
  */
 package org.apache.accumulo.test.functional;
 
+import static java.util.stream.Collectors.toMap;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assume.assumeFalse;
+import static org.junit.Assume.assumeTrue;
 
 import java.util.Collections;
 import java.util.Map;
@@ -25,13 +30,15 @@ import java.util.TreeMap;
 import java.util.concurrent.TimeUnit;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Stream;
 
 import org.apache.accumulo.core.conf.Property;
 import org.apache.accumulo.gc.metrics.GcMetrics;
-import org.apache.accumulo.harness.AccumuloClusterHarness;
 import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
 import org.apache.accumulo.test.metrics.MetricsFileTailer;
 import org.apache.hadoop.conf.Configuration;
+import org.junit.After;
+import org.junit.Before;
 import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -40,22 +47,22 @@ import org.slf4j.LoggerFactory;
  * Functional test that uses a hadoop metrics 2 file sink to read published 
metrics for
  * verification.
  */
-public class GcMetricsIT extends AccumuloClusterHarness {
+public class GcMetricsIT extends ConfigurableMacBase {
 
   private static final Logger log = LoggerFactory.getLogger(GcMetricsIT.class);
-
   private static final int NUM_TAIL_ATTEMPTS = 20;
   private static final long TAIL_DELAY = 5_000;
-
+  private static final Pattern metricLinePattern = 
Pattern.compile("^(?\\d+).*");
   private static final String[] EXPECTED_METRIC_KEYS = new String[] 
{"AccGcCandidates",
   "AccGcDeleted", "AccGcErrors", 

[accumulo] branch main updated (185fc90 -> 243e577)

2020-12-16 Thread ctubbsii
This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a change to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git.


from 185fc90  Fix ClientPropertiesTest for #1839
 new 0773baa  Fix #1792 flaky/broken GcMetricsIT
 new 243e577  Merge branch '1.10' into main

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../accumulo/test/functional/GcMetricsIT.java  | 268 +++--
 .../accumulo/test/functional/MasterMetricsIT.java  |  11 +-
 .../accumulo/test/metrics/MetricsFileTailer.java   |  27 ++-
 .../resources/hadoop-metrics2-accumulo.properties  |   2 +-
 .../test/metrics/MetricsFileTailerTest.java|  49 ++--
 5 files changed, 133 insertions(+), 224 deletions(-)



[accumulo] 01/01: Merge branch '1.10' into main

2020-12-16 Thread ctubbsii
This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git

commit 243e5771b03281c64115a625da83a20cd586adde
Merge: 185fc90 0773baa
Author: Christopher Tubbs 
AuthorDate: Wed Dec 16 23:23:29 2020 -0500

Merge branch '1.10' into main

 .../accumulo/test/functional/GcMetricsIT.java  | 268 +++--
 .../accumulo/test/functional/MasterMetricsIT.java  |  11 +-
 .../accumulo/test/metrics/MetricsFileTailer.java   |  27 ++-
 .../resources/hadoop-metrics2-accumulo.properties  |   2 +-
 .../test/metrics/MetricsFileTailerTest.java|  49 ++--
 5 files changed, 133 insertions(+), 224 deletions(-)

diff --cc 
test/src/main/java/org/apache/accumulo/test/functional/GcMetricsIT.java
index 94884b4,120ac35..5d76a21
--- a/test/src/main/java/org/apache/accumulo/test/functional/GcMetricsIT.java
+++ b/test/src/main/java/org/apache/accumulo/test/functional/GcMetricsIT.java
@@@ -18,7 -18,10 +18,9 @@@
   */
  package org.apache.accumulo.test.functional;
  
+ import static java.util.stream.Collectors.toMap;
  import static org.junit.Assert.assertTrue;
 -import static org.junit.Assume.assumeFalse;
+ import static org.junit.Assume.assumeTrue;
  
  import java.util.Collections;
  import java.util.Map;
@@@ -27,14 -30,14 +29,14 @@@ import java.util.TreeMap
  import java.util.concurrent.TimeUnit;
  import java.util.regex.Matcher;
  import java.util.regex.Pattern;
+ import java.util.stream.Stream;
  
- import org.apache.accumulo.core.client.Accumulo;
- import org.apache.accumulo.core.client.AccumuloClient;
  import org.apache.accumulo.core.conf.Property;
  import org.apache.accumulo.gc.metrics.GcMetrics;
 -import org.apache.accumulo.minicluster.impl.MiniAccumuloConfigImpl;
 +import org.apache.accumulo.miniclusterImpl.MiniAccumuloConfigImpl;
  import org.apache.accumulo.test.metrics.MetricsFileTailer;
  import org.apache.hadoop.conf.Configuration;
+ import org.junit.After;
  import org.junit.Before;
  import org.junit.Test;
  import org.slf4j.Logger;
@@@ -73,137 -70,38 +69,36 @@@ public class GcMetricsIT extends Config
  return 4 * 60;
}
  
-   @Test
-   public void gcMetricsPublished() throws InterruptedException {
- boolean gcMetricsEnabled =
- 
cluster.getSiteConfiguration().getBoolean(Property.GC_METRICS_ENABLED);
- 
- if (!gcMetricsEnabled) {
-   log.info("gc metrics are disabled with GC_METRICS_ENABLED=true");
-   return;
- }
- 
- log.debug("Client started, properties:{}", accumuloClient.properties());
- 
- MetricsFileTailer gcTail = new MetricsFileTailer("accumulo.sink.file-gc");
- Thread t1 = new Thread(gcTail);
- t1.start();
- 
- // uncomment for manual jmx / jconsole validation - not for automated 
testing
- // Thread.sleep(320_000);
- 
- try {
- 
-   var updateTimestamp = System.currentTimeMillis();
+   private final MetricsFileTailer gcTail = new 
MetricsFileTailer("accumulo.sink.file-gc");
  
-   // Get next update after current time
-   LineUpdate firstUpdate = waitForUpdate(updateTimestamp, gcTail);
- 
-   Map firstSeenMap = parseLine(firstUpdate.getLine());
- 
-   log.debug("L:{}", firstUpdate.getLine());
-   log.debug("M:{}", firstSeenMap);
- 
-   assertTrue(lookForExpectedKeys(firstSeenMap));
-   sanity(updateTimestamp, firstSeenMap);
- 
-   // Get next update after the first one
-   updateTimestamp = firstUpdate.getLastUpdate();
-   LineUpdate nextUpdate = waitForUpdate(updateTimestamp, gcTail);
- 
-   Map updateSeenMap = parseLine(nextUpdate.getLine());
- 
-   log.debug("Line received:{}", nextUpdate.getLine());
-   log.debug("Mapped values:{}", updateSeenMap);
- 
-   assertTrue(lookForExpectedKeys(updateSeenMap));
-   sanity(updateTimestamp, updateSeenMap);
- 
-   validate(firstSeenMap, updateSeenMap);
- 
- } catch (Exception ex) {
-   log.debug("reads", ex);
- }
+   @Before
+   public void startTailer() {
+ gcTail.startDaemonThread();
}
  
-   /**
-* Validate metrics for consistency withing a run cycle.
-*
-* @param values
-*  map of values from one run cycle.
-*/
-   private void sanity(final long testStart, final Map values) {
- 
- long start = values.get("AccGcStarted");
- long finished = values.get("AccGcFinished");
- 
- log.debug("test start: {}, gc start: {}, gc finished: {}", testStart, 
start, finished);
- 
- assertTrue(start >= testStart);
- assertTrue(finished >= start);
- 
- start = values.get("AccGcWalStarted");
- finished = values.get("AccGcWalFinished");
- 
- log.debug("test start: {}, walgc start: {}, walgc finished: {}", 
testStart, start, finished);
- 
- assertTrue(start >= testStart);
- assertTrue(finished >= start);
- 
+   @After
+   public void stopTailer() {
+ gcTail.close();
}
  
-   /**
-* A series of sanity checks for the metrics 

[accumulo] branch main updated: Fix ClientPropertiesTest for #1839

2020-12-16 Thread ctubbsii
This is an automated email from the ASF dual-hosted git repository.

ctubbsii pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/accumulo.git


The following commit(s) were added to refs/heads/main by this push:
 new 185fc90  Fix ClientPropertiesTest for #1839
185fc90 is described below

commit 185fc90dcc8e738f2f77b9668a1f918bc5b8e4b0
Author: Christopher Tubbs 
AuthorDate: Wed Dec 16 14:36:05 2020 -0500

Fix ClientPropertiesTest for #1839

Ensure correct copy behavior when constructing a client from a previous
client's properties.
---
 .../accumulo/core/client/ClientPropertiesTest.java | 27 ++
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git 
a/core/src/test/java/org/apache/accumulo/core/client/ClientPropertiesTest.java 
b/core/src/test/java/org/apache/accumulo/core/client/ClientPropertiesTest.java
index ebaabd0..2b364ed 100644
--- 
a/core/src/test/java/org/apache/accumulo/core/client/ClientPropertiesTest.java
+++ 
b/core/src/test/java/org/apache/accumulo/core/client/ClientPropertiesTest.java
@@ -19,7 +19,7 @@
 package org.apache.accumulo.core.client;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.assertThrows;
 
 import java.nio.file.Paths;
 import java.util.Properties;
@@ -39,20 +39,27 @@ public class ClientPropertiesTest {
 assertEquals("password", ClientProperty.AUTH_TYPE.getValue(props1));
 assertEquals("pass1", ClientProperty.AUTH_TOKEN.getValue(props1));
 
+ClientProperty.validate(props1);
+
 Properties props2 =
 Accumulo.newClientProperties().from(props1).as("user2", 
Paths.get("/path2")).build();
+
+// verify props1 is unchanged
 assertEquals("inst1", ClientProperty.INSTANCE_NAME.getValue(props1));
 assertEquals("zoo1", ClientProperty.INSTANCE_ZOOKEEPERS.getValue(props1));
-assertEquals("user2", ClientProperty.AUTH_PRINCIPAL.getValue(props1));
-assertEquals("kerberos", ClientProperty.AUTH_TYPE.getValue(props1));
-assertEquals("/path2", ClientProperty.AUTH_TOKEN.getValue(props1));
+assertEquals("user1", ClientProperty.AUTH_PRINCIPAL.getValue(props1));
+assertEquals("password", ClientProperty.AUTH_TYPE.getValue(props1));
+assertEquals("pass1", ClientProperty.AUTH_TOKEN.getValue(props1));
+
+// verify props2 has new values for overridden fields
+assertEquals("inst1", ClientProperty.INSTANCE_NAME.getValue(props2));
+assertEquals("zoo1", ClientProperty.INSTANCE_ZOOKEEPERS.getValue(props2));
+assertEquals("user2", ClientProperty.AUTH_PRINCIPAL.getValue(props2));
+assertEquals("kerberos", ClientProperty.AUTH_TYPE.getValue(props2));
+assertEquals("/path2", ClientProperty.AUTH_TOKEN.getValue(props2));
 
 props2.remove(ClientProperty.AUTH_PRINCIPAL.getKey());
-try {
-  ClientProperty.validate(props2);
-  fail();
-} catch (IllegalArgumentException e) {
-  // expected
-}
+var e = assertThrows(IllegalArgumentException.class, () -> 
ClientProperty.validate(props2));
+assertEquals("auth.principal is not set", e.getMessage());
   }
 }