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

Consider upgrade to thrift 0.9.2 (or later)

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Minor
    • Resolution: Fixed
    • Fix Version/s: 2.1.3
    • Component/s: Configuration, Packaging
    • Labels:
      None

      Description

      Folks using Astyanax and the like are subject to https://issues.apache.org/jira/browse/THRIFT-1457 and may run into heap pressure on the Cassandra side for larger read request, as thrift doesn't reset its internal buffer. This can lead to larger TFramedTransport instances will be kept on the heap.

      I've seen at least one situation where this has saved around 1Gb of heap space on average.

      1. thrift1457fix_cassandra_heap_framedtransport.png
        97 kB
        Thomas Steinmaurer
      2. thrift1457fix_cassandra_heap.png
        4 kB
        Thomas Steinmaurer

        Activity

        Hide
        tjake T Jake Luciani added a comment -

        This would need to go in for 3.0 does that work?

        Show
        tjake T Jake Luciani added a comment - This would need to go in for 3.0 does that work?
        Hide
        tsteinmaurer Thomas Steinmaurer added a comment -

        As a DataStax customer and currently/still in a Astyanax/Thrift-based scenario, I would like to sincerely ask the Cassandra project to investigate including the newer library in Cassandra 2.0x already. I guess 3.0 is a long-road ahead until it is ready for production. As far as I can see is that Cassandra 2.0.11 resp. at least DSE 4.5.3 is using 0.9.1 so an upgrade to 0.9.2 might not be that tricky. I must admit that I haven't investigated the code or library dependency changes in 0.9.2, but I can say that a simple drop-in replacement of 0.9.2 in our DSE 4.5.3 based loadtest environment works without troubles. Attached a screen of the Cassandra heap improvement in our tests simply by replacing thrift 0.9.1 with 0.9.2.

        This is in an environment with 3 of our servers hitting a 6 node EC2 (m1.xlarge) based Cassandra cluster with an Astyanax max connection pool setting of 50 per Cassandra node, thus max. 150 (3 * 50) connections per Cassandra nodes, where each connection is represented by a TFramedTransport instance on the Cassandra side which can grow dramatically depending on the Client/Astyanax single read request size.

        Show
        tsteinmaurer Thomas Steinmaurer added a comment - As a DataStax customer and currently/still in a Astyanax/Thrift-based scenario, I would like to sincerely ask the Cassandra project to investigate including the newer library in Cassandra 2.0x already. I guess 3.0 is a long-road ahead until it is ready for production. As far as I can see is that Cassandra 2.0.11 resp. at least DSE 4.5.3 is using 0.9.1 so an upgrade to 0.9.2 might not be that tricky. I must admit that I haven't investigated the code or library dependency changes in 0.9.2, but I can say that a simple drop-in replacement of 0.9.2 in our DSE 4.5.3 based loadtest environment works without troubles. Attached a screen of the Cassandra heap improvement in our tests simply by replacing thrift 0.9.1 with 0.9.2. This is in an environment with 3 of our servers hitting a 6 node EC2 (m1.xlarge) based Cassandra cluster with an Astyanax max connection pool setting of 50 per Cassandra node, thus max. 150 (3 * 50) connections per Cassandra nodes, where each connection is represented by a TFramedTransport instance on the Cassandra side which can grow dramatically depending on the Client/Astyanax single read request size.
        Hide
        tsteinmaurer Thomas Steinmaurer added a comment -

        I have also attached a memdump of a single Cassandra node after applying the 0.9.2 library showing the tiny TFramedTransport instance sizes although larger single read requests are going on.

        Show
        tsteinmaurer Thomas Steinmaurer added a comment - I have also attached a memdump of a single Cassandra node after applying the 0.9.2 library showing the tiny TFramedTransport instance sizes although larger single read requests are going on.
        Hide
        tjake T Jake Luciani added a comment -

        The 2.0 series has been out far too long to upgrade in a dependency like thrift. Can you drop this in without re-compiling? If so that might be a viable workaround

        Show
        tjake T Jake Luciani added a comment - The 2.0 series has been out far too long to upgrade in a dependency like thrift. Can you drop this in without re-compiling? If so that might be a viable workaround
        Hide
        tjake T Jake Luciani added a comment -

        Since this is a major fix for all Thrift users. I'm ok with putting this into 2.1.

        Show
        tjake T Jake Luciani added a comment - Since this is a major fix for all Thrift users. I'm ok with putting this into 2.1.
        Hide
        jbellis Jonathan Ellis added a comment -

        +1 for 2.1

        Show
        jbellis Jonathan Ellis added a comment - +1 for 2.1
        Hide
        tjake T Jake Luciani added a comment -

        Branch here https://github.com/tjake/cassandra/tree/8685-thrift-upgrade

        Philip Thompson can you ensure all dtests look good with thrift ?

        Show
        tjake T Jake Luciani added a comment - Branch here https://github.com/tjake/cassandra/tree/8685-thrift-upgrade Philip Thompson can you ensure all dtests look good with thrift ?
        Hide
        philipthompson Philip Thompson added a comment -

        Will do. It will be a few hours for them to finish, then I'll report back.

        Show
        philipthompson Philip Thompson added a comment - Will do. It will be a few hours for them to finish, then I'll report back.
        Hide
        tsteinmaurer Thomas Steinmaurer added a comment -

        Can you drop this in without re-compiling?

        Yes.

        • Stopping Cassandra 2.0.11
        • Replacing thrift library 0.9.1 with 0.9.2
        • Starting Cassandra 2.0.11

        Haven't seen any sign of troubles in the Cassandra log or Astyanax based log in our application. To be honest I had hoped that it will be considered even for 2.0.x public. I have no idea about other users roadmap regarding 2.1 in production, but we are moving from 1.2.15 (DSE 3.2.5) to 2.0.11 (DSE 4.5.3) in production soon and will settle with 2.0 for at least this year I guess.

        Anyway, thanks for your swift reply here!

        Show
        tsteinmaurer Thomas Steinmaurer added a comment - Can you drop this in without re-compiling? Yes. Stopping Cassandra 2.0.11 Replacing thrift library 0.9.1 with 0.9.2 Starting Cassandra 2.0.11 Haven't seen any sign of troubles in the Cassandra log or Astyanax based log in our application. To be honest I had hoped that it will be considered even for 2.0.x public. I have no idea about other users roadmap regarding 2.1 in production, but we are moving from 1.2.15 (DSE 3.2.5) to 2.0.11 (DSE 4.5.3) in production soon and will settle with 2.0 for at least this year I guess. Anyway, thanks for your swift reply here!
        Hide
        philipthompson Philip Thompson added a comment -

        Nothing failed that isn't also failing on HEAD.

        Show
        philipthompson Philip Thompson added a comment - Nothing failed that isn't also failing on HEAD.
        Hide
        philipthompson Philip Thompson added a comment -

        +1 for inclusion in 2.1, as no dtests are broken by the change.

        Show
        philipthompson Philip Thompson added a comment - +1 for inclusion in 2.1, as no dtests are broken by the change.
        Hide
        tjake T Jake Luciani added a comment -

        committed

        Show
        tjake T Jake Luciani added a comment - committed

          People

          • Assignee:
            tjake T Jake Luciani
            Reporter:
            ahattrell Adam Hattrell
            Reviewer:
            Philip Thompson
            Tester:
            Philip Thompson
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development