Cassandra
  1. Cassandra
  2. CASSANDRA-4307

isMarkedForDelete can return false if it is a few seconds in the future

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Fix Version/s: 1.1.2
    • Component/s: Core
    • Labels:
      None

      Description

      The patch in CASSANDRA-3716 causes some weird issues to arrise when server times don't exactly match up (and since the resolution is seconds, it is easy to be off by just enough to see it).

      I am seeing a case where during schema propagation .isMarkedForDelete() is checked, but the timestamp is a few seconds in the future because the schema was sent from a different node. The code then happily tries to interpret the value of the column as a String, but it is actually the Int encoded deletion time.

      Here is an example in the code that does this check and will do the wrong thing if the deletion timestamp is even just a few seconds in the future: https://github.com/apache/cassandra/blob/47f0cc5d38d272ec9f7d6179eb3ffa28c6f74107/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java#L607-609

      To prove that this is a problem, here is a stack trace of a machine trying to interpret the "localDeletionTime" value of a DeletedColumn as UTF-8 because the .isMarkedForDeletion() check failed:

      https://gist.github.com/deb064d4377d206368d3

      1. 4307-test.txt
        2 kB
        Wade Simmons
      2. 4307.txt
        1.0 kB
        Sylvain Lebresne

        Activity

        Wade Simmons created issue -
        Wade Simmons made changes -
        Field Original Value New Value
        Description The patch in CASSANDRA-3716 causes some weird issues to arrise when server times don't exactly match up (and since the resolution is seconds, it is easy to be off by just enough to see it).

        I am seeing a case where during schema propagation .isMarkedForDelete() is checked, but the timestamp is a few seconds in the future because the schema was sent from a different node. The code then happily tries to interpret the value of the column as a String, but it is actually the Int encoded deletion time.

        Here is an example in the code that does this check and will do the wrong thing if the deletion timestamp is even just a few seconds in the future: https://github.com/apache/cassandra/blob/trunk/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java#L607-609

        To prove that this is a problem, here is a stack trace of a machine trying to interpret the "localDeletionTime" value of a DeletedColumn as UTF-8 because the .isMarkedForDeletion() check failed:

        https://gist.github.com/deb064d4377d206368d3
        The patch in CASSANDRA-3716 causes some weird issues to arrise when server times don't exactly match up (and since the resolution is seconds, it is easy to be off by just enough to see it).

        I am seeing a case where during schema propagation .isMarkedForDelete() is checked, but the timestamp is a few seconds in the future because the schema was sent from a different node. The code then happily tries to interpret the value of the column as a String, but it is actually the Int encoded deletion time.

        Here is an example in the code that does this check and will do the wrong thing if the deletion timestamp is even just a few seconds in the future: https://github.com/apache/cassandra/blob/47f0cc5d38d272ec9f7d6179eb3ffa28c6f74107/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java#L607-609

        To prove that this is a problem, here is a stack trace of a machine trying to interpret the "localDeletionTime" value of a DeletedColumn as UTF-8 because the .isMarkedForDeletion() check failed:

        https://gist.github.com/deb064d4377d206368d3
        Sylvain Lebresne made changes -
        Attachment 4307.txt [ 12530959 ]
        Sylvain Lebresne made changes -
        Assignee Sylvain Lebresne [ slebresne ]
        Status Open [ 1 ] Patch Available [ 10002 ]
        Fix Version/s 1.1.2 [ 12321445 ]
        Wade Simmons made changes -
        Attachment 4307-test.txt [ 12530986 ]
        Sylvain Lebresne made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Reviewer jbellis
        Resolution Fixed [ 1 ]
        Gavin made changes -
        Workflow no-reopen-closed, patch-avail [ 12672053 ] patch-available, re-open possible [ 12749822 ]
        Gavin made changes -
        Workflow patch-available, re-open possible [ 12749822 ] reopen-resolved, no closed status, patch-avail, testing [ 12757333 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development