Cassandra
  1. Cassandra
  2. CASSANDRA-3989

nodetool cleanup/scrub/upgradesstables promotes all sstables to next level (LeveledCompaction)

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Fix Version/s: 1.0.9, 1.1.0
    • Component/s: Core
    • Labels:
      None
    • Environment:

      RHEL6

      Description

      1.0.7 + LeveledCompactionStrategy
      If you run nodetool cleanup, scrub, or upgradesstables, Cassandra execute compaction for each sstable. During the compaction, it put the new sstable to next level of the original sstable. If you run cleanup many times, sstables will reached to the highest level, and CASSANDRA-3608 will happens at next cleanup.

      Reproduce procedure:

      1. create column family CF1 with compaction_strategy=LeveledCompactionStrategy and compaction_strategy_options= {sstable_size_in_mb: 5}

        ;

      2. Insert some data into CF1.
      3. nodetool flush
      4. Verify the sstable is created at L1 in CF1.json
      5. nodetool cleanup
      6. Verify sstable in L1 is removed and new sstable is created at L2 in CF1.json
      7. repeat nodetool cleanup some times
      1. 3989.txt
        19 kB
        Sylvain Lebresne

        Activity

        Maki Watanabe created issue -
        Maki Watanabe made changes -
        Field Original Value New Value
        Status Open [ 1 ] Patch Available [ 10002 ]
        Maki Watanabe made changes -
        Comment [ Fix LeveledManifest.promote not to promote sstables to next level if the number of source sstables (removed) is 1. ]
        Hide
        Maki Watanabe added a comment -

        Fix LeveledManifest.promote will not promote sstables to next level if the number of source sstables (removed) is one.

        Show
        Maki Watanabe added a comment - Fix LeveledManifest.promote will not promote sstables to next level if the number of source sstables (removed) is one.
        Maki Watanabe made changes -
        Attachment 0001-Fix-promote-not-to-promote-files-at-cleanup-compacti.patch [ 12516782 ]
        Jonathan Ellis made changes -
        Fix Version/s 1.0.9 [ 12319856 ]
        Fix Version/s 1.1.0 [ 12317615 ]
        Affects Version/s 1.0.0 [ 12316349 ]
        Affects Version/s 1.0.7 [ 12319244 ]
        Hide
        Jonathan Ellis added a comment -

        Nice bug hunting. Committed a lightly edited version of your fix: https://git-wip-us.apache.org/repos/asf?p=cassandra.git;a=commitdiff;h=53fb52ac713e5471edd988b59cbd75f202a4f57b

        Thanks!

        Show
        Jonathan Ellis added a comment - Nice bug hunting. Committed a lightly edited version of your fix: https://git-wip-us.apache.org/repos/asf?p=cassandra.git;a=commitdiff;h=53fb52ac713e5471edd988b59cbd75f202a4f57b Thanks!
        Jonathan Ellis made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Assignee Maki Watanabe [ makiw ]
        Reviewer jbellis
        Resolution Fixed [ 1 ]
        Hide
        Maki Watanabe added a comment -

        Johathan,
        This fix has a problem. If you have only one Level 0 sstable, and there are no Level 1 sstables involved in the compaction, promote method will chooses 0 as newLevel, and it causes assertion.
        Should I create a new ticket?

        Show
        Maki Watanabe added a comment - Johathan, This fix has a problem. If you have only one Level 0 sstable, and there are no Level 1 sstables involved in the compaction, promote method will chooses 0 as newLevel, and it causes assertion. Should I create a new ticket?
        Hide
        Jonathan Ellis added a comment -

        No, let's reopen this one

        Show
        Jonathan Ellis added a comment - No, let's reopen this one
        Jonathan Ellis made changes -
        Resolution Fixed [ 1 ]
        Status Resolved [ 5 ] Reopened [ 4 ]
        Hide
        Maki Watanabe added a comment - - edited

        Another problem with the fix.
        In following condition, the background compaction task start looping.

        • No L0 sstable
        • Enough number of L1 sstables, exceed L1 capacity (compaction score > 1.1)

        It seems the background task try to promote sstables to reduce compaction score, but it can't by this fix, then it will compact each L1 sstables forever.

        Show
        Maki Watanabe added a comment - - edited Another problem with the fix. In following condition, the background compaction task start looping. No L0 sstable Enough number of L1 sstables, exceed L1 capacity (compaction score > 1.1) It seems the background task try to promote sstables to reduce compaction score, but it can't by this fix, then it will compact each L1 sstables forever.
        Hide
        Maki Watanabe added a comment - - edited

        It is more complicated than I first thought.
        getCandidatesFor(int level) returns next compaction candidates for the level by:

        • sort the generations and find a sstable where we left off last time
        • and then find overlappng sstables from next level

        So if we add a new sstable into same level, getCompactionCandidates won't return empty, and then getNextBackgoundTask returns non-null task forever.
        I think we should better to back out the fix and look for better strategy to resolve the issue.

        Show
        Maki Watanabe added a comment - - edited It is more complicated than I first thought. getCandidatesFor(int level) returns next compaction candidates for the level by: sort the generations and find a sstable where we left off last time and then find overlappng sstables from next level So if we add a new sstable into same level, getCompactionCandidates won't return empty, and then getNextBackgoundTask returns non-null task forever. I think we should better to back out the fix and look for better strategy to resolve the issue.
        Hide
        Sylvain Lebresne added a comment -

        I agree, the committed fix does more harm than it helps so I reverted it.
        I'm trying some other approach so will hopefully attach another version soon.

        Show
        Sylvain Lebresne added a comment - I agree, the committed fix does more harm than it helps so I reverted it. I'm trying some other approach so will hopefully attach another version soon.
        Hide
        Sylvain Lebresne added a comment -

        Attaching another approach that makes the compaction type available to the leveled compaction that for cleanup, scrub and upgradeSSTables simply replace the old sstable by the new one (without changing level or anything else). The rational is those operation don't really "change" the sstable content and should simply "replace" sstables.

        Show
        Sylvain Lebresne added a comment - Attaching another approach that makes the compaction type available to the leveled compaction that for cleanup, scrub and upgradeSSTables simply replace the old sstable by the new one (without changing level or anything else). The rational is those operation don't really "change" the sstable content and should simply "replace" sstables.
        Sylvain Lebresne made changes -
        Attachment 3989.txt [ 12517399 ]
        Sylvain Lebresne made changes -
        Status Reopened [ 4 ] Patch Available [ 10002 ]
        Hide
        Jonathan Ellis added a comment -

        +1

        Show
        Jonathan Ellis added a comment - +1
        Hide
        Sylvain Lebresne added a comment -

        Committed, thanks

        Show
        Sylvain Lebresne added a comment - Committed, thanks
        Sylvain Lebresne made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Assignee Maki Watanabe [ makiw ] Sylvain Lebresne [ slebresne ]
        Resolution Fixed [ 1 ]
        Hide
        Peter Schuller added a comment -

        Hmm, Does this address the case from CASSANDRA-3952?

        I think this might happen whenever there is exactly one sstable in L0 that is large enough for the score for L0 to be > 1, and if L1 is full (so that skipLevels doesn't promote).

        It's possible the sstables I had lying around from sized-tiered compaction combined with my write load conspired to trigger this case.

        Show
        Peter Schuller added a comment - Hmm, Does this address the case from CASSANDRA-3952 ? I think this might happen whenever there is exactly one sstable in L0 that is large enough for the score for L0 to be > 1, and if L1 is full (so that skipLevels doesn't promote). It's possible the sstables I had lying around from sized-tiered compaction combined with my write load conspired to trigger this case.
        Hide
        Peter Schuller added a comment -

        Sorry, I think I just pulled before merge-to-trunk and was still looking at a trunk containing the original version of the patch.

        Show
        Peter Schuller added a comment - Sorry, I think I just pulled before merge-to-trunk and was still looking at a trunk containing the original version of the patch.
        Maki Watanabe made changes -
        Attachment 0001-Fix-promote-not-to-promote-files-at-cleanup-compacti.patch [ 12516782 ]
        Hide
        Maki Watanabe added a comment -

        Removed first patch file I attached here because it is harmful.

        Show
        Maki Watanabe added a comment - Removed first patch file I attached here because it is harmful.
        Gavin made changes -
        Workflow no-reopen-closed, patch-avail [ 12655596 ] patch-available, re-open possible [ 12749591 ]
        Gavin made changes -
        Workflow patch-available, re-open possible [ 12749591 ] reopen-resolved, no closed status, patch-avail, testing [ 12757118 ]

          People

          • Assignee:
            Sylvain Lebresne
            Reporter:
            Maki Watanabe
            Reviewer:
            Jonathan Ellis
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development