[
https://issues.apache.org/jira/browse/CASSANDRA-5220?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14661132#comment-14661132
]
Stefania commented on CASSANDRA-5220:
-------------------------------------
Attaching information on the Coverity defects reported against this patch. I
propose to handle as follows:
* CID 1315416 - TokenRangeComparator should be serializable - ignore since we
have several other comparators that are not serializable
* CID 1315412: RESOURCE_LEAK in CompactionStrategyManager line 368 - false
positive, even though the lists aren't closed, their iterators are closed or
returned for future use, same problem was already present before
* CID 1315410: Possible NPE in MerkleTrees line 172 - this code is for testing
only so I propose to convert possible NPE to AssertionError and add
@VisibleForTesting
* CID 1315409: Possible NPE in MerkleTrees line 130 - this code is for testing
only so I propose to convert possible NPE to AssertionError and add
@VisibleForTesting
* CID 1315407: Possible NPE in MerkleTrees line 162 - I verified it cannot
happen so I propose to convert possible NPE to AssertionError
Proposed patch on the [3.0
branch|https://github.com/stef1927/cassandra/tree/5220-3.0] :
[here|https://github.com/stef1927/cassandra/commit/aa419e331783e78c9aafe79eaeb0362e2338a6b6].
Here are the defects details:
{code}
** CID 1315416: FindBugs: Bad practice
(FB.SE_COMPARATOR_SHOULD_BE_SERIALIZABLE)
/src/java/org/apache/cassandra/utils/MerkleTrees.java: 423 in ()
________________________________________________________________________________________________________
*** CID 1315416: FindBugs: Bad practice
(FB.SE_COMPARATOR_SHOULD_BE_SERIALIZABLE)
/src/java/org/apache/cassandra/utils/MerkleTrees.java: 423 in ()
417 }
418 return size;
419 }
420
421 }
422
>>> CID 1315416: FindBugs: Bad practice
>>> (FB.SE_COMPARATOR_SHOULD_BE_SERIALIZABLE)
>>> org.apache.cassandra.utils.MerkleTrees$TokenRangeComparator implements
>>> Comparator but not Serializable.
423 private static class TokenRangeComparator implements
Comparator<Range<Token>>
424 {
425 @Override
426 public int compare(Range<Token> rt1, Range<Token> rt2)
427 {
428 if (rt1.left.compareTo(rt2.left) == 0)
** CID 1315412: (RESOURCE_LEAK)
/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java:
368 in
org.apache.cassandra.db.compaction.CompactionStrategyManager.getScanners(java.util.Collection,
java.util.Collection)()
/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java:
368 in
org.apache.cassandra.db.compaction.CompactionStrategyManager.getScanners(java.util.Collection,
java.util.Collection)()
________________________________________________________________________________________________________
*** CID 1315412: (RESOURCE_LEAK)
/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java:
368 in
org.apache.cassandra.db.compaction.CompactionStrategyManager.getScanners(java.util.Collection,
java.util.Collection)()
362
363 for (ISSTableScanner scanner :
Iterables.concat(repairedScanners.scanners, unrepairedScanners.scanners))
364 {
365 if (!scanners.add(scanner))
366 scanner.close();
367 }
>>> CID 1315412: (RESOURCE_LEAK)
>>> Variable "repairedScanners" going out of scope leaks the resource it
>>> refers to.
368 }
369
370 return new AbstractCompactionStrategy.ScannerList(new
ArrayList<>(scanners));
371 }
372
373 public synchronized AbstractCompactionStrategy.ScannerList
getScanners(Collection<SSTableReader> sstables)
/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java:
368 in
org.apache.cassandra.db.compaction.CompactionStrategyManager.getScanners(java.util.Collection,
java.util.Collection)()
362
363 for (ISSTableScanner scanner :
Iterables.concat(repairedScanners.scanners, unrepairedScanners.scanners))
364 {
365 if (!scanners.add(scanner))
366 scanner.close();
367 }
>>> CID 1315412: (RESOURCE_LEAK)
>>> Variable "unrepairedScanners" going out of scope leaks the resource it
>>> refers to.
368 }
369
370 return new AbstractCompactionStrategy.ScannerList(new
ArrayList<>(scanners));
371 }
372
373 public synchronized AbstractCompactionStrategy.ScannerList
getScanners(Collection<SSTableReader> sstables)
** CID 1315410: Null pointer dereferences (NULL_RETURNS)
/src/java/org/apache/cassandra/utils/MerkleTrees.java: 172 in
org.apache.cassandra.utils.MerkleTrees.invalidate(org.apache.cassandra.dht.Token)()
________________________________________________________________________________________________________
*** CID 1315410: Null pointer dereferences (NULL_RETURNS)
/src/java/org/apache/cassandra/utils/MerkleTrees.java: 172 in
org.apache.cassandra.utils.MerkleTrees.invalidate(org.apache.cassandra.dht.Token)()
166 * Invalidate the MerkleTree responsible for the given token.
167 *
168 * @param t
169 */
170 public void invalidate(Token t)
171 {
>>> CID 1315410: Null pointer dereferences (NULL_RETURNS)
>>> Calling a method on null object "getMerkleTree(t)".
172 getMerkleTree(t).invalidate(t);
173 }
174
175 /**
176 * Get the MerkleTree responsible for the given token range.
177 *
** CID 1315409: Null pointer dereferences (NULL_RETURNS)
/src/java/org/apache/cassandra/utils/MerkleTrees.java: 130 in
org.apache.cassandra.utils.MerkleTrees.get(org.apache.cassandra.dht.Token)()
________________________________________________________________________________________________________
*** CID 1315409: Null pointer dereferences (NULL_RETURNS)
/src/java/org/apache/cassandra/utils/MerkleTrees.java: 130 in
org.apache.cassandra.utils.MerkleTrees.get(org.apache.cassandra.dht.Token)()
124 *
125 * @param t
126 * @return
127 */
128 public MerkleTree.TreeRange get(Token t)
129 {
>>> CID 1315409: Null pointer dereferences (NULL_RETURNS)
>>> Calling a method on null object "getMerkleTree(t)".
130 return getMerkleTree(t).get(t);
131 }
132
133 /**
134 * Init all MerkleTree's with an even tree distribution.
135 */
** CID 1315407: Null pointer dereferences (NULL_RETURNS)
/src/java/org/apache/cassandra/utils/MerkleTrees.java: 162 in
org.apache.cassandra.utils.MerkleTrees.split(org.apache.cassandra.dht.Token)()
________________________________________________________________________________________________________
*** CID 1315407: Null pointer dereferences (NULL_RETURNS)
/src/java/org/apache/cassandra/utils/MerkleTrees.java: 162 in
org.apache.cassandra.utils.MerkleTrees.split(org.apache.cassandra.dht.Token)()
156 *
157 * @param t
158 * @return
159 */
160 public boolean split(Token t)
161 {
>>> CID 1315407: Null pointer dereferences (NULL_RETURNS)
>>> Calling a method on null object "getMerkleTree(t)".
162 return getMerkleTree(t).split(t);
163 }
164
165 /**
166 * Invalidate the MerkleTree responsible for the given token.
167
{code}
> Repair improvements when using vnodes
> -------------------------------------
>
> Key: CASSANDRA-5220
> URL: https://issues.apache.org/jira/browse/CASSANDRA-5220
> Project: Cassandra
> Issue Type: Improvement
> Components: Core
> Affects Versions: 1.2.0 beta 1
> Reporter: Brandon Williams
> Assignee: Marcus Olsson
> Labels: performance, repair
> Attachments: 5220-yourkit.png, 5220-yourkit.tar.bz2,
> cassandra-3.0-5220-1.patch, cassandra-3.0-5220-2.patch,
> cassandra-3.0-5220.patch
>
>
> Currently when using vnodes, repair takes much longer to complete than
> without them. This appears at least in part because it's using a session per
> range and processing them sequentially. This generates a lot of log spam
> with vnodes, and while being gentler and lighter on hard disk deployments,
> ssd-based deployments would often prefer that repair be as fast as possible.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
