You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by "Edward Ribeiro (JIRA)" <ji...@apache.org> on 2016/09/02 20:33:20 UTC
[jira] [Comment Edited] (ZOOKEEPER-2517) jute.maxbuffer is ignored
[ https://issues.apache.org/jira/browse/ZOOKEEPER-2517?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15459520#comment-15459520 ]
Edward Ribeiro edited comment on ZOOKEEPER-2517 at 9/2/16 8:32 PM:
-------------------------------------------------------------------
Hi [~arshad.mohammad],
A few comments:
{code}
String value = getProperty(key);
if (value == null) {
return defaultValue;
} else {
try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Configured value {} for property {} can not be parsed to int. Using default value {}.",
value, key, defaultValue);
}
}
}
return defaultValue;
{code}
can be rewritten as:
{code}
String value = getProperty(key);
if (value != null && value.trim().length > 0) {
try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
LOG.warn("Configured value {} for property {} can not be parsed to int. Using default value {}.",
value, key, defaultValue);
}
}
return defaultValue;
{code}
Also, I think a parser error of a parameter can be more insidious and dangerous bug to relegate it to debug only logging. I think it should be at least {{LOG.WARN}}. wdyt?
Finally, I don't like the name of the unit test: {{testGetInt}}. I think it's a bit vague so a more verbose, but clear name would be nice. Maybe {{testIntegerRetrievalFromProperty}} or something else... I no good with naming, btw.
Best regards!
was (Author: eribeiro):
Hi [~arshad.mohammad],
A few comments:
{code}
String value = getProperty(key);
if (value == null) {
return defaultValue;
} else {
try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Configured value {} for property {} can not be parsed to int. Using default value {}.",
value, key, defaultValue);
}
}
}
return defaultValue;
{code}
can be rewritten as:
{code}
String value = getProperty(key);
if (value != null) {
try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
if (LOG.isDebugEnabled()) {
LOG.debug("Configured value {} for property {} can not be parsed to int. Using default value {}.",
value, key, defaultValue);
}
}
}
return defaultValue;
{code}
Also, I think a parser error of a parameter can be more insidious and dangerous bug to relegate it to debug only logging. I think it should be at least {{LOG.WARN}}. wdyt?
Finally, I don't like the name of the unit test: {{testGetInt}}. I think it's a bit vague so a more verbose, but clear name would be nice. Maybe {{testIntegerRetrievalFromProperty}} or something else... I no good with naming, btw.
Best regards!
> jute.maxbuffer is ignored
> -------------------------
>
> Key: ZOOKEEPER-2517
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2517
> Project: ZooKeeper
> Issue Type: Bug
> Affects Versions: 3.5.2
> Reporter: Benjamin Jaton
> Assignee: Arshad Mohammad
> Priority: Critical
> Fix For: 3.5.3
>
> Attachments: ZOOKEEPER-2517-01.patch, ZOOKEEPER-2517.patch
>
>
> In ClientCnxnSocket.java the parsing of the system property is erroneous:
> {code}packetLen = Integer.getInteger(
> clientConfig.getProperty(ZKConfig.JUTE_MAXBUFFER),
> ZKClientConfig.CLIENT_MAX_PACKET_LENGTH_DEFAULT
> );{code}
> Javadoc of Integer.getInteger states "The first argument is treated as the name of a system property", whereas here the value of the property is passed.
> Instead I believe the author meant to write something like:
> {code}packetLen = Integer.parseInt(
> clientConfig.getProperty(
> ZKConfig.JUTE_MAXBUFFER,
> String.valueOf(ZKClientConfig.CLIENT_MAX_PACKET_LENGTH_DEFAULT)
> )
> );{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)