Details

    • Type: Task
    • Status: Resolved
    • Priority: Trivial
    • Resolution: Fixed
    • Fix Version/s: 1.1.0
    • Component/s: None
    • Labels:
    1. v1-0003-update-build-xml.patch
      0.6 kB
      Jake Farrell
    2. v1-0002-upgrade-thrift-jar-and-license.patch
      315 kB
      Jake Farrell
    3. v1-0001-update-generated-thrift-code.patch
      260 kB
      Jake Farrell

      Activity

      Hide
      jfarrell Jake Farrell added a comment -

      yes, all work for the libthrift 0.9.x will go against CASSANDRA-3719, this ticket was initially for the libthrift upgrade from 0.6 to 0.7.

      Show
      jfarrell Jake Farrell added a comment - yes, all work for the libthrift 0.9.x will go against CASSANDRA-3719 , this ticket was initially for the libthrift upgrade from 0.6 to 0.7.
      Hide
      brandon.williams Brandon Williams added a comment -

      Better continued on CASSANDRA-3719?

      Show
      brandon.williams Brandon Williams added a comment - Better continued on CASSANDRA-3719 ?
      Hide
      jfarrell Jake Farrell added a comment -

      i'll be submitting the updated patch for this when I publish the Thrift 0.9 release

      Show
      jfarrell Jake Farrell added a comment - i'll be submitting the updated patch for this when I publish the Thrift 0.9 release
      Hide
      jbellis Jonathan Ellis added a comment -

      Oh, right – we're on 0.7 right now. So yes, I would say that makes upgrading to 0.9 more urgent.

      Show
      jbellis Jonathan Ellis added a comment - Oh, right – we're on 0.7 right now. So yes, I would say that makes upgrading to 0.9 more urgent.
      Hide
      jbellis Jonathan Ellis added a comment -

      I could be wrong, but I'm reading THRIFT-1121 as saying that the bug was fixed in 0.8 by reverting the patch that caused the regression.

      Show
      jbellis Jonathan Ellis added a comment - I could be wrong, but I'm reading THRIFT-1121 as saying that the bug was fixed in 0.8 by reverting the patch that caused the regression.
      Hide
      dbrosius@apache.org Dave Brosius added a comment -

      does THRIFT-1121 make thrift 0.9 important then?

      Show
      dbrosius@apache.org Dave Brosius added a comment - does THRIFT-1121 make thrift 0.9 important then?
      Hide
      tjake T Jake Luciani added a comment -

      committed

      Show
      tjake T Jake Luciani added a comment - committed
      Hide
      jfarrell Jake Farrell added a comment -

      The exclusion of slf4j-log4j12 in the cass build.xml within the thrift stanza was due to the thrift initially including this as a dependency in thrift 0.6.1. I fixed this in the 0.7 release so the exclusion will no longer be needed the cass build.xml. slf4j 1.6.1 is a listed cassandra dependency which should not be getting removed by the listed thrift exclusion. The contrib item should be using this version of slf4j provided by cassandra

      Show
      jfarrell Jake Farrell added a comment - The exclusion of slf4j-log4j12 in the cass build.xml within the thrift stanza was due to the thrift initially including this as a dependency in thrift 0.6.1. I fixed this in the 0.7 release so the exclusion will no longer be needed the cass build.xml. slf4j 1.6.1 is a listed cassandra dependency which should not be getting removed by the listed thrift exclusion. The contrib item should be using this version of slf4j provided by cassandra
      Hide
      appodictic Edward Capriolo added a comment -

      I am not sure if your patch catches this but the contrib that is used in unit testing seems unhappy because it is using a removed slf4j logging function.

      Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.548 sec <<< FAILURE!
      com.jointhegrid.bakery.BakeryTest  Time elapsed: 0 sec  <<< ERROR!
      java.lang.NoSuchMethodError: org.slf4j.helpers.MessageFormatter.format(Ljava/lang/String;Ljava/lang/Object;)Lorg/slf4j/helper
      s/FormattingTuple;
        at org.slf4j.impl.Log4jLoggerAdapter.info(Log4jLoggerAdapter.java:323)
        at org.apache.cassandra.config.DatabaseDescriptor.<clinit>(DatabaseDescriptor.java:235)
        at org.apache.cassandra.contrib.utils.service.CassandraServiceDataCleaner.makeDirsIfNotExist(Unknown Source)
        at org.apache.cassandra.contrib.utils.service.CassandraServiceDataCleaner.prepare(Unknown Source)
       
      Show
      appodictic Edward Capriolo added a comment - I am not sure if your patch catches this but the contrib that is used in unit testing seems unhappy because it is using a removed slf4j logging function. Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.548 sec <<< FAILURE! com.jointhegrid.bakery.BakeryTest Time elapsed: 0 sec <<< ERROR! java.lang.NoSuchMethodError: org.slf4j.helpers.MessageFormatter.format(Ljava/lang/String;Ljava/lang/Object;)Lorg/slf4j/helper s/FormattingTuple; at org.slf4j.impl.Log4jLoggerAdapter.info(Log4jLoggerAdapter.java:323) at org.apache.cassandra.config.DatabaseDescriptor.<clinit>(DatabaseDescriptor.java:235) at org.apache.cassandra.contrib.utils.service.CassandraServiceDataCleaner.makeDirsIfNotExist(Unknown Source) at org.apache.cassandra.contrib.utils.service.CassandraServiceDataCleaner.prepare(Unknown Source)
      Hide
      jfarrell Jake Farrell added a comment -

      Hive issue found to be IncompatibleClassChangeError due to compiling against newer version of thrift and then using version within cassandra at runtime.

      Show
      jfarrell Jake Farrell added a comment - Hive issue found to be IncompatibleClassChangeError due to compiling against newer version of thrift and then using version within cassandra at runtime.
      Hide
      jfarrell Jake Farrell added a comment -

      Edward is there an open Thrift ticket on the problem you are having?

      Show
      jfarrell Jake Farrell added a comment - Edward is there an open Thrift ticket on the problem you are having?
      Hide
      appodictic Edward Capriolo added a comment -

      Hive made the jump to 0.7.0 thrift for hive 0.8.0. Currently this makes it hard to initialize an embedded C* process inside a hive unit test.

      Show
      appodictic Edward Capriolo added a comment - Hive made the jump to 0.7.0 thrift for hive 0.8.0. Currently this makes it hard to initialize an embedded C* process inside a hive unit test.
      Hide
      jbellis Jonathan Ellis added a comment -

      Where was the Thrift wire-compatibility change? Was that 0.6 -> 0.7? If so maybe we should upgrade to 0.7 for our 1.1 release so that people can use a modern Thrift client-side.

      Show
      jbellis Jonathan Ellis added a comment - Where was the Thrift wire-compatibility change? Was that 0.6 -> 0.7? If so maybe we should upgrade to 0.7 for our 1.1 release so that people can use a modern Thrift client-side.
      Hide
      jfarrell Jake Farrell added a comment -

      0.8 was released about 2 weeks ago and there are no urgent blockers or huge additions to justify a new rc right now. If you need me to rebase the 0.7 version attached to this ticket let me know

      Show
      jfarrell Jake Farrell added a comment - 0.8 was released about 2 weeks ago and there are no urgent blockers or huge additions to justify a new rc right now. If you need me to rebase the 0.7 version attached to this ticket let me know
      Hide
      slebresne Sylvain Lebresne added a comment -

      Is there any chance 0.9 will be released any time soon ? Because otherwise maybe it's worth updating to 0.7 for 1.1.

      Show
      slebresne Sylvain Lebresne added a comment - Is there any chance 0.9 will be released any time soon ? Because otherwise maybe it's worth updating to 0.7 for 1.1.
      Hide
      jfarrell Jake Farrell added a comment -

      In THRIFT-1167 the TNonblockingTransport in TNonblockingServer.FrameBuffer was moved into AbstractNonblockingServer.FrameBuffer and was changed from public to private. This causes the transport to not be available for SocketSessionManagementService as noted above. There is no short term workaround for this. I have everything else ready for patching but with this issue it will be impossible to use Thrift 0.8.0. Fix committed for this (THRIFT-1464) and will be available in the next Thrift release. I'll keep this ticket and update the server then.

      Show
      jfarrell Jake Farrell added a comment - In THRIFT-1167 the TNonblockingTransport in TNonblockingServer.FrameBuffer was moved into AbstractNonblockingServer.FrameBuffer and was changed from public to private. This causes the transport to not be available for SocketSessionManagementService as noted above. There is no short term workaround for this. I have everything else ready for patching but with this issue it will be impossible to use Thrift 0.8.0. Fix committed for this ( THRIFT-1464 ) and will be available in the next Thrift release. I'll keep this ticket and update the server then.
      Hide
      jfarrell Jake Farrell added a comment -

      Jake Luciani and I where talking about this, changing to update to 0.8 and removing custom THsHa and use the default. I'll have a patch for this shortly

      Show
      jfarrell Jake Farrell added a comment - Jake Luciani and I where talking about this, changing to update to 0.8 and removing custom THsHa and use the default. I'll have a patch for this shortly
      Hide
      slebresne Sylvain Lebresne added a comment -

      Thrift 0.8 is out now. However, I quickly tried updating trunk to it but there is a few hiccups. In particular, it has change the access to the trans_ field of AbstractNonBlockingServer.FrameBuffer which we use in our custom servers to record the socket in SocketSessionManagementService. I'm far from being a thrift expert but I didn't see a very trivial way to deal with that.

      Also, probably most of the bulk of our current CustomTHsHaServer can probably go away since 0.8 includes THRIFT-1167 (though we still want to keep the part that register the socket).

      Anyway, that task may be a tiny bit more than trivial if we want to upgrade to 0.8 (which we should). So let's avoid doing this the day before the 1.1 freeze.

      Show
      slebresne Sylvain Lebresne added a comment - Thrift 0.8 is out now. However, I quickly tried updating trunk to it but there is a few hiccups. In particular, it has change the access to the trans_ field of AbstractNonBlockingServer.FrameBuffer which we use in our custom servers to record the socket in SocketSessionManagementService. I'm far from being a thrift expert but I didn't see a very trivial way to deal with that. Also, probably most of the bulk of our current CustomTHsHaServer can probably go away since 0.8 includes THRIFT-1167 (though we still want to keep the part that register the socket). Anyway, that task may be a tiny bit more than trivial if we want to upgrade to 0.8 (which we should). So let's avoid doing this the day before the 1.1 freeze.
      Hide
      jfarrell Jake Farrell added a comment -

      I'm preparing the 0.8.0rc now, we should wait and go with that once its done

      Show
      jfarrell Jake Farrell added a comment - I'm preparing the 0.8.0rc now, we should wait and go with that once its done
      Hide
      jbellis Jonathan Ellis added a comment -

      How close is 0.8? Should we just skip to that in 1.1?

      Show
      jbellis Jonathan Ellis added a comment - How close is 0.8? Should we just skip to that in 1.1?
      Hide
      jfarrell Jake Farrell added a comment -

      nothing urgent that i'm aware of

      Show
      jfarrell Jake Farrell added a comment - nothing urgent that i'm aware of
      Hide
      jbellis Jonathan Ellis added a comment -

      Does this fix anything urgent?

      If not I'd be included to put it in 1.1 instead of risking 1.0 stability during the freeze.

      Show
      jbellis Jonathan Ellis added a comment - Does this fix anything urgent? If not I'd be included to put it in 1.1 instead of risking 1.0 stability during the freeze.
      Hide
      jfarrell Jake Farrell added a comment -

      Upgrades thrift jar and generated code to latest 0.7.0 version

      Show
      jfarrell Jake Farrell added a comment - Upgrades thrift jar and generated code to latest 0.7.0 version

        People

        • Assignee:
          jfarrell Jake Farrell
          Reporter:
          jfarrell Jake Farrell
          Reviewer:
          T Jake Luciani
        • Votes:
          0 Vote for this issue
          Watchers:
          5 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved:

            Development