[jira] [Commented] (CASSANDRA-6947) SocketException being swallowed

2014-03-28 Thread Ding Yuan (JIRA)

[ 
https://issues.apache.org/jira/browse/CASSANDRA-6947?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13950779#comment-13950779
 ] 

Ding Yuan commented on CASSANDRA-6947:
--

I see. Thanks for the feedbacks!

> SocketException being swallowed
> ---
>
> Key: CASSANDRA-6947
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6947
> Project: Cassandra
>  Issue Type: Improvement
>Reporter: Ding Yuan
> Attachments: trunk-socketExceptions.txt
>
>
> There are a few cases where a SocketException is swallowed. For example: in 
> the following code snippet in TCustomServerSocket.java, a SocketException is 
> ignored even though the method declares that TTransportException can be 
> thrown. 
> {noformat}
> public void listen() throws TTransportException
> {
> // Make sure not to block on accept
> if (serverSocket != null)
> {
> try
> {
> serverSocket.setSoTimeout(100);
> }
> catch (SocketException sx)
> {
> logger.error("Could not set socket timeout.", sx);
> }
> }
> }
> {noformat}
> Propose to thrown TTransportException on SocketExceptions. Attaching a patch 
> against trunk for review. Thanks!



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Created] (CASSANDRA-6947) SocketException being swallowed

2014-03-27 Thread Ding Yuan (JIRA)
Ding Yuan created CASSANDRA-6947:


 Summary: SocketException being swallowed
 Key: CASSANDRA-6947
 URL: https://issues.apache.org/jira/browse/CASSANDRA-6947
 Project: Cassandra
  Issue Type: Improvement
Reporter: Ding Yuan
 Attachments: trunk-socketExceptions.txt

There are a few cases where a SocketException is swallowed. For example: in the 
following code snippet in TCustomServerSocket.java, a SocketException is 
ignored even though the method declares that TTransportException can be thrown. 

{noformat}
public void listen() throws TTransportException
{
// Make sure not to block on accept
if (serverSocket != null)
{
try
{
serverSocket.setSoTimeout(100);
}
catch (SocketException sx)
{
logger.error("Could not set socket timeout.", sx);
}
}
}
{noformat}

Propose to thrown TTransportException on SocketExceptions. Attaching a patch 
against trunk for review. Thanks!



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Created] (CASSANDRA-6946) Catching RequestExecutionException instead of Throwable

2014-03-27 Thread Ding Yuan (JIRA)
Ding Yuan created CASSANDRA-6946:


 Summary: Catching RequestExecutionException instead of Throwable
 Key: CASSANDRA-6946
 URL: https://issues.apache.org/jira/browse/CASSANDRA-6946
 Project: Cassandra
  Issue Type: Improvement
  Components: Core
Reporter: Ding Yuan
Priority: Trivial
 Attachments: trunk-overthrow.txt

In CassandraAuthorizer.java, there are a few cases where a Throwable is used to 
catch RequestExecutionException. Propose to catch the exact exception. 
Attaching a patch against trunk for review. Thanks.



