You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by jd...@apache.org on 2017/02/11 03:32:59 UTC
kudu git commit: [java client] Limit the message size in Status
Repository: kudu
Updated Branches:
refs/heads/master 41dd9d41a -> a92870939
[java client] Limit the message size in Status
We've seen cases where RPC traces that are getting logged are MBs in size.
Even if we limit the number of traces, the messages themselves can get quite big.
This fix is only for the Java client since it's the only client with tracing at
the moment. Ideally we'd limit this also in the server but it's still desirable
to have the clients protect themselves.
With the limits in place, we can have a maximum of 100 * 256 = 25KB of messages
in a single RPC's trace.
Change-Id: I5868da9ee6b4befd1dc863570b8e251369213ffa
Reviewed-on: http://gerrit.cloudera.org:8080/5956
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a9287093
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a9287093
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a9287093
Branch: refs/heads/master
Commit: a9287093954bf2de64642605c7f9522f281830db
Parents: 41dd9d4
Author: Jean-Daniel Cryans <jd...@apache.org>
Authored: Thu Feb 9 10:38:30 2017 -0800
Committer: Jean-Daniel Cryans <jd...@apache.org>
Committed: Sat Feb 11 03:32:37 2017 +0000
----------------------------------------------------------------------
.../java/org/apache/kudu/client/KuduRpc.java | 4 +
.../java/org/apache/kudu/client/Status.java | 90 +++++++++++---------
.../java/org/apache/kudu/client/TestStatus.java | 31 +++++++
3 files changed, 87 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/kudu/blob/a9287093/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
index 387a6fa..1ba909d 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduRpc.java
@@ -67,6 +67,10 @@ import org.apache.kudu.util.Slice;
@InterfaceAudience.Private
public abstract class KuduRpc<R> {
+ /**
+ * This along with {@link Status#MAX_MESSAGE_LENGTH} dictates how big all the messages
+ * in a trace can be.
+ */
@VisibleForTesting
public static final int MAX_TRACES_SIZE = 100;
http://git-wip-us.apache.org/repos/asf/kudu/blob/a9287093/java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/Status.java b/java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
index 07be8b1..fcd4339 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
@@ -17,6 +17,7 @@
package org.apache.kudu.client;
+import com.google.common.annotations.VisibleForTesting;
import org.apache.kudu.WireProtocol;
import org.apache.kudu.annotations.InterfaceAudience;
import org.apache.kudu.annotations.InterfaceStability;
@@ -31,22 +32,35 @@ import org.apache.kudu.tserver.Tserver;
@InterfaceStability.Evolving
public class Status {
+ // Limit the message size we get from the servers as it can be quite large.
+ @VisibleForTesting
+ static final int MAX_MESSAGE_LENGTH = 256;
+ @VisibleForTesting
+ static final String ABBREVIATION_CHARS = "...";
+ @VisibleForTesting
+ static final int ABBREVIATION_CHARS_LENGTH = ABBREVIATION_CHARS.length();
+
// Keep a single OK status object else we'll end up instantiating tons of them.
private static final Status STATIC_OK = new Status(WireProtocol.AppStatusPB.ErrorCode.OK);
- private final WireProtocol.AppStatusPB appStatusPB;
+ private final WireProtocol.AppStatusPB.ErrorCode code;
+ private final String message;
+ private final int posixCode;
- private Status(WireProtocol.AppStatusPB appStatusPB) {
- this.appStatusPB = appStatusPB;
+ private Status(WireProtocol.AppStatusPB.ErrorCode code, String msg, int posixCode) {
+ this.code = code;
+ this.posixCode = posixCode;
+
+ if (msg.length() > MAX_MESSAGE_LENGTH) {
+ // Truncate the message and indicate that it was abbreviated.
+ this.message = msg.substring(0, MAX_MESSAGE_LENGTH - ABBREVIATION_CHARS_LENGTH)
+ + ABBREVIATION_CHARS;
+ } else
+ this.message = msg;
}
- private Status(WireProtocol.AppStatusPB.ErrorCode code, String msg, int posixCode) {
- this.appStatusPB =
- WireProtocol.AppStatusPB.newBuilder()
- .setCode(code)
- .setMessage(msg)
- .setPosixCode(posixCode)
- .build();
+ private Status(WireProtocol.AppStatusPB appStatusPB) {
+ this(appStatusPB.getCode(), appStatusPB.getMessage(), appStatusPB.getPosixCode());
}
private Status(WireProtocol.AppStatusPB.ErrorCode code, String msg) {
@@ -226,79 +240,79 @@ public class Status {
// Boolean status checks.
public boolean ok() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.OK;
+ return code == WireProtocol.AppStatusPB.ErrorCode.OK;
}
public boolean isCorruption() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.CORRUPTION;
+ return code == WireProtocol.AppStatusPB.ErrorCode.CORRUPTION;
}
public boolean isNotFound() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.NOT_FOUND;
+ return code == WireProtocol.AppStatusPB.ErrorCode.NOT_FOUND;
}
public boolean isNotSupported() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.NOT_SUPPORTED;
+ return code == WireProtocol.AppStatusPB.ErrorCode.NOT_SUPPORTED;
}
public boolean isInvalidArgument() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.INVALID_ARGUMENT;
+ return code == WireProtocol.AppStatusPB.ErrorCode.INVALID_ARGUMENT;
}
public boolean isIOError() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.IO_ERROR;
+ return code == WireProtocol.AppStatusPB.ErrorCode.IO_ERROR;
}
public boolean isAlreadyPresent() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.ALREADY_PRESENT;
+ return code == WireProtocol.AppStatusPB.ErrorCode.ALREADY_PRESENT;
}
public boolean isRuntimeError() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.RUNTIME_ERROR;
+ return code == WireProtocol.AppStatusPB.ErrorCode.RUNTIME_ERROR;
}
public boolean isNetworkError() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.NETWORK_ERROR;
+ return code == WireProtocol.AppStatusPB.ErrorCode.NETWORK_ERROR;
}
public boolean isIllegalState() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.ILLEGAL_STATE;
+ return code == WireProtocol.AppStatusPB.ErrorCode.ILLEGAL_STATE;
}
public boolean isNotAuthorized() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.NOT_AUTHORIZED;
+ return code == WireProtocol.AppStatusPB.ErrorCode.NOT_AUTHORIZED;
}
public boolean isAborted() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.ABORTED;
+ return code == WireProtocol.AppStatusPB.ErrorCode.ABORTED;
}
public boolean isRemoteError() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.REMOTE_ERROR;
+ return code == WireProtocol.AppStatusPB.ErrorCode.REMOTE_ERROR;
}
public boolean isServiceUnavailable() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.SERVICE_UNAVAILABLE;
+ return code == WireProtocol.AppStatusPB.ErrorCode.SERVICE_UNAVAILABLE;
}
public boolean isTimedOut() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.TIMED_OUT;
+ return code == WireProtocol.AppStatusPB.ErrorCode.TIMED_OUT;
}
public boolean isUninitialized() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.UNINITIALIZED;
+ return code == WireProtocol.AppStatusPB.ErrorCode.UNINITIALIZED;
}
public boolean isConfigurationError() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.CONFIGURATION_ERROR;
+ return code == WireProtocol.AppStatusPB.ErrorCode.CONFIGURATION_ERROR;
}
public boolean isIncomplete() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.INCOMPLETE;
+ return code == WireProtocol.AppStatusPB.ErrorCode.INCOMPLETE;
}
public boolean isEndOfFile() {
- return appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.END_OF_FILE;
+ return code == WireProtocol.AppStatusPB.ErrorCode.END_OF_FILE;
}
/**
@@ -306,7 +320,7 @@ public class Status {
* See also status.cc in the C++ codebase.
*/
private String getCodeAsString() {
- switch (appStatusPB.getCode().getNumber()) {
+ switch (code.getNumber()) {
case WireProtocol.AppStatusPB.ErrorCode.OK_VALUE:
return "OK";
case WireProtocol.AppStatusPB.ErrorCode.NOT_FOUND_VALUE:
@@ -346,7 +360,7 @@ public class Status {
case WireProtocol.AppStatusPB.ErrorCode.END_OF_FILE_VALUE:
return "End of file";
default:
- return "Unknown error (" + appStatusPB.getCode().getNumber() + ")";
+ return "Unknown error (" + code.getNumber() + ")";
}
}
@@ -355,7 +369,7 @@ public class Status {
* @return {@code -1} if no posix code is set. Otherwise, returns the posix code.
*/
public int getPosixCode() {
- return appStatusPB.getPosixCode();
+ return posixCode;
}
/**
@@ -363,7 +377,7 @@ public class Status {
* Intended for internal use only.
*/
String getCodeName() {
- return appStatusPB.getCode().name();
+ return code.name();
}
/**
@@ -371,7 +385,7 @@ public class Status {
* Intended for internal use only.
*/
String getMessage() {
- return appStatusPB.getMessage();
+ return message;
}
/**
@@ -380,12 +394,12 @@ public class Status {
@Override
public String toString() {
String str = getCodeAsString();
- if (appStatusPB.getCode() == WireProtocol.AppStatusPB.ErrorCode.OK) {
+ if (code == WireProtocol.AppStatusPB.ErrorCode.OK) {
return str;
}
- str = String.format("%s: %s", str, appStatusPB.getMessage());
- if (appStatusPB.getPosixCode() != -1) {
- str = String.format("%s (error %d)", str, appStatusPB.getPosixCode());
+ str = String.format("%s: %s", str, message);
+ if (posixCode != -1) {
+ str = String.format("%s (error %d)", str, posixCode);
}
return str;
}
http://git-wip-us.apache.org/repos/asf/kudu/blob/a9287093/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatus.java
----------------------------------------------------------------------
diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatus.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatus.java
index 2da9055..162283f 100644
--- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatus.java
+++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestStatus.java
@@ -22,6 +22,8 @@ import static org.junit.Assert.assertTrue;
import org.junit.Test;
+import java.util.Arrays;
+
public class TestStatus {
@Test
@@ -53,4 +55,33 @@ public class TestStatus {
assertEquals(2, s.getPosixCode());
assertEquals("Not found: File not found (error 2)", s.toString());
}
+
+ @Test
+ public void testMessageTooLong() {
+
+ // Test string that will not get abbreviated.
+ char[] chars = new char[Status.MAX_MESSAGE_LENGTH];
+ Arrays.fill(chars, 'a');
+ Status s = Status.Corruption(new String(chars));
+ assertEquals(Status.MAX_MESSAGE_LENGTH, s.getMessage().length());
+ assertEquals(s.getMessage().substring(Status.MAX_MESSAGE_LENGTH -
+ Status.ABBREVIATION_CHARS_LENGTH), "aaa");
+
+
+ // Test string just over the limit that will get abbreviated.
+ chars = new char[Status.MAX_MESSAGE_LENGTH + 1];
+ Arrays.fill(chars, 'a');
+ s = Status.Corruption(new String(chars));
+ assertEquals(Status.MAX_MESSAGE_LENGTH, s.getMessage().length());
+ assertEquals(s.getMessage().substring(Status.MAX_MESSAGE_LENGTH -
+ Status.ABBREVIATION_CHARS_LENGTH), Status.ABBREVIATION_CHARS);
+
+ // Test string that's way too big that will get abbreviated.
+ chars = new char[Status.MAX_MESSAGE_LENGTH * 2];
+ Arrays.fill(chars, 'a');
+ s = Status.Corruption(new String(chars));
+ assertEquals(Status.MAX_MESSAGE_LENGTH, s.getMessage().length());
+ assertEquals(s.getMessage().substring(Status.MAX_MESSAGE_LENGTH -
+ Status.ABBREVIATION_CHARS_LENGTH), Status.ABBREVIATION_CHARS);
+ }
}