CASSANDRA-3989 (was: nodetool cleanup promotes all sstable and may cause CASSANDRA-3608)

[Maki Watanabe <wa...@gmail.com>]

Created a ticket.
https://issues.apache.org/jira/browse/CASSANDRA-3989

2012/3/2 Maki Watanabe <wa...@gmail.com>:
> Hello, I've faced same issue reported at:
>
> https://issues.apache.org/jira/browse/CASSANDRA-3608
>  nodetool cleanup fails on LeveledCompactionStrategy tables with
> ArrayIndexOutOfBoundsException
>
> in 1.0.7, and investigated the issue.
> I think there are two problems in LeveledManifest.java.
>
> 1. "generations" is a fixed length list, but add(SSTableReader, int)
> method doesn't check the boundary.
>     So if Cassandra try to promote sstables in the highest
> generation, it will cause ArrayIndexOutOfBoundsException.
>
> 2. promote method promotes all sstables during cleanup compaction.
>    If you have only one sstable at L1, nodetool cleanup promotes it to L2.
>    If you run cleanup 6 times, the sstable is promoted to L7.
>    7th cleanup causes ArrayIndexOutOfBoundsException when you use
> sstable_size_in_mb: 5.
>
> I made a patch for the issue#2, because issue#1 will not happen until
> the node will store
> Peta Bytes of data with the issue#2 patch.
>
> The patch is made against cassandra-1.0.7.
> I'm not familiar with git. If I need to make the patch in other
> format, please let me know.
>
> maki
>
>
> From 47ab7dc007db4c1d4ff28786acea1c99cbf6e19e Mon Sep 17 00:00:00 2001
> From: Maki Watanabe <wa...@gmail.com>
> Date: Fri, 2 Mar 2012 00:28:30 +0900
> Subject: [PATCH] Fix promote() not to promote files at cleanup compaction
>  etc.
>
> ---
>  .../cassandra/db/compaction/LeveledManifest.java   |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/src/java/org/apache/cassandra/db/compaction/LeveledManifest.java
> b/src/java/org/apache/cassandra/db/compaction/LeveledManifest.java
> index 40a0a17..8eea2e3 100644
> --- a/src/java/org/apache/cassandra/db/compaction/LeveledManifest.java
> +++ b/src/java/org/apache/cassandra/db/compaction/LeveledManifest.java
> @@ -163,19 +163,27 @@ public class LeveledManifest
>         // plus one if the removed were all on the same level
>         int minimumLevel = Integer.MAX_VALUE;
>         int maximumLevel = 0;
> +        int removedCount = 0;
>         for (SSTableReader sstable : removed)
>         {
>             int thisLevel = levelOf(sstable);
>             maximumLevel = Math.max(maximumLevel, thisLevel);
>             minimumLevel = Math.min(minimumLevel, thisLevel);
>             remove(sstable);
> +            removedCount++;
>         }
>
>         // it's valid to do a remove w/o an add (e.g. on truncate)
>         if (!added.iterator().hasNext())
>             return;
>
> -        int newLevel = minimumLevel == maximumLevel ? maximumLevel +
> 1 : maximumLevel;
> +        int newLevel = 0;
> +        if (removedCount == 1)
> +               // We don't want to promote sstable at cleanup compaction etc.
> +               newLevel = maximumLevel;
> +        else
> +               newLevel = minimumLevel == maximumLevel ? maximumLevel + 1 :
> maximumLevel;
> +
>         newLevel = skipLevels(newLevel, added);
>         assert newLevel > 0;
>         if (logger.isDebugEnabled())
> @@ -295,6 +303,7 @@ public class LeveledManifest
>
>     private void add(SSTableReader sstable, int level)
>     {
> +       // If you promote highest generation, this will cause
> ArrayIndexOutOfBoundsException (CASSANDRA-3608)
>         generations[level].add(sstable);
>     }
>
> --
> 1.7.7.4



-- 
w3m