Uploaded image for project: 'Cassandra'
  1. Cassandra
  2. CASSANDRA-4432

Change nanoTime() to currentTimeInMillis() in schema related code.

    Details

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

      Description

      From nanoTime() description:

      "The value returned represents nanoseconds since some fixed but arbitrary time (perhaps in the future, so values may be negative). This method provides nanosecond precision, but not necessarily nanosecond accuracy. No guarantees are made about how frequently values change."

      Also see http://www.mail-archive.com/dev@cassandra.apache.org/msg04992.html

      1. CASSANDRA-4432.patch
        3 kB
        Pavel Yaskevich

        Activity

        Hide
        jbellis Jonathan Ellis added a comment -

        +1

        Show
        jbellis Jonathan Ellis added a comment - +1
        Hide
        mtheroux2 Michael Theroux added a comment -

        In order to be consistent with timestamps elsewhere, shouldn't the changes read System.currentTimeMills * 1000?

        Show
        mtheroux2 Michael Theroux added a comment - In order to be consistent with timestamps elsewhere, shouldn't the changes read System.currentTimeMills * 1000?
        Hide
        xedin Pavel Yaskevich added a comment -

        We have millis * 1000 for columns but I don't see a reason to do that for schema as all columns would be created with the same millis resolution by all migrations.

        Show
        xedin Pavel Yaskevich added a comment - We have millis * 1000 for columns but I don't see a reason to do that for schema as all columns would be created with the same millis resolution by all migrations.
        Hide
        mtheroux2 Michael Theroux added a comment - - edited

        Its a consideration for existing users. Any user who would have created a keyspace before this fix, will have timestamps in whatever System.nanoTime() returns. The bigger you make the timestamp, the less users will be stuck attempting to fix their timestamps so they can update their schema (I guess using sstable2json?). Plus, its consistent with columns

        Show
        mtheroux2 Michael Theroux added a comment - - edited Its a consideration for existing users. Any user who would have created a keyspace before this fix, will have timestamps in whatever System.nanoTime() returns. The bigger you make the timestamp, the less users will be stuck attempting to fix their timestamps so they can update their schema (I guess using sstable2json?). Plus, its consistent with columns
        Hide
        xedin Pavel Yaskevich added a comment -

        Committed.

        Show
        xedin Pavel Yaskevich added a comment - Committed.
        Hide
        slebresne Sylvain Lebresne added a comment -

        I suppose that since we'll access the system table (including the schema) directly in CQL3, it might be worth using micro-seconds.

        Show
        slebresne Sylvain Lebresne added a comment - I suppose that since we'll access the system table (including the schema) directly in CQL3, it might be worth using micro-seconds.
        Hide
        jbellis Jonathan Ellis added a comment -

        Good point.

        Show
        jbellis Jonathan Ellis added a comment - Good point.
        Hide
        xedin Pavel Yaskevich added a comment -

        Ok, I will add * 1000 there.

        Show
        xedin Pavel Yaskevich added a comment - Ok, I will add * 1000 there.
        Hide
        slebresne Sylvain Lebresne added a comment -

        nit: we have a FBUtilities.timestampMicros()

        Show
        slebresne Sylvain Lebresne added a comment - nit: we have a FBUtilities.timestampMicros()
        Hide
        xedin Pavel Yaskevich added a comment -

        Right, this is what I was thinking about

        Show
        xedin Pavel Yaskevich added a comment - Right, this is what I was thinking about
        Hide
        xedin Pavel Yaskevich added a comment -

        Committed change to timestampMicros().

        Show
        xedin Pavel Yaskevich added a comment - Committed change to timestampMicros().
        Hide
        hudson Hudson added a comment -

        Integrated in Cassandra #1673 (See https://builds.apache.org/job/Cassandra/1673/)
        change System.currentTimeMillis() to FBUtilities.timestampMicros(), related to CASSANDRA-4432 (Revision 346ac03c29cd1fe763bd01077a5e0c59f12181b3)

        Result = ABORTED
        xedin :
        Files :

        • src/java/org/apache/cassandra/service/MigrationManager.java
        Show
        hudson Hudson added a comment - Integrated in Cassandra #1673 (See https://builds.apache.org/job/Cassandra/1673/ ) change System.currentTimeMillis() to FBUtilities.timestampMicros(), related to CASSANDRA-4432 (Revision 346ac03c29cd1fe763bd01077a5e0c59f12181b3) Result = ABORTED xedin : Files : src/java/org/apache/cassandra/service/MigrationManager.java

          People

          • Assignee:
            xedin Pavel Yaskevich
            Reporter:
            xedin Pavel Yaskevich
            Reviewer:
            Jonathan Ellis
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development