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 2022/11/05 21:43:40 UTC

[GitHub] [accumulo-testing] keith-turner commented on a diff in pull request #254: Refactor logging in Verify for RW Bulk

keith-turner commented on code in PR #254:
URL: https://github.com/apache/accumulo-testing/pull/254#discussion_r1014730163


##########
src/main/java/org/apache/accumulo/testing/randomwalk/bulk/Verify.java:
##########
@@ -87,26 +90,36 @@ public void visit(State state, RandWalkEnv env, Properties props) throws Excepti
         while (row.hasNext()) {
           Entry<Key,Value> entry = row.next();
 
-          if (rowText == null)
+          if (rowText == null) {
             rowText = entry.getKey().getRow();
+          }
 
           long curr = Long.parseLong(entry.getKey().getColumnQualifier().toString());
 
-          if (curr - 1 != prev)
-            throw new Exception(
-                "Bad marker count " + entry.getKey() + " " + entry.getValue() + " " + prev);
+          if (curr - 1 != prev) {
+            log.error("Bad market count. Current row: {} {}, Previous row marker: {}",
+                entry.getKey(), entry.getValue(), prev);
+            break;
+          }
 
-          if (!entry.getValue().toString().equals("1"))
-            throw new Exception("Bad marker value " + entry.getKey() + " " + entry.getValue());
+          if (!entry.getValue().toString().equals("1")) {
+            log.error("Bad marker value for row {} {}.\n Value expected to be one", entry.getKey(),
+                entry.getValue());
+            break;

Review Comment:
   ```suggestion
               errorFound=true;
               break;
   ```



##########
src/main/java/org/apache/accumulo/testing/randomwalk/bulk/Verify.java:
##########
@@ -87,26 +90,36 @@ public void visit(State state, RandWalkEnv env, Properties props) throws Excepti
         while (row.hasNext()) {
           Entry<Key,Value> entry = row.next();
 
-          if (rowText == null)
+          if (rowText == null) {
             rowText = entry.getKey().getRow();
+          }
 
           long curr = Long.parseLong(entry.getKey().getColumnQualifier().toString());
 
-          if (curr - 1 != prev)
-            throw new Exception(
-                "Bad marker count " + entry.getKey() + " " + entry.getValue() + " " + prev);
+          if (curr - 1 != prev) {
+            log.error("Bad market count. Current row: {} {}, Previous row marker: {}",
+                entry.getKey(), entry.getValue(), prev);
+            break;

Review Comment:
   Will breaking stop it from printing other problems in the test data?  
   
   Should this set errorFound?
   
   ```suggestion
               errorFound=true;
               break;
   ```



##########
src/main/java/org/apache/accumulo/testing/randomwalk/bulk/Verify.java:
##########
@@ -87,26 +90,36 @@ public void visit(State state, RandWalkEnv env, Properties props) throws Excepti
         while (row.hasNext()) {
           Entry<Key,Value> entry = row.next();
 
-          if (rowText == null)
+          if (rowText == null) {
             rowText = entry.getKey().getRow();
+          }
 
           long curr = Long.parseLong(entry.getKey().getColumnQualifier().toString());
 
-          if (curr - 1 != prev)
-            throw new Exception(
-                "Bad marker count " + entry.getKey() + " " + entry.getValue() + " " + prev);
+          if (curr - 1 != prev) {
+            log.error("Bad market count. Current row: {} {}, Previous row marker: {}",
+                entry.getKey(), entry.getValue(), prev);
+            break;
+          }
 
-          if (!entry.getValue().toString().equals("1"))
-            throw new Exception("Bad marker value " + entry.getKey() + " " + entry.getValue());
+          if (!entry.getValue().toString().equals("1")) {
+            log.error("Bad marker value for row {} {}.\n Value expected to be one", entry.getKey(),
+                entry.getValue());
+            break;
+          }
 
           prev = curr;
         }
 
         if (BulkPlusOne.counter.get() != prev) {
-          throw new Exception("Row " + rowText + " does not have all markers "
-              + BulkPlusOne.counter.get() + " " + prev);
+          log.error("Row {} does not have all markers. Current marker: {}, Previous marker:{}",
+              rowText, BulkPlusOne.counter.get(), prev);
+          break;

Review Comment:
   ```suggestion
             errorFound=true;
             break;
   ```



-- 
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