You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by GitBox <gi...@apache.org> on 2022/08/22 04:00:10 UTC

[GitHub] [incubator-pegasus] ninsmiracle opened a new pull request, #1104: refactor(java-client): add more detail message for error log

ninsmiracle opened a new pull request, #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104

   #1105 
   add more detail log in java-client.
   ### What problem does this PR solve? 
   Add value length to PegasusTable's request log in java-client.
   
   ##### Tests 
   - Unit test.Test case has been written in Client/TestBasic.java
   
   ##### Code changes
   The struct of request and some related interfaces function have been changed. 
   
   
   


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] hycdong commented on a diff in pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
hycdong commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r951050454


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -1672,14 +1731,18 @@ public int batchDel2(List<Pair<byte[], byte[]>> keys, List<PException> results,
   @Override
   public void multiDel(byte[] hashKey, List<byte[]> sortKeys, int timeout) throws PException {
     if (timeout <= 0) timeout = defaultTimeout;
-    int count = sortKeys == null ? 0 : sortKeys.size();
+    int count = sortKeys == null ? -1 : sortKeys.size();

Review Comment:
   Why would you update count from 0 into -1 when sortKeys is null?



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r942263060


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -51,6 +51,8 @@ public class PegasusTable implements PegasusTableInterface {
   private WriteLimiter writeLimiter;
   private String metaList;
 
+  public static final byte[] defaultByte = "".getBytes();

Review Comment:
   rename `EMPTY_BYTES`, const variable is suggested named uppercase letter



##########
java-client/src/test/java/org/apache/pegasus/client/TestBasic.java:
##########
@@ -2780,4 +2780,103 @@ private void assertScanResult(
           new String(actuallyRes.results.get(i - startIndex).getRight()));
     }
   }
+
+  @Test // To create a timeout condition,need to change configuration/pegasus.properties timeout
+  // parameter to 1 in this case

Review Comment:
   I don't see the change of this pr, so your unit test shouldn't pass?
   
   However, you shouldn't change the config, which may be used for other unit test and cause timeout. if you want create `client` with specific parameters, you should use `createClient` api.



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1104: refactor(java): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r939777082


##########
java-client/idl/recompile_thrift.sh:
##########
@@ -19,7 +19,7 @@
 # under the License.
 #
 
-function GenThriftTool() {
+function GenThriftTool(){

Review Comment:
   revert



##########
java-client/src/test/java/org/apache/pegasus/client/TestBasic.java:
##########
@@ -2780,4 +2783,78 @@ private void assertScanResult(
           new String(actuallyRes.results.get(i - startIndex).getRight()));
     }
   }
+  @Test   //To create a timeout condition,need to change configuration/pegasus.properties timeout parameter to 1 in this case
+  public void testRequestDetail() throws PException {
+    PegasusClientInterface client = PegasusClientFactory.getSingletonClient();
+    String tableName = "temp";
+    PegasusTableInterface tb = client.openTable(tableName);
+
+    String HashPrefix = "TestHash";
+    String SortPrefix = "TestSort";
+    String hashKey = HashPrefix + "_0";
+    String sortKey = SortPrefix + "_0";
+
+    try {
+      // multiSet timeout
+      System.out.println("Test multiSet PException request");
+
+      String multiValue2 = RandomStringUtils.random(5, true, true);
+      List<Pair<byte[], byte[]>> multiValues2 = new ArrayList<Pair<byte[], byte[]>>();
+      int count2 = 500;
+      while (count2-- > 0) {
+        multiValues2.add(Pair.of(sortKey.getBytes(), multiValue2.getBytes()));
+      }
+
+      Throwable exception = Assertions.assertThrows(PException.class,()->{
+        client.multiSet(tableName, hashKey.getBytes(), multiValues2);
+      });
+      System.out.println(exception.getMessage());
+
+      //checkAndMutate timeout
+      System.out.println("Test checkAndMutate PException request");
+      Mutations mutations = new Mutations();
+      mutations.set(sortKey.getBytes(), "2".getBytes());
+
+      CheckAndMutateOptions options = new CheckAndMutateOptions();
+      options.returnCheckValue = true;
+      Throwable exception2 =  Assertions.assertThrows(PException.class,()->{
+        client.checkAndMutate(
+                tableName,
+                hashKey.getBytes(),
+                "k5".getBytes(),
+                CheckType.CT_VALUE_INT_LESS,
+                "2".getBytes(),
+                mutations,
+                options);
+      });
+      System.out.println(exception2.getMessage());
+
+      //multiDel timeout
+      System.out.println("Test multiDel PException request");
+      List<Pair<byte[], byte[]>> multiValues3 = new ArrayList<Pair<byte[], byte[]>>();
+      List<byte[]> sortKeys = new ArrayList<byte[]>();
+      multiValues3.add(Pair.of("basic_test_sort_key_0".getBytes(), "basic_test_value_0".getBytes()));
+      multiValues3.add(Pair.of("basic_test_sort_key_1".getBytes(), "basic_test_value_1".getBytes()));
+      multiValues3.add(Pair.of("basic_test_sort_key_2".getBytes(), "basic_test_value_2".getBytes()));
+      sortKeys.add("basic_test_sort_key_0".getBytes());
+      sortKeys.add("basic_test_sort_key_1".getBytes());
+      sortKeys.add("basic_test_sort_key_2".getBytes());
+
+      tb.multiSet(hashKey.getBytes(),multiValues3,5000);
+      Assertions.assertDoesNotThrow(()->{
+        tb.multiSet(hashKey.getBytes(),multiValues3,5000);
+      });
+
+      Throwable exception3 =  Assertions.assertThrows(PException.class,()->{
+        client.multiDel(tableName,hashKey.getBytes(),sortKeys);
+      });
+      System.out.println(exception3);

Review Comment:
   you should test the log content, so just assert and print is not enough



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer closed pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer closed pull request #1104: refactor(java-client): add more detail message for error log
URL: https://github.com/apache/incubator-pegasus/pull/1104


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r944066327


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -51,6 +51,12 @@ public class PegasusTable implements PegasusTableInterface {
   private WriteLimiter writeLimiter;
   private String metaList;
 
+  public static final byte[] EMPTY_BYTES = "".getBytes();
+
+  private int getLength(byte[] value){

Review Comment:
   put into `static class Request` 



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r944067364


##########
java-client/src/test/java/org/apache/pegasus/client/TestBasic.java:
##########
@@ -2780,4 +2782,108 @@ private void assertScanResult(
           new String(actuallyRes.results.get(i - startIndex).getRight()));
     }
   }
+
+  @Test
+  public void testRequestDetail() throws PException {
+    Duration caseTimeout = Duration.ofMillis(1);
+    ClientOptions client_opt = ClientOptions.builder()
+            .operationTimeout(caseTimeout)
+            .build();
+
+    PegasusClientFactory.createClient(client_opt);
+    PegasusClientInterface client = PegasusClientFactory.createClient(client_opt);
+    String tableName = "temp";
+    PegasusTableInterface tb = client.openTable(tableName);
+
+    String HashPrefix = "TestHash";
+    String SortPrefix = "TestSort";
+    String hashKey = HashPrefix + "_0";
+    String sortKey = SortPrefix + "_0";
+
+    try {
+      // multiSet timeout
+      System.out.println("Test multiSet PException request");
+
+      String multiValue2 = RandomStringUtils.random(5, true, true);
+      List<Pair<byte[], byte[]>> multiValues2 = new ArrayList<Pair<byte[], byte[]>>();
+      int count2 = 500;
+      while (count2-- > 0) {
+        multiValues2.add(Pair.of(sortKey.getBytes(), multiValue2.getBytes()));
+      }
+
+      Throwable exception =
+          Assertions.assertThrows(
+              PException.class,
+              () -> {
+                client.multiSet(tableName, hashKey.getBytes(), multiValues2);
+              });
+      Assert.assertTrue(
+          exception
+              .getMessage()
+              .contains(
+                  "request=[hashKey[:32]=\"TestHash_0\",sortKey[:32]=\"\",sortKeyCount=500,valueLength=2500]"));
+
+      // checkAndMutate timeout
+      System.out.println("Test checkAndMutate PException request");
+      Mutations mutations = new Mutations();
+      mutations.set(sortKey.getBytes(), "2".getBytes());
+
+      CheckAndMutateOptions options = new CheckAndMutateOptions();
+      options.returnCheckValue = true;
+      Throwable exception2 =
+          Assertions.assertThrows(
+              PException.class,
+              () -> {
+                client.checkAndMutate(
+                    tableName,
+                    hashKey.getBytes(),
+                    "k5".getBytes(),
+                    CheckType.CT_VALUE_INT_LESS,
+                    "2".getBytes(),
+                    mutations,
+                    options);
+              });
+      Assert.assertTrue(
+          exception2
+              .getMessage()
+              .contains(
+                  "request=[hashKey[:32]=\"TestHash_0\",sortKey[:32]=\"k5\",sortKeyCount=1,valueLength=1]"));
+
+      // multiDel timeout
+      System.out.println("Test multiDel PException request");
+      List<Pair<byte[], byte[]>> multiValues3 = new ArrayList<Pair<byte[], byte[]>>();
+      List<byte[]> sortKeys = new ArrayList<byte[]>();
+      multiValues3.add(
+          Pair.of("basic_test_sort_key_0".getBytes(), "basic_test_value_0".getBytes()));
+      multiValues3.add(
+          Pair.of("basic_test_sort_key_1".getBytes(), "basic_test_value_1".getBytes()));
+      multiValues3.add(
+          Pair.of("basic_test_sort_key_2".getBytes(), "basic_test_value_2".getBytes()));
+      sortKeys.add("basic_test_sort_key_0".getBytes());
+      sortKeys.add("basic_test_sort_key_1".getBytes());
+      sortKeys.add("basic_test_sort_key_2".getBytes());
+
+      tb.multiSet(hashKey.getBytes(), multiValues3, 5000);
+      Assertions.assertDoesNotThrow(
+          () -> {
+            tb.multiSet(hashKey.getBytes(), multiValues3, 5000);
+          });
+
+      Throwable exception3 =
+          Assertions.assertThrows(
+              PException.class,
+              () -> {
+                client.multiDel(tableName, hashKey.getBytes(), sortKeys);
+              });
+      Assert.assertTrue(
+          exception3
+              .getMessage()
+              .contains(
+                  "request=[hashKey[:32]=\"TestHash_0\",sortKey[:32]=\"\",sortKeyCount=3,valueLength=-1]"));
+
+    } catch (Throwable e) {
+      e.printStackTrace();

Review Comment:
   delete



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1104: refactor(java): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r939787887


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -2109,40 +2177,69 @@ private void handleWriteLimiterException(DefaultPromise promise, String message)
   }
 
   static class Request {
-    byte[] hashKey = null;
-    byte[] sortKey = null;
-    int sortKeyCount = 0;
+    byte[] hashKey = "".getBytes();
+    byte[] sortKey = "".getBytes();
+    int sortKeyCount = 1;
+

Review Comment:
   delete space



##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -2109,40 +2177,69 @@ private void handleWriteLimiterException(DefaultPromise promise, String message)
   }
 
   static class Request {
-    byte[] hashKey = null;
-    byte[] sortKey = null;
-    int sortKeyCount = 0;
+    byte[] hashKey = "".getBytes();
+    byte[] sortKey = "".getBytes();
+    int sortKeyCount = 1;
+
+    int valueLength = -1;
 
     Request(byte[] hashKey) {
       this.hashKey = hashKey;
     }
 
     Request(byte[] hashKey, byte[] sortKey) {
       this.hashKey = hashKey;
-      this.sortKey = sortKey;
+      this.sortKey = sortKey == null ? "".getBytes() : sortKey;
     }
 
-    Request(byte[] hashKey, int sortKeyCount) {
+    Request(byte[] hashKey, byte[] sortKey, int sortKeyCount, int valueLength) {
       this.hashKey = hashKey;
+      this.sortKey = sortKey == null ? "".getBytes() : sortKey;
       this.sortKeyCount = sortKeyCount;
+      this.valueLength = valueLength;
     }
 
     private String getSubstring(byte[] key) {
       String keyStr = key == null ? "" : new String(key);
       return keyStr.length() < 32 ? keyStr : keyStr.substring(0, 32);
     }
 
+    public static int getValueLength(List<Pair<byte[], byte[]>> values) {
+      if (values == null) {
+        return 0;
+      }
+      int valuesLength = 0;
+      for (Pair<byte[], byte[]> pair : values) {
+        byte[] multiValue = pair.getRight() == null ? "".getBytes() : pair.getRight();
+        valuesLength += multiValue.length;
+      }
+      return valuesLength;
+    }
+
+    public static int getValueLength(Mutations mutations) {
+      if (mutations == null) {
+        return 0;
+      }
+      int valuesLength = 0;
+      for (mutate mu : mutations.getMutations()) {
+        byte[] MutateValue = mu.value == null ? "".getBytes() : mu.value.data;
+        valuesLength += MutateValue.length;
+      }
+      return valuesLength;
+    }
+
     @Override
     public String toString() {
       if (sortKey != null) {
         return String.format(
-            "[hashKey[:32]=\"%s\",sortKey[:32]=\"%s\"]",
-            getSubstring(hashKey), getSubstring(sortKey));
+            "[hashKey[:32]=\"%s\",sortKey[:32]=\"%s\",sortKeyCount=%d,valueLength=%d]",
+            getSubstring(hashKey), getSubstring(sortKey), sortKeyCount, valueLength);
       }
 
       if (sortKeyCount > 0) {
         return String.format(
-            "[hashKey[:32]=\"%s\",sortKeyCount=%d]", getSubstring(hashKey), sortKeyCount);
+            "[hashKey[:32]=\"%s\",sortKeyCount=%d,valueLength = %d]",
+            getSubstring(hashKey), sortKeyCount);

Review Comment:
   These code won't run for your default(sortKey, sortKeyCount ) value



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] hycdong merged pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
hycdong merged PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104


-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] ninsmiracle commented on a diff in pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
ninsmiracle commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r951071861


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -1672,14 +1731,18 @@ public int batchDel2(List<Pair<byte[], byte[]>> keys, List<PException> results,
   @Override
   public void multiDel(byte[] hashKey, List<byte[]> sortKeys, int timeout) throws PException {
     if (timeout <= 0) timeout = defaultTimeout;
-    int count = sortKeys == null ? 0 : sortKeys.size();
+    int count = sortKeys == null ? -1 : sortKeys.size();

Review Comment:
   There are nothing special meaning,just to keep consistent with other codes and -1 meaning invalid.



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r950988569


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -35,19 +35,7 @@
 import org.apache.commons.lang3.Validate;
 import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.apache.commons.lang3.tuple.Pair;
-import org.apache.pegasus.apps.batch_get_request;
-import org.apache.pegasus.apps.cas_check_type;
-import org.apache.pegasus.apps.check_and_mutate_request;
-import org.apache.pegasus.apps.check_and_set_request;
-import org.apache.pegasus.apps.filter_type;
-import org.apache.pegasus.apps.full_data;
-import org.apache.pegasus.apps.full_key;
-import org.apache.pegasus.apps.incr_request;
-import org.apache.pegasus.apps.key_value;
-import org.apache.pegasus.apps.multi_get_request;
-import org.apache.pegasus.apps.multi_put_request;
-import org.apache.pegasus.apps.multi_remove_request;
-import org.apache.pegasus.apps.update_request;
+import org.apache.pegasus.apps.*;

Review Comment:
   revert



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1104: refactor(java): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r940828338


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -260,7 +261,11 @@ public void onCompletion(client_operator clientOP) {
             rrdb_multi_get_operator gop = (rrdb_multi_get_operator) clientOP;
             if (gop.rpc_error.errno != error_code.error_types.ERR_OK) {
               handleReplicaException(
-                  new Request(hashKey, sortKeyBlobs.size()), promise, op, table, timeout);
+                  new Request(hashKey, "".getBytes(), sortKeyBlobs.size(), -1),

Review Comment:
   `"".getBytes()` is used frequently, you can define it as const static variable



##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -740,7 +758,12 @@ public Future<CheckAndSetResult> asyncCheckAndSet(
           public void onCompletion(client_operator clientOP) {
             rrdb_check_and_set_operator op2 = (rrdb_check_and_set_operator) clientOP;
             if (op2.rpc_error.errno != error_code.error_types.ERR_OK) {
-              handleReplicaException(new Request(hashKey, setSortKey), promise, op, table, timeout);
+              handleReplicaException(
+                  new Request(hashKey, setSortKey, 1, setValue == null ? 0 : setValue.length),

Review Comment:
   `setValue == null ? 0 : setValue.length` is also refactor as one method



##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -2109,43 +2177,68 @@ private void handleWriteLimiterException(DefaultPromise promise, String message)
   }
 
   static class Request {
-    byte[] hashKey = null;
-    byte[] sortKey = null;
-    int sortKeyCount = 0;
+    byte[] hashKey = "".getBytes();
+    byte[] sortKey = "".getBytes();
+    int sortKeyCount = 1;
+
+    int valueLength = -1;
 
     Request(byte[] hashKey) {
       this.hashKey = hashKey;
     }
 
     Request(byte[] hashKey, byte[] sortKey) {
       this.hashKey = hashKey;
-      this.sortKey = sortKey;
+      this.sortKey = sortKey == null ? "".getBytes() : sortKey;
     }
 
-    Request(byte[] hashKey, int sortKeyCount) {
+    Request(byte[] hashKey, byte[] sortKey, int sortKeyCount, int valueLength) {
       this.hashKey = hashKey;
+      this.sortKey = sortKey == null ? "".getBytes() : sortKey;
       this.sortKeyCount = sortKeyCount;
+      this.valueLength = valueLength;
     }
 
     private String getSubstring(byte[] key) {
       String keyStr = key == null ? "" : new String(key);
       return keyStr.length() < 32 ? keyStr : keyStr.substring(0, 32);
     }
 
+    public static int getValueLength(List<Pair<byte[], byte[]>> values) {
+      if (values == null) {
+        return 0;
+      }
+      int valuesLength = 0;
+      for (Pair<byte[], byte[]> pair : values) {
+        byte[] multiValue = pair.getRight() == null ? "".getBytes() : pair.getRight();
+        valuesLength += multiValue.length;
+      }
+      return valuesLength;
+    }
+
+    public static int getValueLength(Mutations mutations) {
+      if (mutations == null) {
+        return 0;
+      }
+      int valuesLength = 0;
+      for (mutate mu : mutations.getMutations()) {
+        byte[] MutateValue = mu.value == null ? "".getBytes() : mu.value.data;
+        valuesLength += MutateValue.length;
+      }
+      return valuesLength;
+    }
+
     @Override
     public String toString() {
       if (sortKey != null) {
         return String.format(
-            "[hashKey[:32]=\"%s\",sortKey[:32]=\"%s\"]",
-            getSubstring(hashKey), getSubstring(sortKey));
-      }
-
-      if (sortKeyCount > 0) {
-        return String.format(
-            "[hashKey[:32]=\"%s\",sortKeyCount=%d]", getSubstring(hashKey), sortKeyCount);
+            "[hashKey[:32]=\"%s\",sortKey[:32]=\"%s\",sortKeyCount=%d,valueLength=%d]",

Review Comment:
   `sortKey` has been assign `no-null`, the code is invalid



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org


[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1104: refactor(java-client): add more detail message for error log

Posted by GitBox <gi...@apache.org>.
foreverneverer commented on code in PR #1104:
URL: https://github.com/apache/incubator-pegasus/pull/1104#discussion_r944152490


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -2109,43 +2180,68 @@ private void handleWriteLimiterException(DefaultPromise promise, String message)
   }
 
   static class Request {
-    byte[] hashKey = null;
-    byte[] sortKey = null;
-    int sortKeyCount = 0;
+    public static final byte[] EMPTY_BYTES = "".getBytes();
+
+    public static int getLength(byte[] value) {

Review Comment:
   put all the function in same block



##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -2109,43 +2180,68 @@ private void handleWriteLimiterException(DefaultPromise promise, String message)
   }
 
   static class Request {
-    byte[] hashKey = null;
-    byte[] sortKey = null;
-    int sortKeyCount = 0;
+    public static final byte[] EMPTY_BYTES = "".getBytes();
+
+    public static int getLength(byte[] value) {

Review Comment:
   rename `getValueLength` so that is consist with other function, or you rename all of them `getLength`



##########
java-client/src/main/java/org/apache/pegasus/client/PegasusTable.java:
##########
@@ -2109,43 +2180,68 @@ private void handleWriteLimiterException(DefaultPromise promise, String message)
   }
 
   static class Request {
-    byte[] hashKey = null;
-    byte[] sortKey = null;
-    int sortKeyCount = 0;
+    public static final byte[] EMPTY_BYTES = "".getBytes();
+
+    public static int getLength(byte[] value) {
+      return value == null ? 0 : value.length;
+    }
+
+    byte[] hashKey = EMPTY_BYTES;
+    byte[] sortKey = EMPTY_BYTES;
+    int sortKeyCount = 1;
+

Review Comment:
   delete space



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org