--
This message was sent by Atlassian JIRA
(v6.2#6252)


[jira] [Updated] (CASSANDRA-6656) Exception logging

2014-02-06 Thread Ding Yuan (JIRA)

 [ 
https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ding Yuan updated CASSANDRA-6656:
-

Attachment: trunk-6656-v3.txt

> Exception logging
> -
>
> Key: CASSANDRA-6656
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6656
> Project: Cassandra
>  Issue Type: Improvement
>  Components: Core, Tools
>Reporter: Ding Yuan
>Assignee: Ding Yuan
>Priority: Trivial
> Fix For: 2.1
>
> Attachments: trunk-6656-v2.txt, trunk-6656-v3.txt, trunk-6656.txt
>
>
> Reporting a few cases where informative exceptions can be silently swallowed. 
> Attaching a proposed patch. 
> =
> Case 1
>   Line: 95, File: "org/apache/cassandra/utils/Hex.java"
> An actual failure in the underlying constructor will be lost.
> Propose to log it.
> {noformat}
> try
> {
> s = stringConstructor.newInstance(0, c.length, c);
> +   }
> +   catch (InvocationTargetException ite) {
> +   // The underlying constructor failed. Unwrapping the 
> exception.
> +   logger.info("Underlying constructor throws exception: ", 
> ite.getCause());
> }
> catch (Exception e)
> {
> // Swallowing as we'll just use a copying constructor
> }
> return s == null ? new String(c) : s;
> {noformat}
> ==
> =
> Case 2
>   Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java"
> The actual cause of comparator error can be lost as it can fail in multiple 
> locations.
> {noformat}
> AbstractType comparator = null;
> int header = getShortLength(bb);
> if ((header & 0x8000) == 0)
> {
> ByteBuffer value = getBytes(bb, header);
> try
> {
> comparator = TypeParser.parse(ByteBufferUtil.string(value));
> }
> catch (Exception e)
> {
> <--- can fail here
> // we'll deal with this below since comparator == null
> }
> }
> else
> {
> comparator = aliases.get((byte)(header & 0xFF));
> <--- can fail here
> }
> if (comparator == null)
> throw new MarshalException("Cannot find comparator for component 
> " + i);
> {noformat}
> Propose to log the exception.
> ==
> =
> Case 3
>   Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java"
> Exception ignored in finally. Propose log them with debug or trace.
> {noformat}
> 232: finally
> 233: {
> 234: try
> 235: {
> 236: ssProxy.removeNotificationListener(runner);
> 236: ssProxy.removeNotificationListener(runner);
> 237: jmxc.removeConnectionNotificationListener(runner);
> 238: }
> 239: catch (Throwable ignored) {}
> 240: }
> {noformat}
> Similar case is at line 264 in the same file.
> ==



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Commented] (CASSANDRA-6656) Exception logging

2014-02-06 Thread Ding Yuan (JIRA)

[ 
https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13894124#comment-13894124
 ] 

Ding Yuan commented on CASSANDRA-6656:
--

Agreed Mikhail Stepura! Attached another one to address your comment. 

> Exception logging
> -
>
> Key: CASSANDRA-6656
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6656
> Project: Cassandra
>  Issue Type: Improvement
>  Components: Core, Tools
>Reporter: Ding Yuan
>Assignee: Ding Yuan
>Priority: Trivial
> Fix For: 2.1
>
> Attachments: trunk-6656-v2.txt, trunk-6656-v3.txt, trunk-6656.txt
>
>
> Reporting a few cases where informative exceptions can be silently swallowed. 
> Attaching a proposed patch. 
> =
> Case 1
>   Line: 95, File: "org/apache/cassandra/utils/Hex.java"
> An actual failure in the underlying constructor will be lost.
> Propose to log it.
> {noformat}
> try
> {
> s = stringConstructor.newInstance(0, c.length, c);
> +   }
> +   catch (InvocationTargetException ite) {
> +   // The underlying constructor failed. Unwrapping the 
> exception.
> +   logger.info("Underlying constructor throws exception: ", 
> ite.getCause());
> }
> catch (Exception e)
> {
> // Swallowing as we'll just use a copying constructor
> }
> return s == null ? new String(c) : s;
> {noformat}
> ==
> =
> Case 2
>   Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java"
> The actual cause of comparator error can be lost as it can fail in multiple 
> locations.
> {noformat}
> AbstractType comparator = null;
> int header = getShortLength(bb);
> if ((header & 0x8000) == 0)
> {
> ByteBuffer value = getBytes(bb, header);
> try
> {
> comparator = TypeParser.parse(ByteBufferUtil.string(value));
> }
> catch (Exception e)
> {
> <--- can fail here
> // we'll deal with this below since comparator == null
> }
> }
> else
> {
> comparator = aliases.get((byte)(header & 0xFF));
> <--- can fail here
> }
> if (comparator == null)
> throw new MarshalException("Cannot find comparator for component 
> " + i);
> {noformat}
> Propose to log the exception.
> ==
> =
> Case 3
>   Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java"
> Exception ignored in finally. Propose log them with debug or trace.
> {noformat}
> 232: finally
> 233: {
> 234: try
> 235: {
> 236: ssProxy.removeNotificationListener(runner);
> 236: ssProxy.removeNotificationListener(runner);
> 237: jmxc.removeConnectionNotificationListener(runner);
> 238: }
> 239: catch (Throwable ignored) {}
> 240: }
> {noformat}
> Similar case is at line 264 in the same file.
> ==



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Commented] (CASSANDRA-6656) Exception logging

2014-02-05 Thread Ding Yuan (JIRA)

[ 
https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13892919#comment-13892919
 ] 

Ding Yuan commented on CASSANDRA-6656:
--

Thanks Mikhail. Updated the patch. 

> Exception logging
> -
>
> Key: CASSANDRA-6656
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6656
> Project: Cassandra
>  Issue Type: Improvement
>  Components: Core, Tools
>Reporter: Ding Yuan
>Assignee: Ding Yuan
>Priority: Trivial
> Fix For: 2.1
>
> Attachments: trunk-6656-v2.txt, trunk-6656.txt
>
>
> Reporting a few cases where informative exceptions can be silently swallowed. 
> Attaching a proposed patch. 
> =
> Case 1
>   Line: 95, File: "org/apache/cassandra/utils/Hex.java"
> An actual failure in the underlying constructor will be lost.
> Propose to log it.
> {noformat}
> try
> {
> s = stringConstructor.newInstance(0, c.length, c);
> +   }
> +   catch (InvocationTargetException ite) {
> +   // The underlying constructor failed. Unwrapping the 
> exception.
> +   logger.info("Underlying constructor throws exception: ", 
> ite.getCause());
> }
> catch (Exception e)
> {
> // Swallowing as we'll just use a copying constructor
> }
> return s == null ? new String(c) : s;
> {noformat}
> ==
> =
> Case 2
>   Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java"
> The actual cause of comparator error can be lost as it can fail in multiple 
> locations.
> {noformat}
> AbstractType comparator = null;
> int header = getShortLength(bb);
> if ((header & 0x8000) == 0)
> {
> ByteBuffer value = getBytes(bb, header);
> try
> {
> comparator = TypeParser.parse(ByteBufferUtil.string(value));
> }
> catch (Exception e)
> {
> <--- can fail here
> // we'll deal with this below since comparator == null
> }
> }
> else
> {
> comparator = aliases.get((byte)(header & 0xFF));
> <--- can fail here
> }
> if (comparator == null)
> throw new MarshalException("Cannot find comparator for component 
> " + i);
> {noformat}
> Propose to log the exception.
> ==
> =
> Case 3
>   Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java"
> Exception ignored in finally. Propose log them with debug or trace.
> {noformat}
> 232: finally
> 233: {
> 234: try
> 235: {
> 236: ssProxy.removeNotificationListener(runner);
> 236: ssProxy.removeNotificationListener(runner);
> 237: jmxc.removeConnectionNotificationListener(runner);
> 238: }
> 239: catch (Throwable ignored) {}
> 240: }
> {noformat}
> Similar case is at line 264 in the same file.
> ==



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Updated] (CASSANDRA-6656) Exception logging

2014-02-05 Thread Ding Yuan (JIRA)

 [ 
https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ding Yuan updated CASSANDRA-6656:
-

Attachment: trunk-6656-v2.txt

> Exception logging
> -
>
> Key: CASSANDRA-6656
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6656
> Project: Cassandra
>  Issue Type: Improvement
>  Components: Core, Tools
>Reporter: Ding Yuan
>Assignee: Ding Yuan
>Priority: Trivial
> Fix For: 2.1
>
> Attachments: trunk-6656-v2.txt, trunk-6656.txt
>
>
> Reporting a few cases where informative exceptions can be silently swallowed. 
> Attaching a proposed patch. 
> =
> Case 1
>   Line: 95, File: "org/apache/cassandra/utils/Hex.java"
> An actual failure in the underlying constructor will be lost.
> Propose to log it.
> {noformat}
> try
> {
> s = stringConstructor.newInstance(0, c.length, c);
> +   }
> +   catch (InvocationTargetException ite) {
> +   // The underlying constructor failed. Unwrapping the 
> exception.
> +   logger.info("Underlying constructor throws exception: ", 
> ite.getCause());
> }
> catch (Exception e)
> {
> // Swallowing as we'll just use a copying constructor
> }
> return s == null ? new String(c) : s;
> {noformat}
> ==
> =
> Case 2
>   Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java"
> The actual cause of comparator error can be lost as it can fail in multiple 
> locations.
> {noformat}
> AbstractType comparator = null;
> int header = getShortLength(bb);
> if ((header & 0x8000) == 0)
> {
> ByteBuffer value = getBytes(bb, header);
> try
> {
> comparator = TypeParser.parse(ByteBufferUtil.string(value));
> }
> catch (Exception e)
> {
> <--- can fail here
> // we'll deal with this below since comparator == null
> }
> }
> else
> {
> comparator = aliases.get((byte)(header & 0xFF));
> <--- can fail here
> }
> if (comparator == null)
> throw new MarshalException("Cannot find comparator for component 
> " + i);
> {noformat}
> Propose to log the exception.
> ==
> =
> Case 3
>   Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java"
> Exception ignored in finally. Propose log them with debug or trace.
> {noformat}
> 232: finally
> 233: {
> 234: try
> 235: {
> 236: ssProxy.removeNotificationListener(runner);
> 236: ssProxy.removeNotificationListener(runner);
> 237: jmxc.removeConnectionNotificationListener(runner);
> 238: }
> 239: catch (Throwable ignored) {}
> 240: }
> {noformat}
> Similar case is at line 264 in the same file.
> ==



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Updated] (CASSANDRA-6656) Exception logging

2014-02-05 Thread Ding Yuan (JIRA)

 [ 
https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ding Yuan updated CASSANDRA-6656:
-

Description: 
Reporting a few cases where informative exceptions can be silently swallowed. 
Attaching a proposed patch. 

=
Case 1
  Line: 95, File: "org/apache/cassandra/utils/Hex.java"

An actual failure in the underlying constructor will be lost.
Propose to log it.

{noformat}
try
{
s = stringConstructor.newInstance(0, c.length, c);
+   }
+   catch (InvocationTargetException ite) {
+   // The underlying constructor failed. Unwrapping the exception.
+   logger.info("Underlying constructor throws exception: ", 
ite.getCause());
}
catch (Exception e)
{
// Swallowing as we'll just use a copying constructor
}
return s == null ? new String(c) : s;
{noformat}
==
=
Case 2
  Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java"

The actual cause of comparator error can be lost as it can fail in multiple 
locations.
{noformat}
AbstractType comparator = null;
int header = getShortLength(bb);
if ((header & 0x8000) == 0)
{
ByteBuffer value = getBytes(bb, header);
try
{
comparator = TypeParser.parse(ByteBufferUtil.string(value));
}
catch (Exception e)
{
<--- can fail here
// we'll deal with this below since comparator == null
}
}
else
{
comparator = aliases.get((byte)(header & 0xFF));
<--- can fail here
}
if (comparator == null)
throw new MarshalException("Cannot find comparator for component " 
+ i);
{noformat}
Propose to log the exception.
==
=
Case 3
  Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java"
Exception ignored in finally. Propose log them with debug or trace.
{noformat}
232: finally
233: {
234: try
235: {
236: ssProxy.removeNotificationListener(runner);
236: ssProxy.removeNotificationListener(runner);
237: jmxc.removeConnectionNotificationListener(runner);
238: }
239: catch (Throwable ignored) {}
240: }
{noformat}
Similar case is at line 264 in the same file.
==

  was:
Reporting a few cases where informative exceptions can be silently swallowed. 
Attaching a proposed patch. 

=
Case 1
  Line: 95, File: "org/apache/cassandra/utils/Hex.java"

An actual failure in the underlying constructor will be lost.
Propose to log it.

try
{
s = stringConstructor.newInstance(0, c.length, c);
+   }
+   catch (InvocationTargetException ite) {
+   // The underlying constructor failed. Unwrapping the exception.
+   logger.info("Underlying constructor throws exception: ", 
ite.getCause());
}
catch (Exception e)
{
// Swallowing as we'll just use a copying constructor
}
return s == null ? new String(c) : s;
==
=
Case 2
  Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java"

The actual cause of comparator error can be lost as it can fail in multiple 
locations.

AbstractType comparator = null;
int header = getShortLength(bb);
if ((header & 0x8000) == 0)
{
ByteBuffer value = getBytes(bb, header);
try
{
comparator = TypeParser.parse(ByteBufferUtil.string(value));
}
catch (Exception e)
{
<--- can fail here
// we'll deal with this below since comparator == null
}
}
else
{
comparator = aliases.get((byte)(header & 0xFF));
<--- can fail here
}
if (comparator == null)
throw new MarshalException("Cannot find comparator for component " 
+ i);

Propose to log the exception.
==
=
Case 3
  Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java"
Exception ignored in finally. Propose log them with debug or trace.

232: finally
233: {
234: try
235: {
236: ssProxy.removeNotificationListener(runner);
236: ssProxy.removeNotificationListener(runner);
237: jmxc.remo

[jira] [Updated] (CASSANDRA-6656) Exception logging

2014-02-05 Thread Ding Yuan (JIRA)

 [ 
https://issues.apache.org/jira/browse/CASSANDRA-6656?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Ding Yuan updated CASSANDRA-6656:
-

Attachment: trunk-6656.txt

> Exception logging
> -
>
> Key: CASSANDRA-6656
> URL: https://issues.apache.org/jira/browse/CASSANDRA-6656
> Project: Cassandra
>  Issue Type: Improvement
>  Components: Core, Tools
>Reporter: Ding Yuan
> Fix For: 2.0.4
>
> Attachments: trunk-6656.txt
>
>
> Reporting a few cases where informative exceptions can be silently swallowed. 
> Attaching a proposed patch. 
> =
> Case 1
>   Line: 95, File: "org/apache/cassandra/utils/Hex.java"
> An actual failure in the underlying constructor will be lost.
> Propose to log it.
> try
> {
> s = stringConstructor.newInstance(0, c.length, c);
> +   }
> +   catch (InvocationTargetException ite) {
> +   // The underlying constructor failed. Unwrapping the 
> exception.
> +   logger.info("Underlying constructor throws exception: ", 
> ite.getCause());
> }
> catch (Exception e)
> {
> // Swallowing as we'll just use a copying constructor
> }
> return s == null ? new String(c) : s;
> ==
> =
> Case 2
>   Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java"
> The actual cause of comparator error can be lost as it can fail in multiple 
> locations.
> AbstractType comparator = null;
> int header = getShortLength(bb);
> if ((header & 0x8000) == 0)
> {
> ByteBuffer value = getBytes(bb, header);
> try
> {
> comparator = TypeParser.parse(ByteBufferUtil.string(value));
> }
> catch (Exception e)
> {
> <--- can fail here
> // we'll deal with this below since comparator == null
> }
> }
> else
> {
> comparator = aliases.get((byte)(header & 0xFF));
> <--- can fail here
> }
> if (comparator == null)
> throw new MarshalException("Cannot find comparator for component 
> " + i);
> Propose to log the exception.
> ==
> =
> Case 3
>   Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java"
> Exception ignored in finally. Propose log them with debug or trace.
> 232: finally
> 233: {
> 234: try
> 235: {
> 236: ssProxy.removeNotificationListener(runner);
> 236: ssProxy.removeNotificationListener(runner);
> 237: jmxc.removeConnectionNotificationListener(runner);
> 238: }
> 239: catch (Throwable ignored) {}
> 240: }
> Similar case is at line 264 in the same file.
> ==



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)


[jira] [Created] (CASSANDRA-6656) Exception logging

2014-02-05 Thread Ding Yuan (JIRA)
Ding Yuan created CASSANDRA-6656:


 Summary: Exception logging
 Key: CASSANDRA-6656
 URL: https://issues.apache.org/jira/browse/CASSANDRA-6656
 Project: Cassandra
  Issue Type: Improvement
  Components: Core, Tools
Reporter: Ding Yuan
 Fix For: 2.0.4


Reporting a few cases where informative exceptions can be silently swallowed. 
Attaching a proposed patch. 

=
Case 1
  Line: 95, File: "org/apache/cassandra/utils/Hex.java"

An actual failure in the underlying constructor will be lost.
Propose to log it.

try
{
s = stringConstructor.newInstance(0, c.length, c);
+   }
+   catch (InvocationTargetException ite) {
+   // The underlying constructor failed. Unwrapping the exception.
+   logger.info("Underlying constructor throws exception: ", 
ite.getCause());
}
catch (Exception e)
{
// Swallowing as we'll just use a copying constructor
}
return s == null ? new String(c) : s;
==
=
Case 2
  Line: 192, File: "org/apache/cassandra/db/marshal/DynamicCompositeType.java"

The actual cause of comparator error can be lost as it can fail in multiple 
locations.

AbstractType comparator = null;
int header = getShortLength(bb);
if ((header & 0x8000) == 0)
{
ByteBuffer value = getBytes(bb, header);
try
{
comparator = TypeParser.parse(ByteBufferUtil.string(value));
}
catch (Exception e)
{
<--- can fail here
// we'll deal with this below since comparator == null
}
}
else
{
comparator = aliases.get((byte)(header & 0xFF));
<--- can fail here
}
if (comparator == null)
throw new MarshalException("Cannot find comparator for component " 
+ i);

Propose to log the exception.
==
=
Case 3
  Line: 239, File: "org/apache/cassandra/tools/NodeProbe.java"
Exception ignored in finally. Propose log them with debug or trace.

232: finally
233: {
234: try
235: {
236: ssProxy.removeNotificationListener(runner);
236: ssProxy.removeNotificationListener(runner);
237: jmxc.removeConnectionNotificationListener(runner);
238: }
239: catch (Throwable ignored) {}
240: }

Similar case is at line 264 in the same file.
==



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)