[GitHub] [hbase] virajjasani commented on a diff in pull request #4937: HBASE-27536: improve slowlog payload

[GitBox <gi...@apache.org>]

virajjasani commented on code in PR #4937:
URL: https://github.com/apache/hbase/pull/4937#discussion_r1059141123


##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -2184,6 +2193,19 @@ public static SlowLogParams getSlowLogParams(Message message) {
     return new SlowLogParams(params);
   }
 
+  private static String toJson(boolean slowLogOperationJsonEnabled, int maxCols,
+    Callable<Operation> operation) {
+    if (!slowLogOperationJsonEnabled) {
+      return StringUtils.EMPTY;
+    }
+    try {
+      return operation.call().toJSON(maxCols);
+    } catch (Exception e) {
+      LOG.warn("Exception when deriving operation JSON", e);

Review Comment:
   Wondering if logging `maxCols` might be helpful?



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -2142,38 +2148,41 @@ private static String getStringForByteString(ByteString bs) {
    * @param message Message object {@link Message}
    * @return SlowLogParams with regionName(for filter queries) and params
    */
-  public static SlowLogParams getSlowLogParams(Message message) {
+  public static SlowLogParams getSlowLogParams(Message message, boolean slowLogOperationJsonEnabled,
+    int maxCols) {

Review Comment:
   nit: add additional params to javadoc?



##########
hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java:
##########
@@ -1564,6 +1564,14 @@ public enum OperationStatusCode {
   // Default 10 mins.
   public static final int DEFAULT_SLOW_LOG_SYS_TABLE_CHORE_DURATION = 10 * 60 * 1000;
 
+  public static final String SLOW_LOG_OPERATION_JSON_MAX_COLS =
+    "hbase.regionserver.slowlog.operation.json.max.cols";
+  public static final int SLOW_LOG_OPERATION_JSON_MAX_COLS_DEFAULT = 50;
+
+  public static final String SLOW_LOG_OPERATION_JSON_ENABLED =
+    "hbase.regionserver.slowlog.operation.json.enabled";

Review Comment:
   Would be good to remove `regionserver` from the config names, since the slowlog is used for master RPC calls as well.



##########
hbase-client/src/main/java/org/apache/hadoop/hbase/shaded/protobuf/ProtobufUtil.java:
##########
@@ -2184,6 +2193,19 @@ public static SlowLogParams getSlowLogParams(Message message) {
     return new SlowLogParams(params);
   }
 
+  private static String toJson(boolean slowLogOperationJsonEnabled, int maxCols,

Review Comment:
   nit: replace `slowLogOperationJsonEnabled` with `jsonEnabled` as generic?



-- 
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: issues-unsubscribe@hbase.apache.org

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