Cassandra
  1. Cassandra
  2. CASSANDRA-5026

Reduce log spam from counter shard warnings

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Fix Version/s: 1.1.8, 1.2.0 rc1
    • Component/s: Core
    • Labels:
      None

      Description

      The invalid counter shard warning we can get after unclean shutdown in periodic commitlog mode or after node movement (CASSANDRA-4071) can spam the log hard since it is logged once per read until compaction merges it away.

      1. 5026-v2.txt
        3 kB
        Jonathan Ellis
      2. 5026.txt
        5 kB
        Sylvain Lebresne

        Activity

        Hide
        Jonathan Ellis added a comment -

        renamed to isCompactionManager, edited log message, and committed

        Show
        Jonathan Ellis added a comment - renamed to isCompactionManager, edited log message, and committed
        Hide
        Sylvain Lebresne added a comment -

        I'm good with v2, though as said before it sets the flag for anything that is executed by the compactionExecutor (validation, cleanup, scrub, ...) not just actual compaction, so maybe we can rename it from "isCompacting"?

        Show
        Sylvain Lebresne added a comment - I'm good with v2, though as said before it sets the flag for anything that is executed by the compactionExecutor (validation, cleanup, scrub, ...) not just actual compaction, so maybe we can rename it from "isCompacting"?
        Hide
        Jonathan Ellis added a comment -

        v2 sets in beforeExecute. (would prefer to set in the ThreadFactory but that doesn't seem possible, since there is no api to set a ThreadLocal for an arbitrary Thread object.)

        Show
        Jonathan Ellis added a comment - v2 sets in beforeExecute. (would prefer to set in the ThreadFactory but that doesn't seem possible, since there is no api to set a ThreadLocal for an arbitrary Thread object.)
        Hide
        Sylvain Lebresne added a comment -

        That wouldn't be crazy but where else do you propose to set it to true? I suppose we could avoid to turn it off each time, but at the same time it's not like it's costing anything and it has the vague advantages that we can control which operation actually triggers it (for instance the patch don't trigger it with validation compaction (that don't "repair" the shard either)).

        Show
        Sylvain Lebresne added a comment - That wouldn't be crazy but where else do you propose to set it to true? I suppose we could avoid to turn it off each time, but at the same time it's not like it's costing anything and it has the vague advantages that we can control which operation actually triggers it (for instance the patch don't trigger it with validation compaction (that don't "repair" the shard either)).
        Hide
        Jonathan Ellis added a comment -

        Can we just assume that any CompactionManager thread will want to log and not have to worry about trying to turn it on and off constantly?

        Show
        Jonathan Ellis added a comment - Can we just assume that any CompactionManager thread will want to log and not have to worry about trying to turn it on and off constantly?
        Hide
        Sylvain Lebresne added a comment -

        only logging it during compaction

        Agreed that it's the best option. It'll also make the message more true in the sense that the message says "will pick highest to self-heal", but we only "self-heal" things on compaction.

        Patch attached for that.

        Show
        Sylvain Lebresne added a comment - only logging it during compaction Agreed that it's the best option. It'll also make the message more true in the sense that the message says "will pick highest to self-heal", but we only "self-heal" things on compaction. Patch attached for that.
        Hide
        Jonathan Ellis added a comment -

        Possible fixes include:

        1. forcing compaction when we see the problem
        2. keeping a Map of shards we've logged it for and shutting up about it after the first time
        3. only logging it during compaction instead of the read path

        (1) is conceptually simple but problematic for LCS. (2) intertwines logging w/ server logic at a level that makes me uncomfortable. Which leaves (3) as the best option IMO.

        Show
        Jonathan Ellis added a comment - Possible fixes include: forcing compaction when we see the problem keeping a Map of shards we've logged it for and shutting up about it after the first time only logging it during compaction instead of the read path (1) is conceptually simple but problematic for LCS. (2) intertwines logging w/ server logic at a level that makes me uncomfortable. Which leaves (3) as the best option IMO.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development