You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by GitBox <gi...@apache.org> on 2022/01/21 03:08:46 UTC
[GitHub] [hadoop] smarthanwang commented on a change in pull request #3909: HDFS-16431. Truncate CallerContext in client side.
smarthanwang commented on a change in pull request #3909:
URL: https://github.com/apache/hadoop/pull/3909#discussion_r789318117
##########
File path: hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/CallerContext.java
##########
@@ -114,114 +120,160 @@ public String toString() {
return str;
}
- /** The caller context builder. */
- public static final class Builder {
+ /**
+ * The caller context related configs.
+ * Use singleton to avoid creating new instance every time.
+ */
+ private static final class Configs {
+
+ /** the maximum length limit for a context string*/
+ private int contextMaxLen;
+ /** the maximum length limit for a signature byte array*/
+ private int signatureMaxLen;
+ /** the filed separator for a context string */
+ private String fieldSeparator;
+
private static final String KEY_VALUE_SEPARATOR = ":";
+
/**
* The illegal separators include '\t', '\n', '='.
- * User should not set illegal separator.
+ * If the configured separator is illegal, use default
+ * {@link org.apache.hadoop.fs.CommonConfigurationKeysPublic#HADOOP_CALLER_CONTEXT_SEPARATOR_DEFAULT}.
*/
private static final Set<String> ILLEGAL_SEPARATORS =
Collections.unmodifiableSet(
new HashSet<>(Arrays.asList("\t", "\n", "=")));
- private final String fieldSeparator;
- private final StringBuilder sb = new StringBuilder();
- private byte[] signature;
- public Builder(String context) {
- this(context, HADOOP_CALLER_CONTEXT_SEPARATOR_DEFAULT);
+ private static volatile Configs INSTANCE;
+
+ private Configs(Configuration conf) {
+ this.init(conf);
}
- public Builder(String context, Configuration conf) {
- if (isValid(context)) {
- sb.append(context);
+ private void init(Configuration conf) {
+ this.contextMaxLen = conf.getInt(HADOOP_CALLER_CONTEXT_MAX_SIZE_KEY,
+ HADOOP_CALLER_CONTEXT_MAX_SIZE_DEFAULT);
+ if (this.contextMaxLen < 0) {
+ this.contextMaxLen = HADOOP_CALLER_CONTEXT_MAX_SIZE_DEFAULT;
}
- fieldSeparator = conf.get(HADOOP_CALLER_CONTEXT_SEPARATOR_KEY,
+
+ this.signatureMaxLen =
+ conf.getInt(HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_KEY,
+ HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_DEFAULT);
+ if (this.signatureMaxLen < 0) {
+ this.signatureMaxLen = HADOOP_CALLER_CONTEXT_SIGNATURE_MAX_SIZE_DEFAULT;
+ }
+
+ this.fieldSeparator = conf.get(HADOOP_CALLER_CONTEXT_SEPARATOR_KEY,
HADOOP_CALLER_CONTEXT_SEPARATOR_DEFAULT);
- checkFieldSeparator(fieldSeparator);
+ if (ILLEGAL_SEPARATORS.contains(fieldSeparator)) {
+ this.fieldSeparator = HADOOP_CALLER_CONTEXT_SEPARATOR_DEFAULT;
+ }
}
- public Builder(String context, String separator) {
- if (isValid(context)) {
- sb.append(context);
+ private static Configs getInstance(Configuration conf) {
+ if (INSTANCE == null) {
+ synchronized (Configs.class) {
+ if (INSTANCE == null) {
+ if (conf == null) {
+ conf = new Configuration();
+ }
+ INSTANCE = new Configs(conf);
+ }
+ }
}
- fieldSeparator = separator;
- checkFieldSeparator(fieldSeparator);
+ return INSTANCE;
}
+ }
- /**
- * Check whether the separator is legal.
- * The illegal separators include '\t', '\n', '='.
- * Throw IllegalArgumentException if the separator is Illegal.
- * @param separator the separator of fields.
- */
- private void checkFieldSeparator(String separator) {
- if (ILLEGAL_SEPARATORS.contains(separator)) {
- throw new IllegalArgumentException("Illegal field separator: "
- + separator);
+ /** The caller context builder. */
+ public static final class Builder {
+
+ private byte[] signature;
+ private final Configs configs;
+ private final StringBuilder ctx = new StringBuilder();
+
+ public Builder(String context) {
+ this(context, null);
+ }
+
+ public Builder(String context, Configuration conf) {
+ if (isValidFiled(context)) {
+ ctx.append(context);
+ }
+ configs = Configs.getInstance(conf);
+ }
+
+ @VisibleForTesting
+ Builder(String context, Configuration conf, boolean refresh) {
+ this(context, null);
+ if (refresh) {
+ configs.init(conf); // refresh configs for testing
}
}
/**
* Whether the field is valid.
- * @param field one of the fields in context.
+ * @param field target field.
* @return true if the field is not null or empty.
*/
- private boolean isValid(String field) {
+ private boolean isValidFiled(String field) {
return field != null && field.length() > 0;
}
public Builder setSignature(byte[] signature) {
- if (signature != null && signature.length > 0) {
+ if (isSignatureValid(signature)) {
this.signature = Arrays.copyOf(signature, signature.length);
}
return this;
}
/**
- * Get the context.
- * For example, the context is "key1:value1,key2:value2".
- * @return the valid context or null.
- */
- public String getContext() {
- return sb.length() > 0 ? sb.toString() : null;
- }
-
- /**
- * Get the signature.
- * @return the signature.
+ * Whether the signature is valid.
+ * @param signature target signature.
+ * @return true if the signature is not null and the length of signature
+ * is greater than 0 and less than or equal to the length limit.
*/
- public byte[] getSignature() {
- return signature;
+ private boolean isSignatureValid(byte[] signature) {
+ return signature != null && signature.length > 0
+ && signature.length <= configs.signatureMaxLen;
}
/**
- * Append new field to the context.
+ * Append new field to the context if the filed is not empty and length
+ * of the context is not exceeded limit.
* @param field one of fields to append.
* @return the builder.
*/
public Builder append(String field) {
- if (isValid(field)) {
- if (sb.length() > 0) {
- sb.append(fieldSeparator);
+ if (isValidFiled(field) && !isContextLengthExceedLimit()) {
Review comment:
> How about logging a warning message here or throw an exception? Makes troubleshooting easier.
Thanks for review, I think logging a warning message seems better, I will add it soon.
--
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: common-issues-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org